-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
rustdoc: Collect traits in scope for foreign inherent impls #93539
Conversation
r? @jackh726 (rust-highfive has picked a reviewer for you, use r? to override) |
r? @camelid |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit dfbf5b3a81fdacb90d5a3dfd1e9c4a8de896b727 with merge d3dcd7b981fb43fad7a16c0dbc2f20898a0579c5... |
☀️ Try build successful - checks-actions |
Queued d3dcd7b981fb43fad7a16c0dbc2f20898a0579c5 with parent 25862ff, future comparison URL. |
Finished benchmarking commit (d3dcd7b981fb43fad7a16c0dbc2f20898a0579c5): comparison url. Summary: This benchmark run shows 39 relevant regressions 😿 to instruction counts.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never |
The rustdoc side looks good to me, but I'd like someone else to review the metadata code. @michaelwoerister could you look at that part? |
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.
Looks good to me! To clarify, this PR does not make the compiler encode anything new in crate metadata, right? It just adds a method for retrieving something that is already there?
Yes, the method just decodes the whole table of inherent impls which is already there. |
OK, r=me then with the doc comments added. |
@bors r=camelid,michaelwoerister |
📌 Commit afc0030 has been approved by |
⌛ Testing commit afc0030 with merge f9c3bb5dfc36d618e3be59ade04586b9c86c2112... |
💔 Test failed - checks-actions |
@bors retry |
☀️ Test successful - checks-actions |
Finished benchmarking commit (88fb06a): comparison url. Summary: This benchmark run shows 41 relevant regressions 😿 to instruction counts.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression |
@rustbot label: +perf-regression-triaged This was necessary to fix a |
Inherent impls can be inlined for variety of reasons (impls of reexported types, impls available through
Deref
, impls inlined for unclear reasons like in #88679 (comment)).If an impl is inlined, then doc links in its comments are resolved and we may need the set of traits that are in scope at that impl's definition point.
So in this PR we simply collect traits in scope for all inherent impls from other crates if their
Self
type is public, which is very similar for the strategy for trait impls previously used in #88679.Fixes #93476
Fixes #88679 (comment)
Fixes #88679 (comment)