-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove TypeAndMut
from ty::RawPtr
variant, make it take Ty
and Mutability
#122852
Conversation
Some changes occurred in exhaustiveness checking cc @Nadrieril Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in compiler/rustc_symbol_mangling/src/typeid cc @rust-lang/project-exploit-mitigations, @rcvalle Some changes occurred in compiler/rustc_codegen_gcc Type relation code was changed Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri The Miri subtree was changed cc @rust-lang/miri |
This comment has been minimized.
This comment has been minimized.
9df16cc
to
4f90688
Compare
(&ty::RawPtr(mt_a), &ty::RawPtr(mt_b)) => { | ||
check_mutbl(mt_a, mt_b, &|ty| Ty::new_imm_ptr(tcx, ty)) | ||
} | ||
(&ty::Ref(_, ty_a, mutbl_a), &ty::RawPtr(ty_b, mutbl_b)) => check_mutbl( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these the only remaining uses of TypeAndMut
?
This could just be check_mutbl(mutbl_a, mutbl_b)
and then use target
and source
for the error messagte instead.
ty::RawPtr(ref mt) => { | ||
self.add_constraints_from_mt(current, mt, variance); | ||
ty::RawPtr(ty, mutbl) => { | ||
self.add_constraints_from_mt(current, &ty::TypeAndMut { ty, mutbl }, variance); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay... not the only use of TypeAndMut
🤷 idk if it's useful to fully remove it then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an issue tracking "entirely remove TypeAndMut
"? That would be the logical next step IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not totally sure, some of the remaining uses here seem appropriate as an abstraction over both Ref
and RawPtr
. Personally don't care too much about keeping it around 🤷
@@ -984,13 +978,13 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { | |||
|
|||
let (is_ref, mt_a) = match *a.kind() { | |||
ty::Ref(_, ty, mutbl) => (true, ty::TypeAndMut { ty, mutbl }), | |||
ty::RawPtr(mt) => (false, mt), | |||
ty::RawPtr(ty, mutbl) => (false, ty::TypeAndMut { ty, mutbl }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this canreturn (is_ref, ty_a, mutbl_a)
instead
r: Region<'tcx>, | ||
ty: Ty<'tcx>, | ||
mutbl: ty::Mutability, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vibe: would have expected r, mutbl, ty
to match the &'r mutbl ty
syntax, same for new_ptr
. Anyways, fine to merge as is
@bors r+ rollup=never |
☔ The latest upstream changes (presumably #120926) made this pull request unmergeable. Please resolve the merge conflicts. |
4f90688
to
6a40dab
Compare
HIR ty lowering was modified cc @fmease |
@bors r=lcnr |
☀️ Test successful - checks-actions |
Finished benchmarking commit (85e449a): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 669.004s -> 669.446s (0.07%) |
Remove `TypeAndMut` from `ty::RawPtr` variant, make it take `Ty` and `Mutability` Pretty much mechanically converting `ty::RawPtr(ty::TypeAndMut { ty, mutbl })` to `ty::RawPtr(ty, mutbl)` and its fallout. r? lcnr cc rust-lang/types-team#124
Pretty much mechanically converting
ty::RawPtr(ty::TypeAndMut { ty, mutbl })
toty::RawPtr(ty, mutbl)
and its fallout.r? lcnr
cc rust-lang/types-team#124