-
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
resolve: Preserve reexport chains in ModChild
ren
#109500
Conversation
r? @notriddle (rustbot has picked a reviewer for you, use r? to override) |
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
⌛ Trying commit c3fc1061cc50992cc89e06dc3588a799abc6d576 with merge 7a2e02ca3818248e22d3a4b9735822679c02374f... |
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (7a2e02ca3818248e22d3a4b9735822679c02374f): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDBenchmarking 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 may lead 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 Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
|
c3fc106
to
f6b4db9
Compare
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
Some regressions in crates with large numbers of reexported names, like libc-0.2.124, are expected, we are simply encoding more information into metadata. Some of the regressed crates (projection-caching, issue-88862) enter the |
The second commit shows some motivation for the first commit, but otherwise they are not related and I can split this PR into two for reviewing by different people if necessary. |
Most of the substantial changes here are being done in the resolver machinery. r? compiler
Can you construct a test case that triggers one of these new warnings? |
The warnings are not new, they are existing, they are just heavily using |
This comment was marked as resolved.
This comment was marked as resolved.
This may be potentially useful for - avoiding uses of `hir::ItemKind::Use` - preserving documentation comments on all reexports - preserving and checking stability/deprecation info on reexports - all kinds of diagnostics
@bors r+ rollup=never About perf: This regresses a few test cases because there's a bit more rustdoc metadata in our metadata files and that just needs a bit more time to decode. These changes are needed to fix ICEs in rustdoc, which is why I'm landing it tregardless of the perf regressions. We should evaluate how to avoid regressions from rustdoc-specific metadata in general and not on an individual PR basis. |
☀️ Test successful - checks-actions |
Finished benchmarking commit (7201301): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
|
It can be decoded on demand from regular `def_span` tables. Partially mitigates perf regressions from rust-lang#109500.
rustc_metadata: Remove `Span` from `ModChild` It can be decoded on demand from regular `def_span` tables. Partially mitigates perf regressions from rust-lang#109500.
There are a few more regressions in the post merge perf run, but they seem to follow the same pattern as the regressions found pre-merge. I think we can just take this as a necessary trade off we need to make for the fixes that landed (as identified by @oli-obk here). @rustbot label: +perf-regression-triaged |
These perf regressions were partially mitigated by #109772. |
This may be potentially useful for
hir::ItemKind::Use
(which usually lead to correctness issues)The second commit then migrates some hacky logic from rustdoc to
module_reexports
to make it simpler and more correct.Ideally rustdoc should use
module_reexports
immediately at the top level, sohir::ItemKind::Use
s are never used.The second commit also fixes issues with #109330 and therefore
Fixes #109631
Fixes #109614
Fixes #109424