Move rustc_middle::ty::Const to rustc_type_ir Part 2 - #163258
Jamesbarford wants to merge 5 commits into
Conversation
|
cc @bjorn3 Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred in cc @BoxyUwU Some changes occurred in exhaustiveness checking cc @Nadrieril Some changes occurred in match checking cc @Nadrieril HIR ty lowering was modified cc @fmease Some changes occurred in match lowering cc @Nadrieril Some changes occurred to the CTFE machinery
cc @rust-lang/clippy Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri
Some changes occurred in compiler/rustc_sanitizers cc @rcvalle |
rustbot
added
A-LLVM
This comment has been minimized.
This comment has been minimized.
This is ty::Const, not mir::Const, right? Would be good to clarify so these PRs are easier to interpret. :) |
Jamesbarford
changed the title
Const from rustc_middle to rustc_type_ir Part 2rustc_middle::ty::Const to rustc_type_ir Part 2
| let valtree = | ||
| ty::ValTree::from_scalar_int(tcx, ScalarInt::try_from_uint(bits, size).unwrap()); | ||
| ty::Const::new_value(tcx, valtree, ty) | ||
| } |
There was a problem hiding this comment.
this function is kind of scuffed :< I don't want this in rustc_type_ir if I am honest. More generally TypingEnv is in a weird state and I'd like to keep this out of rustc_type_ir for now 🤔 can we maybe keep this in an extension trait for now?
There was a problem hiding this comment.
Hmm what about on Interner;
fn const_from_bits(self, bits: u128, typing_env: Self::TypingEnv, ty: Self::Ty) -> Const<Self>;Then keep the implementation in the rustc_middle's interner implementation? That way we don't need to resurrect the ConstExt and the scuffed parts stay in rustc_middle? Obviously happy to revert if that's a terrible idea.
There was a problem hiding this comment.
I've given it a go in this commit; 39c689e, doesn't feel too bad as it uses a few interner methods
| } else { | ||
| const_v.try_to_leaf().map(|s| s.to_target_usize(self)) | ||
| } | ||
| } |
There was a problem hiding this comment.
why exactly does this need to be on the interner? I guess the to_target_usize is the actually relevant part? 🤔
There was a problem hiding this comment.
Yes s.to_target_usize() to the relevant part. I've now made to_target_usize a method on ValueConst in inherent.rs. Could be less bad than bolting another helper method on to Interner?
I've done so in; bcc98fa
This comment has been minimized.
This comment has been minimized.
4f1d48f to
828380b
Compare
|
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. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
☔ The latest upstream changes (presumably #163609) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
This branch has not been deployed
5 participants
Split by commit;
ConstExtfromrustc_middle, creating small helpers to aid this. Subsequently deleteConstExt.ConstExtfrom imports in compiler.ConstExtfrom imports in clippy.r? lcnr