-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix impl being removed excessively #82496
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah thanks for fixing this regression, the fix makes sense to me. I suppose my testing had been too focused on the non-primitive variant...
Thanks to you for the initial implementation! :p I added a foreign non-primitive type ( |
The job Click to see the possible cause of the failure (guessed by this bot)
|
Apparently, the fix is breaking something else later on. Investigating... |
This looks like #75176. Can you get a backtrace of that error with |
Here it is:
|
62fa802
to
8cbc8e0
Compare
I also used this opportunity to remove the useless insert in the primitives. |
This comment has been minimized.
This comment has been minimized.
8cbc8e0
to
5caf793
Compare
This comment has been minimized.
This comment has been minimized.
I was able to track down the origin more precisely:
When we call the
So completely fine from what I can see, however the
Yes, it's the complete output, rustc crashes when trying to display more information because the crate "18" doesn't exist. I precise that we send "core::cmp::Ord" as
And At this point, I'm not sure where to look at to be honest. Is it an issue inside the compiler maybe or are we doing something completely wrong on rustdoc side? cc @rust-lang/compiler |
5caf793
to
237e524
Compare
The job Click to see the possible cause of the failure (guessed by this bot)
|
@rustbot label: -S-waiting-on-review +S-waiting-on-author |
Here's a backtrace with debuginfo:
and the logging immediately before:
|
I don't see anything that looks wrong from resolve's perspective. I'm going to take a look at implementing #68427 (comment). |
Welp, #83738 didn't help unfortunately. It looks like the |
…rochenkov rustdoc: Don't load all extern crates unconditionally Instead, only load the crates that are linked to with intra-doc links. This doesn't help very much with any of rustdoc's fundamental issues with freezing the resolver, but it at least fixes a stable-to-stable regression, and makes the crate loading model somewhat more consistent with rustc's. I tested and it unfortunately does not help at all with rust-lang#82496. Closes rust-lang#68427. Let me know if you want me to open a separate issue for not freezing the resolver. r? `@petrochenkov` cc `@eddyb` `@ollie27`
…rochenkov rustdoc: Don't load all extern crates unconditionally Instead, only load the crates that are linked to with intra-doc links. This doesn't help very much with any of rustdoc's fundamental issues with freezing the resolver, but it at least fixes a stable-to-stable regression, and makes the crate loading model somewhat more consistent with rustc's. I tested and it unfortunately does not help at all with rust-lang#82496. Closes rust-lang#68427. Let me know if you want me to open a separate issue for not freezing the resolver. r? ``@petrochenkov`` cc ``@eddyb`` ``@ollie27``
…chenkov rustdoc: Don't load all extern crates unconditionally Instead, only load the crates that are linked to with intra-doc links. This doesn't help very much with any of rustdoc's fundamental issues with freezing the resolver, but it at least fixes a stable-to-stable regression, and makes the crate loading model somewhat more consistent with rustc's. I tested and it unfortunately does not help at all with rust-lang#82496. Closes rust-lang#68427. Let me know if you want me to open a separate issue for not freezing the resolver. r? `@petrochenkov` cc `@eddyb` `@ollie27`
This can be closed when #84867 lands I believe. |
It can be closed already. :) |
Fixes #82465.
To prevent some impls to get removed, the type has to be "registered". This is because of the change from 8eaf68f , before that commit, the impls were filtered before we added the multiple
Deref
"go down".cc @jryans
r? @jyn514