Repository navigation
Move rustc_middle::ty::Const to rustc_type_ir Part 2 - #163258
Jamesbarford wants to merge 8 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 |
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. :) |
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 comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
39c689e to
2255191
Compare
|
Some changes occurred in compiler/rustc_codegen_llvm/src/debuginfo cc @Walnut356 |
|
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 comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| @@ -228,13 +227,21 @@ impl<'tcx> Value<'tcx> { | |||
| } | |||
|
|
|||
| impl<'tcx> rustc_type_ir::inherent::ValueConst<TyCtxt<'tcx>> for Value<'tcx> { | |||
There was a problem hiding this comment.
future work, we should move Value into rustc_type_ir
|
my only question is Why do we need the this is partially out of cache for me unfortunately 🤔 |
|
|
That's inconsistent, is it not. If we need to use Either have all these functions outside of rustc_type_ir or all of them in it I'd say |
…herent::TypingEnv`
Good point, I've made a |
|
☔ The latest upstream changes (presumably #164052) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
View all comments
Split by commit;
ConstExtfromrustc_middle, creating small helpers to aid this. Subsequently deleteConstExt.ConstExtfrom imports in compiler.ConstExtfrom imports in clippy.r? lcnr