-
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
Reorder check_item_type diagnostics so they occur next to the corresponding check_well_formed
diagnostics
#117213
Conversation
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
check_item_type
to work on LocalDefId
instead of ItemId
check_well_formed
diagnostics
I don't expect a perf effect, but technically some incremental cache dep graph could be affected @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
…try> Reorder check_item_type diagnostics so they occur next to the corresponding `check_well_formed` diagnostics The first commit is just a cleanup. The second commit moves most checks from `check_mod_item_types` into `check_well_formed`, invoking the checks in lockstep per-item instead of iterating over all items twice.
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
cc @rust-lang/rustdoc I had to revert some of #97842, as that only works if we avoid some sanity checks. We'll need to figure out a new way to get whatever was desired by that PR (though my changes did not affect any test output I think?) |
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
…try> Reorder check_item_type diagnostics so they occur next to the corresponding `check_well_formed` diagnostics The first commit is just a cleanup. The second commit moves most checks from `check_mod_item_types` into `check_well_formed`, invoking the checks in lockstep per-item instead of iterating over all items twice.
This comment has been minimized.
This comment has been minimized.
e1ec23b
to
d87c513
Compare
cc @notriddle as you are the author of #97842. |
This comment has been minimized.
This comment has been minimized.
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.
r=me
If it's necessary to make other cleanups on the compiler, reverting #97842 is acceptable. I'll make a follow-up PR to accomplish the same thing in a different way. |
I'm not against it but you're the person who worked on it so just wanted you to be aware of it. :) |
d87c513
to
1af53e4
Compare
Some changes occurred in GUI tests. |
The 1.4% regression must be noise, it is purely in codegen, and this PR does not affect codegen. The other regressions are real, either in incremental cache loading or in wf-check. Surprising, so I'll dig into those. |
The regression is because we actually need to do some work when
|
@bors r=estebank |
🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened. |
…stebank Reorder check_item_type diagnostics so they occur next to the corresponding `check_well_formed` diagnostics The first commit is just a cleanup. The second commit moves most checks from `check_mod_item_types` into `check_well_formed`, invoking the checks in lockstep per-item instead of iterating over all items twice.
💥 Test timed out |
@bors retry |
☀️ Test successful - checks-actions |
Finished benchmarking commit (791a53f): comparison URL. Overall result: ❌✅ regressions and improvements - 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.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 670.353s -> 669.247s (-0.16%) |
The first commit is just a cleanup.
The second commit moves most checks from
check_mod_item_types
intocheck_well_formed
, invoking the checks in lockstep per-item instead of iterating over all items twice.