Skip to content

Commit

Permalink
Unrolled build for rust-lang#136839
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#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.~~
  • Loading branch information
rust-timer authored Feb 11, 2025
2 parents c182ce9 + c1da4f1 commit 8ad7b3a
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 41 deletions.
44 changes: 3 additions & 41 deletions compiler/rustc_const_eval/src/interpret/util.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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> {
Expand Down
12 changes: 12 additions & 0 deletions tests/mir-opt/gvn_type_id_polymorphic.cursed_is_i32.GVN.diff
Original file line number Diff line number Diff line change
@@ -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;
}
}

22 changes: 22 additions & 0 deletions tests/mir-opt/gvn_type_id_polymorphic.rs
Original file line number Diff line number Diff line change
@@ -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>());
}

0 comments on commit 8ad7b3a

Please sign in to comment.