Prefer ModId in more places - #159787
Conversation
|
HIR ty lowering was modified cc @fmease |
|
r? @mu001999 rustbot has assigned @mu001999. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Cache and use more ModId
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (d23ae58): comparison URL. Overall result: ❌ regressions - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never rustc-perf Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -0.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis perf run didn't have relevant results for this metric. Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 488.274s -> 486.771s (-0.31%) |
|
Seems this instead leads to a slight perf regression. Do you have any ideas? @rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
This comment has been minimized.
This comment has been minimized.
Use ModId more for visibility checks from TypeckRootCtxt. This just simplifies things a bit and adds consistency.
We generally expect Visibility to have ModId or LocalModId, so it seems good to restrict the impls as such. There is just one error path needing adjustment to check that we actually have a ModId. It should be okay since, if it is not a module, an error will be emitted elsewhere.
Especially in adjust_ident_and_get_scope and is_accessible_from.
59f64b4 to
afa08c3
Compare
|
cc @rust-lang/clippy |
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
This should be about perf neutral now. Adding a |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Prefer ModId in more places
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (5eb45c3): comparison URL. Overall result: no relevant changes - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -perf-regression Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)Results (primary -2.3%, secondary 2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -0.5%, secondary 10.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 475.629s -> 474.896s (-0.15%) |
| let (module, _span) = tcx.hir_get_module(mod_id); | ||
|
|
||
| cx.with_lint_attrs(hir_id, |cx| { | ||
| cx.with_lint_attrs(HirId::make_owner(mod_id.to_local_def_id()), |cx| { |
There was a problem hiding this comment.
| cx.with_lint_attrs(HirId::make_owner(mod_id.to_local_def_id()), |cx| { | |
| cx.with_lint_attrs(mod_id.into(), |cx| { |
|
r=me after nits @rustbot author |
View all comments
Simplifying code a bit and adding consistency. Generally prefer passing around
ModIdinstead of a more specific ID when aModIdwill do.