Skip to content

Commit

Permalink
Rollup merge of rust-lang#127068 - compiler-errors:stall-drop, r=BoxyUwU
Browse files Browse the repository at this point in the history
Stall computing instance for drop shim until it has no unsubstituted const params

Do not inline the drop shim for types that still have unsubstituted const params.

## Why?

rust-lang#127030 ICEs because it tries to inline the drop shim for a type with an unsubstituted const param. In order to generate this shim, this requires calling the drop shim builder, which invokes the trait solver to compute whether constituent types need drop (since we compute if a type is copy to disqualify any `Drop` behavior):

https://github.com/rust-lang/rust/blob/9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9/compiler/rustc_mir_dataflow/src/elaborate_drops.rs#L378

However, since we don't keep the param-env of the instance we resolved the item for, we use the wrong param-env:
https://github.com/rust-lang/rust/blob/9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9/compiler/rustc_mir_transform/src/shim.rs#L278
(which is the param-env of `std::ptr::drop_in_place`)

This param-env is notably missing `ConstParamHasTy` predicates, and since we removed the type from consts in rust-lang#125958, we literally cannot prove these predicates in this (relatively) empty param-env. This currently happens in places like the MIR inliner, but may happen elsewhere such as in lints that resolve terminators.

## What?

We force the inliner to not consider calls for `drop_in_place` for types that have unsubstituted const params.

## So what?

This may negatively affect MIR inlining, but I doubt this matters in practice, and fixes a beta regression, so let's fix it. I will look into approaches for fixing this in a more maintainable way, perhaps delaying the creation of drop shim bodies until codegen (like how intrinsics work).
  • Loading branch information
matthiaskrgr authored Jun 28, 2024
2 parents afde848 + f17b27b commit 5afb4c2
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
12 changes: 11 additions & 1 deletion compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}
use rustc_middle::mir::visit::*;
use rustc_middle::mir::*;
use rustc_middle::ty::TypeVisitableExt;
use rustc_middle::ty::{self, Instance, InstanceKind, ParamEnv, Ty, TyCtxt};
use rustc_middle::ty::{self, Instance, InstanceKind, ParamEnv, Ty, TyCtxt, TypeFlags};
use rustc_session::config::{DebugInfo, OptLevel};
use rustc_span::source_map::Spanned;
use rustc_span::sym;
Expand Down Expand Up @@ -306,6 +306,16 @@ impl<'tcx> Inliner<'tcx> {
InstanceKind::Intrinsic(_) | InstanceKind::Virtual(..) => {
return Err("instance without MIR (intrinsic / virtual)");
}

// FIXME(#127030): `ConstParamHasTy` has bad interactions with
// the drop shim builder, which does not evaluate predicates in
// the correct param-env for types being dropped. Stall resolving
// the MIR for this instance until all of its const params are
// substituted.
InstanceKind::DropGlue(_, Some(ty)) if ty.has_type_flags(TypeFlags::HAS_CT_PARAM) => {
return Err("still needs substitution");
}

// This cannot result in an immediate cycle since the callee MIR is a shim, which does
// not get any optimizations run on it. Any subsequent inlining may cause cycles, but we
// do not need to catch this here, we can wait until the inliner decides to continue
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/const-generics/polymorphic-drop-shim.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//@ compile-flags: -Zinline-mir=yes --crate-type=lib
//@ build-pass

use std::mem::ManuallyDrop;

pub struct Foo<T, const N: usize>([T; N]);

pub struct Dorp {}

impl Drop for Dorp {
fn drop(&mut self) {}
}

#[inline]
// SAFETY: call this with a valid allocation idk
pub unsafe fn drop<const M: usize>(x: *mut Foo<Dorp, M>) {
std::ptr::drop_in_place(x);
}

0 comments on commit 5afb4c2

Please sign in to comment.