refactor(mgca): Change DefKind::Const and DefKind::AssocConst to have a is_type_const flag#153050
Conversation
|
Some changes occurred to the CTFE machinery Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt changes to the core type system cc @lcnr Some changes occurred in match checking cc @Nadrieril HIR ty lowering was modified cc @fmease This PR changes rustc_public |
This comment has been minimized.
This comment has been minimized.
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
This comment has been minimized.
This comment has been minimized.
|
@BoxyUwU I think this is mostly complete now |
| pub fn as_def_kind(&self) -> DefKind { | ||
| match self { | ||
| Self::Const { .. } => DefKind::AssocConst, | ||
| Self::Const { .. } => DefKind::AssocConst { is_type_const: false }, |
There was a problem hiding this comment.
unconditionally using false for is_type_const seems wrong. AssocKind::Const might need to be extended to also track is_type_const
There was a problem hiding this comment.
Hmm yeah, that seems fair
|
Since this change was largely motivated by a perf regression: @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.
refactor(mgca): Change `DefKind::Const` and `DefKind::AssocConst` to have a `is_type_const` flag
|
Oh an additional thing we can do is remove the |
This comment has been minimized.
This comment has been minimized.
I'd suggest having that in a different PR after this one lands (stacked PRs ftw). This is already a pretty widespread refactor, and IMO having that in a separate PR would be easier to work with |
|
Finished benchmarking commit (c01784d): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -1.4%, secondary 0.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.5%, secondary 7.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.5%, secondary -0.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 480.701s -> 486.306s (1.17%) |
|
@bors p=6 |
|
@BoxyUwU shouldn't we squash the commits (there's a lot of "ci appeasing" commits here) |
|
oh uhh yeah that's probably a good idea hmm |
|
@bors r- |
|
@bors squash |
|
🚧 Squashing... this can take a few minutes. |
…have a `is_type_const` flag * refactor: add `is_type_const` flag to `DefKind::Const` and `AssocConst` * refactor(cleanup) remove the `rhs_is_type_const` query * style: fix formatting * refactor: refactor stuff in librustdoc for new Const and AssocConst * refactor: refactor clippy for the changes * chore: formatting * fix: fix test * fix: fix suggestions * Update context.rs Co-authored-by: Boxy <rust@boxyuwu.dev> * changed AssocKind::Const to store data about being a type const
|
🔨 10 commits were squashed into efc150e. |
b36cd1e to
efc150e
Compare
|
@bors r+ |
This comment has been minimized.
This comment has been minimized.
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing ba15679 (parent) -> 38c0de8 (this PR) Test differencesShow 86 test diffs86 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 38c0de8dcb14d42290042521be9958d37f3fa390 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (38c0de8): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -2.3%, secondary 1.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -3.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.5%, secondary -0.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 479.975s -> 481.431s (0.30%) |
|
perf triage: Tiny regression in @rustbot label: +perf-regression-triaged |
…nst, r=BoxyUwU refactor(mgca): Change `DefKind::Const` and `DefKind::AssocConst` to have a `is_type_const` flag Addresses rust-lang#152940 - Changed `DefKind::Const` and `DefKind::AssocConst` to have a `is_type_const` flag. - changed `is_type_const` query to check for this flag - removed `is_rhs_type_const` query r? @BoxyUwU
View all comments
Addresses #152940
DefKind::ConstandDefKind::AssocConstto have ais_type_constflag.is_type_constquery to check for this flagis_rhs_type_constqueryr? @BoxyUwU