Skip to content

create mir for direct-rhs consts - #165070

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
khyperia:mir_for_direct_const
Nov 2, 2026
Merged

rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
khyperia:mir_for_direct_const

Conversation

@khyperia

@khyperia khyperia commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Alternative to #164982 - see that PR for context.

When referring to a direct constant via a mir::Const::Unevaluated, synthesize a MIR body, whose contents are simply returning a mir::Const::Ty of itself. This effectively provides both a MIR view, as well as a typesystem view, of the same constant, without risking representing the same constant in different ways in the two different representations, because one is just referring to the other.

r? @BoxyUwU

fyi @oli-obk @lcnr

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 28, 2026

let root = tcx.typeck_root_def_id_local(def);
if let Err(e) = tcx.check_transmutes(root) {
if !tcx.is_synthetic_mir(root)

@BoxyUwU BoxyUwU Oct 28, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_transmutes calls typeck, typeck requires HIR, synthetic MIR does not have HIR.

previously, the only synthetic MIR was DefKind::SyntheticCoroutineBody, which is a typeck child, so this call to check_transmutes (which must be called on typeck roots) didn't care and we just happened to be correct here

@rustbot

rustbot commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

BoxyUwU is currently at their maximum review capacity.
They may take a while to respond.

@khyperia

Copy link
Copy Markdown
Member Author

The test differences are generally due to, if the direct const item being referenced by a mir::Const is not WF:

  • Const::Unevaluated bails immediately due to the referenced item's synthesized MIR Body::tainted_by_errors and does not emit additional errors. The taint comes from a tcx.ensure_result().check_well_formed().
  • Const::Ty does not, we just call the try_normalize_generic_arg_after_erasing_regions query and it returns NoSolution and we're like "idk why that is lol, gonna report another error just in case"
    • I think this could match behavior to the mir-wrapper version by returning ErrorHandled::Reported instead of ErrorHandled::TooGeneric here if the trait solver could tell us it resolved to a not-wf impl, but not in this PR IMO
      .map_err(|_| ErrorHandled::TooGeneric(self.cur_span()))

@khyperia
khyperia force-pushed the mir_for_direct_const branch from 6bdcf68 to beca84e Compare October 28, 2026 14:01
@khyperia
khyperia marked this pull request as ready for review October 28, 2026 14:01
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 28, 2026
@rustbot

rustbot commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 28, 2026
@rust-bors

This comment has been minimized.

@khyperia
khyperia force-pushed the mir_for_direct_const branch from beca84e to 6a9928d Compare November 2, 2026 10:13
@rustbot

rustbot commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

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.

} else {
let args = ty::GenericArgs::identity_for_item(tcx, def_id);
let kind = ty::AliasConstKind::InherentImpl { def_id: def_id.into() };
ty::AliasConst::new(tcx, kind, args)

@BoxyUwU BoxyUwU Nov 2, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment for why we don't use mir::Const::Ty(const_of_item(...)), then r=me

View changes since the review

@BoxyUwU

BoxyUwU commented Nov 2, 2026

Copy link
Copy Markdown
Member

@rustbot author

this is a much smaller hack and doesn't touch const eval so im comfortable approving this without talking to const eval folks

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 2, 2026
@khyperia
khyperia force-pushed the mir_for_direct_const branch from 6a9928d to f29e22f Compare November 2, 2026 14:36
@khyperia

khyperia commented Nov 2, 2026

Copy link
Copy Markdown
Member Author

@bors r=BoxyUwU rollup

@rust-bors

rust-bors Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

📌 Commit f29e22f has been tentatively approved by BoxyUwU

It will be put into the queue for this repository once PR CI succeeds.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 2, 2026
rust-bors Bot pushed a commit that referenced this pull request Nov 2, 2026
…uwer

Rollup of 6 pull requests

Successful merges:

 - #163764 (Tweak suggestion of "unreachable due to never pattern")
 - #164771 (std: fix platform checks for hermit)
 - #165070 (create mir for direct-rhs consts)
 - #165121 (test DSL evaluate sequentially)
 - #165288 (Add assembly test for masked index without redundant shifts)
 - #165307 (test(lexer): Add test that ZWJ/ZWNJ are accepted in identifiers)
@rust-bors
rust-bors Bot merged commit eed4130 into rust-lang:main Nov 2, 2026
14 checks passed
@rustbot rustbot added this to the 1.101.0 milestone Nov 2, 2026
rust-bors Bot pushed a commit that referenced this pull request Nov 2, 2026
Rollup merge of #165070 - khyperia:mir_for_direct_const, r=BoxyUwU

create mir for direct-rhs consts

Alternative to #164982 - see that PR for context.

When referring to a direct constant via a `mir::Const::Unevaluated`, synthesize a MIR body, whose contents are simply returning a `mir::Const::Ty` of itself. This effectively provides both a MIR view, as well as a typesystem view, of the same constant, without risking representing the same constant in different ways in the two different representations, because one is just referring to the other.

r? @BoxyUwU

fyi @oli-obk @lcnr
@rust-timer

Copy link
Copy Markdown
Collaborator

Note

This PR was benchmarked as part of triage of its containing rollup: triage URL.

Finished benchmarking commit (e7363cb): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.0% [0.0%, 0.0%] 1
Improvements ✅
(primary)
-0.2% [-0.2%, -0.2%] 3
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.2% [-0.2%, -0.2%] 3

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

Results (secondary -3.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.6% [-3.6%, -3.6%] 1
All ❌✅ (primary) - - 0

Binary size

This perf run didn't have relevant results for this metric.

Artifact size: 406.63 MiB -> 406.59 MiB (-0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf-regression Performance regression. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants