Auto merge of #148602 - BoxyUwU:coercion_cleanup_uncontroversial, r=lcnr

misc coercion cleanups and handle safety correctly

r? lcnr

### "remove normalize call"

Fixes rust-lang/rust#132765

If the normalization fails we would sometimes get a `TypeError` containing inference variables created inside of the probe used by coercion. These would then get leaked out causing ICEs in diagnostics logic

### "leak check and lub for closure<->closure coerce-lubs of same defids"

Fixes https://github.com/rust-lang/trait-system-refactor-initiative/issues/233
```rust
fn peculiar() -> impl Fn(u8) -> u8 {
    return |x| x + 1
}
```
the `|x| x + 1` expr has a type of `Closure(?31t)` which we wind up inferring the RPIT to. The `CoerceMany` `ret_coercion` for the whole `peculiar` typeck has an expected type of `RPIT` (unnormalized). When we type check the `return |x| x + 1` expr we go from the never type to `Closure(?31t)` which then participates in the `ret_coercion` giving us a `coerce-lub(RPIT, Closure(?31t))`.

Normalizing `RPIT` gives us some `Closure(?50t)` where `?31t` and `?50t` have been unified with `?31t` as the root var. `resolve_vars_if_possible` doesn't resolve infer vars to their roots so these wind up with different structural identities so the fast path doesn't apply and we fall back to coercing to a `fn` ptr. cc rust-lang/rust#147193 which also fixes this

New solver probably just gets more inference variables here because canonicalization + generally different approach to normalization of opaques. Idk :3

### FCP worthy stuffy

there are some other FCP worthy things but they're in my FCP comment which also contains some analysis of the breaking nature of the previously listed changes in this PR: https://github.com/rust-lang/rust/pull/148602#issuecomment-3503497467
This commit is contained in:
bors 2025-12-05 11:46:41 +00:00
commit 97b131c900
30 changed files with 656 additions and 444 deletions

View file

