diff options
| author | Jacob Pratt <jacob@jhpratt.dev> | 2025-02-11 01:02:44 -0500 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-02-11 01:02:44 -0500 |
| commit | 21303103c3eef60e027630cd2aea5f54f868c734 (patch) | |
| tree | 9c702c0f98cd21cc499eca556eaae3cbf7fb6e43 | |
| parent | 4ae214b846655d5132b352b294a806a5aba796e5 (diff) | |
| parent | c1da4f1d3c4fc4beb5edcfa8a303a1dcbe27b65e (diff) | |
| download | rust-21303103c3eef60e027630cd2aea5f54f868c734.tar.gz rust-21303103c3eef60e027630cd2aea5f54f868c734.zip | |
Rollup merge of #136839 - lukas-code:actually-monomorphic-enough, r=compiler-errors
fix ensure_monomorphic_enough
When polymorphization was still a thing, the visitor was used to only recurse into *used generic parameters* of function/closure/coroutine types and allow unused parameters (i.e. the polymorphized parameters) to remain generic.
When polymorphization got removed, this got changed to always treat all parameters as polymorphic and never recurse into them: https://github.com/rust-lang/rust/pull/133883/files#diff-210c59e321070d0ca4625c04e9fb064bf43ddc34082e7e33a7ee8a6c577e95afL44-L62
This is clearly wrong and can cause MIR opts to misbehave, for example this currently prints "false" in release mode:
```rust
#![feature(core_intrinsics)]
fn generic<T>() {}
const fn type_id_of_val<T: 'static>(_: &T) -> u128 {
std::intrinsics::type_id::<T>()
}
fn cursed_is_i32<T: 'static>() -> bool {
(const { type_id_of_val(&generic::<T>) } == const { type_id_of_val(&generic::<i32>) })
}
fn main() {
dbg!(cursed_is_i32::<i32>());
}
```
This PR reverts to the old behavior of always treating all types that contain type parameters as too generic, like we used to do without `-Zpolymorphize` before.
~~I'm not including the above as a test case here, because I think there is little value in testing code paths that have been removed and this seems unlikely to regress in a way that would be caught by a regression test, but let me know if you disagree and want me to add a test anyway.~~
| -rw-r--r-- | compiler/rustc_const_eval/src/interpret/util.rs | 44 | ||||
| -rw-r--r-- | tests/mir-opt/gvn_type_id_polymorphic.cursed_is_i32.GVN.diff | 12 | ||||
| -rw-r--r-- | tests/mir-opt/gvn_type_id_polymorphic.rs | 22 |
3 files changed, 37 insertions, 41 deletions
diff --git a/compiler/rustc_const_eval/src/interpret/util.rs b/compiler/rustc_const_eval/src/interpret/util.rs index eb98e3b5380..ba579e25f03 100644 --- a/compiler/rustc_const_eval/src/interpret/util.rs +++ b/compiler/rustc_const_eval/src/interpret/util.rs @@ -1,12 +1,8 @@ -use std::ops::ControlFlow; - use rustc_hir::def_id::LocalDefId; use rustc_middle::mir; use rustc_middle::mir::interpret::{AllocInit, Allocation, InterpResult, Pointer}; use rustc_middle::ty::layout::TyAndLayout; -use rustc_middle::ty::{ - self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, -}; +use rustc_middle::ty::{TyCtxt, TypeVisitable, TypeVisitableExt}; use tracing::debug; use super::{InterpCx, MPlaceTy, MemoryKind, interp_ok, throw_inval}; @@ -20,44 +16,10 @@ where T: TypeVisitable<TyCtxt<'tcx>>, { debug!("ensure_monomorphic_enough: ty={:?}", ty); - if !ty.has_param() { - return interp_ok(()); - } - - struct FoundParam; - struct UsedParamsNeedInstantiationVisitor {} - - impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for UsedParamsNeedInstantiationVisitor { - type Result = ControlFlow<FoundParam>; - - fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result { - if !ty.has_param() { - return ControlFlow::Continue(()); - } - - match *ty.kind() { - ty::Param(_) => ControlFlow::Break(FoundParam), - ty::Closure(..) | ty::CoroutineClosure(..) | ty::Coroutine(..) | ty::FnDef(..) => { - ControlFlow::Continue(()) - } - _ => ty.super_visit_with(self), - } - } - - fn visit_const(&mut self, c: ty::Const<'tcx>) -> Self::Result { - match c.kind() { - ty::ConstKind::Param(..) => ControlFlow::Break(FoundParam), - _ => c.super_visit_with(self), - } - } - } - - let mut vis = UsedParamsNeedInstantiationVisitor {}; - if matches!(ty.visit_with(&mut vis), ControlFlow::Break(FoundParam)) { + if ty.has_param() { throw_inval!(TooGeneric); - } else { - interp_ok(()) } + interp_ok(()) } impl<'tcx> InterpretationResult<'tcx> for mir::interpret::ConstAllocation<'tcx> { diff --git a/tests/mir-opt/gvn_type_id_polymorphic.cursed_is_i32.GVN.diff b/tests/mir-opt/gvn_type_id_polymorphic.cursed_is_i32.GVN.diff new file mode 100644 index 00000000000..2f83f54d2af --- /dev/null +++ b/tests/mir-opt/gvn_type_id_polymorphic.cursed_is_i32.GVN.diff @@ -0,0 +1,12 @@ +- // MIR for `cursed_is_i32` before GVN ++ // MIR for `cursed_is_i32` after GVN + + fn cursed_is_i32() -> bool { + let mut _0: bool; + + bb0: { + _0 = Eq(const cursed_is_i32::<T>::{constant#0}, const cursed_is_i32::<T>::{constant#1}); + return; + } + } + diff --git a/tests/mir-opt/gvn_type_id_polymorphic.rs b/tests/mir-opt/gvn_type_id_polymorphic.rs new file mode 100644 index 00000000000..39bc5c24ecc --- /dev/null +++ b/tests/mir-opt/gvn_type_id_polymorphic.rs @@ -0,0 +1,22 @@ +//@ test-mir-pass: GVN +//@ compile-flags: -C opt-level=2 + +#![feature(core_intrinsics)] + +fn generic<T>() {} + +const fn type_id_of_val<T: 'static>(_: &T) -> u128 { + std::intrinsics::type_id::<T>() +} + +// EMIT_MIR gvn_type_id_polymorphic.cursed_is_i32.GVN.diff +fn cursed_is_i32<T: 'static>() -> bool { + // CHECK-LABEL: fn cursed_is_i32( + // CHECK: _0 = Eq(const cursed_is_i32::<T>::{constant#0}, const cursed_is_i32::<T>::{constant#1}); + // CHECK-NEXT: return; + (const { type_id_of_val(&generic::<T>) } == const { type_id_of_val(&generic::<i32>) }) +} + +fn main() { + dbg!(cursed_is_i32::<i32>()); +} |