@ -1061,7 +1061,10 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
Rvalue::Cast(cast_kind, op, ty) => {
match *cast_kind {
CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer, coercion_source) => {
CastKind::PointerCoercion(
PointerCoercion::ReifyFnPointer(target_safety),
coercion_source,
) => {
let is_implicit_coercion = coercion_source == CoercionSource::Implicit;
let src_ty = op.ty(self.body, tcx);
let mut src_sig = src_ty.fn_sig(tcx);
@ -1078,6 +1081,10 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
src_sig = safe_sig;
}
if src_sig.safety().is_safe() && target_safety.is_unsafe() {
src_sig = tcx.safe_to_unsafe_sig(src_sig);
}
// HACK: This shouldn't be necessary... We can remove this when we actually
// get binders with where clauses, then elaborate implied bounds into that
// binder, and implement a higher-ranked subtyping algorithm that actually

View file

@ -689,7 +689,7 @@ fn codegen_stmt<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, cur_block: Block, stmt:
lval.write_cvalue(fx, res);
}
Rvalue::Cast(
CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer, _),
CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer(_), _),
ref operand,
to_ty,
) => {

View file

@ -405,7 +405,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let lladdr = bx.ptrtoint(llptr, llcast_ty);
OperandValue::Immediate(lladdr)
}
mir::CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer, _) => {
mir::CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer(_), _) => {
match *operand.layout.ty.kind() {
ty::FnDef(def_id, args) => {
let instance = ty::Instance::resolve_for_fn_ptr(

View file

@ -627,7 +627,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
| PointerCoercion::ArrayToPointer
| PointerCoercion::UnsafeFnPointer
| PointerCoercion::ClosureFnPointer(_)
| PointerCoercion::ReifyFnPointer,
| PointerCoercion::ReifyFnPointer(_),
_,
),
_,

View file

@ -74,7 +74,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
bug!("{cast_kind:?} casts are for borrowck only, not runtime MIR");
}
CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer, _) => {
CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer(_), _) => {
// All reifications must be monomorphic, bail out otherwise.
ensure_monomorphic_enough(*self.tcx, src.layout.ty)?;

File diff suppressed because it is too large Load diff

View file

@ -9,8 +9,9 @@ use crate::ty::{Ty, TyCtxt};
#[derive(Clone, Copy, Debug, PartialEq, Eq, TyEncodable, TyDecodable, Hash, HashStable)]
pub enum PointerCoercion {
/// Go from a fn-item type to a fn-pointer type.
ReifyFnPointer,
/// Go from a fn-item type to a fn pointer or an unsafe fn pointer.
/// It cannot convert an unsafe fn-item to a safe fn pointer.
ReifyFnPointer(hir::Safety),
/// Go from a safe fn pointer to an unsafe fn pointer.
UnsafeFnPointer,

View file

@ -2837,7 +2837,7 @@ slice_interners!(
);
impl<'tcx> TyCtxt<'tcx> {
/// Given a `fn` type, returns an equivalent `unsafe fn` type;
/// Given a `fn` sig, returns an equivalent `unsafe fn` type;
/// that is, a `fn` type that is equivalent in every way for being
/// unsafe.
pub fn safe_to_unsafe_fn_ty(self, sig: PolyFnSig<'tcx>) -> Ty<'tcx> {
@ -2845,6 +2845,14 @@ impl<'tcx> TyCtxt<'tcx> {
Ty::new_fn_ptr(self, sig.map_bound(|sig| ty::FnSig { safety: hir::Safety::Unsafe, ..sig }))
}
/// Given a `fn` sig, returns an equivalent `unsafe fn` sig;
/// that is, a `fn` sig that is equivalent in every way for being
/// unsafe.
pub fn safe_to_unsafe_sig(self, sig: PolyFnSig<'tcx>) -> PolyFnSig<'tcx> {
assert!(sig.safety().is_safe());
sig.map_bound(|sig| ty::FnSig { safety: hir::Safety::Unsafe, ..sig })
}
/// Given the def_id of a Trait `trait_def_id` and the name of an associated item `assoc_name`
/// returns true if the `trait_def_id` defines an associated item of name `assoc_name`.
pub fn trait_may_define_assoc_item(self, trait_def_id: DefId, assoc_name: Ident) -> bool {

View file

@ -1518,7 +1518,7 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
return Some(value);
}
if let CastKind::PointerCoercion(ReifyFnPointer | ClosureFnPointer(_), _) = kind {
if let CastKind::PointerCoercion(ReifyFnPointer(_) | ClosureFnPointer(_), _) = kind {
// Each reification of a generic fn may get a different pointer.
// Do not try to merge them.
return Some(self.new_opaque(to));

View file

@ -109,7 +109,7 @@ impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> {
}
// And finally, function pointer reification casts.
mir::Rvalue::Cast(
mir::CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer, _),
mir::CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer(_), _),
ref operand,
_,
) => {

View file

@ -1272,7 +1272,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
match kind {
// FIXME: Add Checks for these
CastKind::PointerWithExposedProvenance | CastKind::PointerExposeProvenance => {}
CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer, _) => {
CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer(_), _) => {
// FIXME: check signature compatibility.
check_kinds!(
op_ty,

View file

@ -765,7 +765,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> {
}
}
mir::Rvalue::Cast(
mir::CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer, _),
mir::CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer(_), _),
ref operand,
_,
) => {

View file

@ -978,7 +978,7 @@ pub enum Safety {
#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, Serialize)]
pub enum PointerCoercion {
/// Go from a fn-item type to a fn-pointer type.
ReifyFnPointer,
ReifyFnPointer(Safety),
/// Go from a safe fn pointer to an unsafe fn pointer.
UnsafeFnPointer,

View file

@ -130,7 +130,9 @@ impl<'tcx> Stable<'tcx> for ty::adjustment::PointerCoercion {
) -> Self::T {
use rustc_middle::ty::adjustment::PointerCoercion;
match self {
PointerCoercion::ReifyFnPointer => crate::mir::PointerCoercion::ReifyFnPointer,
PointerCoercion::ReifyFnPointer(safety) => {
crate::mir::PointerCoercion::ReifyFnPointer(safety.stable(tables, cx))
}
PointerCoercion::UnsafeFnPointer => crate::mir::PointerCoercion::UnsafeFnPointer,
PointerCoercion::ClosureFnPointer(safety) => {
crate::mir::PointerCoercion::ClosureFnPointer(safety.stable(tables, cx))

View file

@ -150,7 +150,7 @@ fn check_rvalue<'tcx>(
CastKind::PointerCoercion(
PointerCoercion::UnsafeFnPointer
| PointerCoercion::ClosureFnPointer(_)
| PointerCoercion::ReifyFnPointer,
| PointerCoercion::ReifyFnPointer(_),
_,
),
_,

View file

@ -1,12 +0,0 @@
//@ known-bug: #132765
trait LendingIterator {
type Item<'q>;
fn for_each(&self, _f: Box<fn(Self::Item<'_>)>) {}
}
fn f(_: ()) {}
fn main() {
LendingIterator::for_each(&(), f);
}

View file

@ -9,7 +9,7 @@ fn main() -> () {
bb0: {
StorageLive(_1);
_1 = foo as for<'a> fn(&'a (), &'a ()) (PointerCoercion(ReifyFnPointer, AsCast));
_1 = foo as for<'a> fn(&'a (), &'a ()) (PointerCoercion(ReifyFnPointer(Safe), AsCast));
FakeRead(ForLet(None), _1);
_0 = const ();
StorageDead(_1);

View file

@ -13,7 +13,7 @@
StorageLive(_1);
StorageLive(_2);
StorageLive(_3);
_3 = main as fn() (PointerCoercion(ReifyFnPointer, AsCast));
_3 = main as fn() (PointerCoercion(ReifyFnPointer(Safe), AsCast));
_2 = move _3 as usize (PointerExposeProvenance);
StorageDead(_3);
_1 = move _2 as *const fn() (PointerWithExposedProvenance);

View file

@ -3,7 +3,7 @@
fn main() {
// CHECK-LABEL: fn main(
// CHECK: [[ptr:_.*]] = main as fn() (PointerCoercion(ReifyFnPointer, AsCast));
// CHECK: [[ptr:_.*]] = main as fn() (PointerCoercion(ReifyFnPointer(Safe), AsCast));
// CHECK: [[addr:_.*]] = move [[ptr]] as usize (PointerExposeProvenance);
// CHECK: [[back:_.*]] = move [[addr]] as *const fn() (PointerWithExposedProvenance);
let _ = main as usize as *const fn();

View file

@ -37,7 +37,7 @@
bb0: {
- StorageLive(_1);
+ nop;
_1 = identity::<u8> as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer, AsCast));
_1 = identity::<u8> as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer(Safe), AsCast));
StorageLive(_2);
StorageLive(_3);
_3 = copy _1;
@ -50,7 +50,7 @@
StorageDead(_2);
- StorageLive(_4);
+ nop;
_4 = identity::<u8> as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer, AsCast));
_4 = identity::<u8> as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer(Safe), AsCast));
StorageLive(_5);
StorageLive(_6);
_6 = copy _4;

View file

@ -37,7 +37,7 @@
bb0: {
- StorageLive(_1);
+ nop;
_1 = identity::<u8> as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer, AsCast));
_1 = identity::<u8> as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer(Safe), AsCast));
StorageLive(_2);
StorageLive(_3);
_3 = copy _1;
@ -50,7 +50,7 @@
StorageDead(_2);
- StorageLive(_4);
+ nop;
_4 = identity::<u8> as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer, AsCast));
_4 = identity::<u8> as fn(u8) -> u8 (PointerCoercion(ReifyFnPointer(Safe), AsCast));
StorageLive(_5);
StorageLive(_6);
_6 = copy _4;

View file

@ -0,0 +1,38 @@
// Regression test for #132765
//
// We have two function parameters with types:
// - `&?0`
// - `Box<for<'a> fn(<?0 as Trait<'a>>::Item)>`
//
// As the alias in the second parameter has a `?0` it is an ambig
// alias, and as it references bound vars it can't be normalized to
// an infer var.
//
// When checking function arguments we try to coerce both:
// - `&()` to `&?0`
// - `FnDef(f)` to `Box<for<'a> fn(<?0 as Trait<'a>>::Item)>`
//
// The first coercion infers `?0=()`. Previously when handling
// the second coercion we wound *re-normalize* the alias, which
// now that `?0` has been inferred allowed us to determine this
// alias is not wellformed and normalize it to some infer var `?1`.
//
// We would then see that `FnDef(f)` can't be coerced to `Box<fn(?1)>`
// and return a `TypeError` referencing this new variable `?1`. This
// then caused ICEs as diagnostics would encounter inferences variables
// from the result of normalization inside of the probe used be coercion.
trait LendingIterator {
type Item<'q>;
fn for_each(&self, _f: Box<fn(Self::Item<'_>)>) {}
}
fn f(_: ()) {}
fn main() {
LendingIterator::for_each(&(), f);
//~^ ERROR: the trait bound `(): LendingIterator` is not satisfied
//~| ERROR: the trait bound `(): LendingIterator` is not satisfied
//~| ERROR: mismatched types
}

View file

@ -0,0 +1,46 @@
error[E0277]: the trait bound `(): LendingIterator` is not satisfied
--> $DIR/hr_alias_normalization_leaking_vars.rs:34:31
|
LL | LendingIterator::for_each(&(), f);
| ------------------------- ^^^ the trait `LendingIterator` is not implemented for `()`
| |
| required by a bound introduced by this call
|
help: this trait has no implementations, consider adding one
--> $DIR/hr_alias_normalization_leaking_vars.rs:26:1
|
LL | trait LendingIterator {
| ^^^^^^^^^^^^^^^^^^^^^
error[E0308]: mismatched types
--> $DIR/hr_alias_normalization_leaking_vars.rs:34:36
|
LL | LendingIterator::for_each(&(), f);
| ------------------------- ^ expected `Box<fn(...)>`, found fn item
| |
| arguments to this function are incorrect
|
= note: expected struct `Box<for<'a> fn(<() as LendingIterator>::Item<'a>)>`
found fn item `fn(()) {f}`
note: method defined here
--> $DIR/hr_alias_normalization_leaking_vars.rs:28:8
|
LL | fn for_each(&self, _f: Box<fn(Self::Item<'_>)>) {}
| ^^^^^^^^ ---------------------------
error[E0277]: the trait bound `(): LendingIterator` is not satisfied
--> $DIR/hr_alias_normalization_leaking_vars.rs:34:36
|
LL | LendingIterator::for_each(&(), f);
| ^ the trait `LendingIterator` is not implemented for `()`
|
help: this trait has no implementations, consider adding one
--> $DIR/hr_alias_normalization_leaking_vars.rs:26:1
|
LL | trait LendingIterator {
| ^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 3 previous errors
Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.

View file

@ -3,6 +3,9 @@
// behavior has been fixed.
//@ check-pass
//@ revisions: current next
//@ ignore-compare-mode-next-solver (explicit revisions)
//@[next] compile-flags: -Znext-solver
fn peculiar() -> impl Fn(u8) -> u8 {
return |x| x + 1

View file

@ -0,0 +1,30 @@
//@ check-pass
fn foo<T>() {}
fn fndef_lub_leak_check() {
macro_rules! lub {
($lhs:expr, $rhs:expr) => {
if true { $lhs } else { $rhs }
};
}
// Unused parameters on FnDefs are considered invariant
let lhs = foo::<for<'a> fn(&'static (), &'a ())>;
let rhs = foo::<for<'a> fn(&'a (), &'static ())>;
// If we leak check then we know we should coerce these
// to `fn()`, if we don't leak check we may try to keep
// them as `FnDef`s which would result in a borrowck
// error.
let lubbed = lub!(lhs, rhs);
// assert that we coerced lhs/rhs to a fn ptr
is_fnptr(lubbed);
}
trait FnPtr {}
impl FnPtr for fn() {}
fn is_fnptr<T: FnPtr>(_: T) {}
fn main() {}

View file

@ -0,0 +1,29 @@
//@ normalize-stderr: "32 bits" -> "64 bits"
fn foo<T>() {}
fn fndef_lub_leak_check() {
macro_rules! lub {
($lhs:expr, $rhs:expr) => {
if true { $lhs } else { $rhs }
};
}
// Unused parameters on FnDefs are considered invariant
let lhs = foo::<for<'a> fn(&'static (), &'a ())>;
let rhs = foo::<for<'a> fn(&'a (), &'static ())>;
loop {}
// If we leak check then we know we should coerce these
// to `fn()`, if we don't leak check we may try to keep
// them as `FnDef`s which would cause this code to compile
// as borrowck won't emit errors for deadcode.
let lubbed = lub!(lhs, rhs);
// assert that `lubbed` is a ZST/`FnDef`
unsafe { std::mem::transmute::<_, ()>(lubbed) }
//~^ ERROR: cannot transmute between types of different sizes
}
fn main() {}

View file

@ -0,0 +1,12 @@
error[E0512]: cannot transmute between types of different sizes, or dependently-sized types
--> $DIR/leak_check_fndef_lub_deadcode_breakage.rs:25:14
|
LL | unsafe { std::mem::transmute::<_, ()>(lubbed) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: source type: `fn()` (64 bits)
= note: target type: `()` (0 bits)
error: aborting due to 1 previous error
For more information about this error, try `rustc --explain E0512`.

View file

@ -0,0 +1,52 @@
//@ check-pass
//@ only-x86_64
// because target features
macro_rules! lub {
($lhs:expr, $rhs:expr) => {
if true { $lhs } else { $rhs }
};
}
fn safety_lub() {
unsafe fn lhs() {}
fn rhs() {}
// We have two different fn defs, the only valid lub here
// is to go to fnptrs. However, in order to go to fnptrs
// `rhs` must coerce from a *safe* function to an *unsafe*
// one.
let lubbed = lub!(lhs, rhs);
is_unsafe_fnptr(lubbed);
}
#[target_feature(enable = "sse2")]
fn target_feature_aware_safety_lub() {
#[target_feature(enable = "sse2")]
fn lhs() {}
fn rhs() {}
unsafe fn rhs_unsafe() {}
// We have two different fn defs, the only valid lub here
// is to go to fnptrs. However, in order to go to fnptrs
// `lhs` must coerce from an unsafe fn to a safe one due
// to the correct target features being enabled
let lubbed = lub!(lhs, rhs);
is_fnptr(lubbed);
// Similar case here except we must recognise that rhs
// is an unsafe fn so lhs must be an unsafe fn even though
// it *could* be safe
let lubbed = lub!(lhs, rhs_unsafe);
is_unsafe_fnptr(lubbed);
}
trait FnPtr {}
impl FnPtr for fn() {}
fn is_fnptr<T: FnPtr>(_: T) {}
trait UnsafeFnPtr {}
impl UnsafeFnPtr for unsafe fn() {}
fn is_unsafe_fnptr<T: UnsafeFnPtr>(_: T) {}
fn main() {}

View file

@ -0,0 +1,24 @@
//@ edition: 2024
// We avoid emitting reborrow coercions if it seems like it would
// not result in a different lifetime on the borrow. This can effect
// capture analysis resulting in borrow checking errors.
fn foo<'a>(b: &'a ()) -> impl Fn() {
|| {
expected::<&()>(b);
}
}
// No reborrow of `b` is emitted which means our closure captures
// `b` by ref resulting in an upvar of `&&'a ()`
fn bar<'a>(b: &'a ()) -> impl Fn() {
|| {
//~^ ERROR: closure may outlive the current function
expected::<&'a ()>(b);
}
}
fn expected<T>(_: T) {}
fn main() {}

View file

@ -0,0 +1,25 @@
error[E0373]: closure may outlive the current function, but it borrows `b`, which is owned by the current function
--> $DIR/structural_identity_dependent_reborrows.rs:16:5
|
LL | || {
| ^^ may outlive borrowed value `b`
LL |
LL | expected::<&'a ()>(b);
| - `b` is borrowed here
|
note: closure is returned here
--> $DIR/structural_identity_dependent_reborrows.rs:16:5
|
LL | / || {
LL | |
LL | | expected::<&'a ()>(b);
LL | | }
| |_____^
help: to force the closure to take ownership of `b` (and any other referenced variables), use the `move` keyword
|
LL | move || {
| ++++
error: aborting due to 1 previous error
For more information about this error, try `rustc --explain E0373`.