Skip to content

Move coercion hack from coerce_unsized to check_cast #138542

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 34 additions & 23 deletions compiler/rustc_hir_typeck/src/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,33 +705,44 @@ impl<'a, 'tcx> CastCheck<'tcx> {
self.report_cast_to_unsized_type(fcx);
} else if self.expr_ty.references_error() || self.cast_ty.references_error() {
// No sense in giving duplicate error messages
} else if self.expr_ty.is_raw_ptr() && self.cast_ty.is_raw_ptr() {
// HACK: We check `may_coerce` first to ensure that the unsizing is
// worth committing to. The weird thing is that `coerce_unsized` is
// somewhat unreliable; it commits to coercions that are doomed to
// fail like `*const W<dyn Trait> -> *const dyn Trait` even though
// the pointee is unsized. Here, we want to fall back to a ptr-ptr
// cast, so we first check `may_coerce` which also checks that all
// of the nested obligations hold first, *then* only commit to the
// coercion cast if definitely holds.
if fcx.may_coerce(self.expr_ty, self.cast_ty) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels iffy to me 🤔

both because this is the first use of may_coerce in the happy path and because the code layout seems a bit odd.

If we were to keep this, please move the may_coerce into try_coercion_cast 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaict we call try_coercion_cast before do_check to lint trivial casts 🤔

What would break if we just always use do_check for raw pointers and never attempt to coerce them?

Copy link
Member Author

@compiler-errors compiler-errors Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we were to keep this, please move the may_coerce into try_coercion_cast

We can't do that. That would negatively affect diagnostics in other coercion casts (e.g. for refs) which currently rely on coercion bubbling up a more specific error. We intentionally only want to check may_coerce when casting only raw pointers, which is the point of this hack in the first place.

The purpose of calling may_coerce is to check that the nested obligations may hold. This could be written more explicitly, as a probe + coerce + check nested obligations, but it's code duplication.

What would break if we just always use do_check for raw pointers and never attempt to coerce them?

All casts that don't involve identical fat pointees. Like, *mut dyn Trait + 'long -> *mut dyn Trait + 'short relies on coercion. We also allow *mut dyn Trait + Send -> *mut dyn Trait, and raw pointer upcasting.

self.try_coercion_cast(fcx).expect("`may_coerce` should imply types can coerce");
// When casting a raw pointer to another raw pointer, we cannot convert the cast into
// a coercion because the pointee types might only differ in regions, which HIR typeck
// cannot distinguish. This would cause us to erroneously discard a cast which will
// lead to a borrowck error like #113257.
// We still did a coercion above to unify inference variables for `ptr as _` casts.
// This does cause us to miss some trivial casts in the trivial cast lint.
} else {
match self.do_check(fcx) {
Ok(k) => {
debug!(" -> {:?}", k);
}
Err(e) => self.report_cast_error(fcx, e),
}
}
} else {
match self.try_coercion_cast(fcx) {
Ok(()) => {
if self.expr_ty.is_raw_ptr() && self.cast_ty.is_raw_ptr() {
// When casting a raw pointer to another raw pointer, we cannot convert the cast into
// a coercion because the pointee types might only differ in regions, which HIR typeck
// cannot distinguish. This would cause us to erroneously discard a cast which will
// lead to a borrowck error like #113257.
// We still did a coercion above to unify inference variables for `ptr as _` casts.
// This does cause us to miss some trivial casts in the trivial cast lint.
debug!(" -> PointerCast");
} else {
self.trivial_cast_lint(fcx);
debug!(" -> CoercionCast");
fcx.typeck_results
.borrow_mut()
.set_coercion_cast(self.expr.hir_id.local_id);
}
}
Err(_) => {
match self.do_check(fcx) {
Ok(k) => {
debug!(" -> {:?}", k);
}
Err(e) => self.report_cast_error(fcx, e),
};
self.trivial_cast_lint(fcx);
debug!(" -> CoercionCast");
fcx.typeck_results.borrow_mut().set_coercion_cast(self.expr.hir_id.local_id);
}
Err(_) => match self.do_check(fcx) {
Ok(k) => {
debug!(" -> {:?}", k);
}
Err(e) => self.report_cast_error(fcx, e),
},
};
}
}
Expand Down
71 changes: 8 additions & 63 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use rustc_attr_parsing::InlineAttr;
use rustc_errors::codes::*;
use rustc_errors::{Applicability, Diag, struct_span_code_err};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::{self as hir, LangItem};
use rustc_hir::{self as hir};
use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer;
use rustc_infer::infer::relate::RelateResult;
use rustc_infer::infer::{Coercion, DefineOpaqueTypes, InferOk, InferResult};
Expand All @@ -55,7 +55,7 @@ use rustc_middle::ty::adjustment::{
Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCoercion,
};
use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::{self, AliasTy, GenericArgsRef, Ty, TyCtxt, TypeVisitableExt};
use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypeVisitableExt};
use rustc_span::{BytePos, DUMMY_SP, DesugaringKind, Span};
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
Expand Down Expand Up @@ -599,57 +599,6 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
ty::TraitRef::new(self.tcx, coerce_unsized_did, [coerce_source, coerce_target]),
);

// If the root `Source: CoerceUnsized<Target>` obligation can't possibly hold,
// we don't have to assume that this is unsizing coercion (it will always lead to an error)
//
// However, we don't want to bail early all the time, since the unholdable obligations
// may be interesting for diagnostics (such as trying to coerce `&T` to `&dyn Id<This = U>`),
// so we only bail if there (likely) is another way to convert the types.
if !self.infcx.predicate_may_hold(&root_obligation) {
if let Some(dyn_metadata_adt_def_id) = self.tcx.lang_items().get(LangItem::DynMetadata)
&& let Some(metadata_type_def_id) = self.tcx.lang_items().get(LangItem::Metadata)
{
self.probe(|_| {
let ocx = ObligationCtxt::new(&self.infcx);

// returns `true` if `<ty as Pointee>::Metadata` is `DynMetadata<_>`
let has_dyn_trait_metadata = |ty| {
let metadata_ty: Result<_, _> = ocx.structurally_normalize_ty(
&ObligationCause::dummy(),
self.fcx.param_env,
Ty::new_alias(
self.tcx,
ty::AliasTyKind::Projection,
AliasTy::new(self.tcx, metadata_type_def_id, [ty]),
),
);

metadata_ty.is_ok_and(|metadata_ty| {
metadata_ty
.ty_adt_def()
.is_some_and(|d| d.did() == dyn_metadata_adt_def_id)
})
};

// If both types are raw pointers to a (wrapper over a) trait object,
// this might be a cast like `*const W<dyn Trait> -> *const dyn Trait`.
// So it's better to bail and try that. (even if the cast is not possible, for
// example due to vtables not matching, cast diagnostic will likely still be better)
//
// N.B. use `target`, not `coerce_target` (the latter is a var)
if let &ty::RawPtr(source_pointee, _) = coerce_source.kind()
&& let &ty::RawPtr(target_pointee, _) = target.kind()
&& has_dyn_trait_metadata(source_pointee)
&& has_dyn_trait_metadata(target_pointee)
{
return Err(TypeError::Mismatch);
}

Ok(())
})?;
}
}

// Use a FIFO queue for this custom fulfillment procedure.
//
// A Vec (or SmallVec) is not a natural choice for a queue. However,
Expand Down Expand Up @@ -725,16 +674,12 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
}

// Dyn-compatibility violations or miscellaneous.
Err(err) => {
let guar = self.err_ctxt().report_selection_error(
obligation.clone(),
&obligation,
&err,
);
self.fcx.set_tainted_by_errors(guar);
// Treat this like an obligation and follow through
// with the unsizing - the lack of a coercion should
// be silent, as it causes a type mismatch later.
Err(_) => {
// Previously we reported an error here; instead of doing that,
// we just register the failing obligation which will be reported
// in the outer selection loop. This prevents reporting errors
// in places like `may_coerce` calls.
coercion.obligations.push(obligation);
}

Ok(Some(impl_source)) => queue.extend(impl_source.nested_obligations()),
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/issues/issue-22034.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ fn main() {
let ptr: *mut () = core::ptr::null_mut();
let _: &mut dyn Fn() = unsafe {
&mut *(ptr as *mut dyn Fn())
//~^ ERROR expected a `Fn()` closure, found `()`
//~^ ERROR cannot cast thin pointer `*mut ()` to wide pointer `*mut dyn Fn()`
};
}
12 changes: 4 additions & 8 deletions tests/ui/issues/issue-22034.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
error[E0277]: expected a `Fn()` closure, found `()`
--> $DIR/issue-22034.rs:4:16
error[E0607]: cannot cast thin pointer `*mut ()` to wide pointer `*mut dyn Fn()`
--> $DIR/issue-22034.rs:4:15
|
LL | &mut *(ptr as *mut dyn Fn())
| ^^^ expected an `Fn()` closure, found `()`
|
= help: the trait `Fn()` is not implemented for `()`
= note: wrap the `()` in a closure with no arguments: `|| { /* code */ }`
= note: required for the cast from `*mut ()` to `*mut dyn Fn()`
| ^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.
For more information about this error, try `rustc --explain E0607`.
4 changes: 2 additions & 2 deletions tests/ui/mismatched_types/cast-rfc0401.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ fn main()

let _ = 42usize as *const [u8]; //~ ERROR cannot cast `usize` to a pointer that is wide
let _ = v as *const [u8]; //~ ERROR cannot cast
let _ = fat_v as *const dyn Foo; //~ ERROR the size for values of type
let _ = fat_v as *const dyn Foo; //~ ERROR casting `*const [u8]` as `*const dyn Foo` is invalid
let _ = foo as *const str; //~ ERROR is invalid
let _ = foo as *mut str; //~ ERROR is invalid
let _ = main as *mut str; //~ ERROR is invalid
Expand All @@ -59,7 +59,7 @@ fn main()
let _ = fat_sv as usize; //~ ERROR is invalid

let a : *const str = "hello";
let _ = a as *const dyn Foo; //~ ERROR the size for values of type
let _ = a as *const dyn Foo; //~ ERROR casting `*const str` as `*const dyn Foo` is invalid

// check no error cascade
let _ = main.f as *const u32; //~ ERROR no field
Expand Down
36 changes: 17 additions & 19 deletions tests/ui/mismatched_types/cast-rfc0401.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,14 @@ error[E0607]: cannot cast thin pointer `*const u8` to wide pointer `*const [u8]`
LL | let _ = v as *const [u8];
| ^^^^^^^^^^^^^^^^

error[E0606]: casting `*const [u8]` as `*const dyn Foo` is invalid
--> $DIR/cast-rfc0401.rs:53:13
|
LL | let _ = fat_v as *const dyn Foo;
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the pointers have different metadata

error[E0606]: casting `&dyn Foo` as `*const str` is invalid
--> $DIR/cast-rfc0401.rs:54:13
|
Expand Down Expand Up @@ -203,6 +211,14 @@ LL | let _ = fat_sv as usize;
|
= help: cast through a thin pointer first

error[E0606]: casting `*const str` as `*const dyn Foo` is invalid
--> $DIR/cast-rfc0401.rs:62:13
|
LL | let _ = a as *const dyn Foo;
| ^^^^^^^^^^^^^^^^^^^
|
= note: the pointers have different metadata

error[E0606]: casting `*const dyn Foo` as `*const [u16]` is invalid
--> $DIR/cast-rfc0401.rs:68:13
|
Expand All @@ -219,24 +235,6 @@ LL | let _ = cf as *const dyn Bar;
|
= note: the trait objects may have different vtables

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
--> $DIR/cast-rfc0401.rs:53:13
|
LL | let _ = fat_v as *const dyn Foo;
| ^^^^^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `[u8]`
= note: required for the cast from `*const [u8]` to `*const dyn Foo`

error[E0277]: the size for values of type `str` cannot be known at compilation time
--> $DIR/cast-rfc0401.rs:62:13
|
LL | let _ = a as *const dyn Foo;
| ^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `str`
= note: required for the cast from `*const str` to `*const dyn Foo`

error[E0606]: casting `&{float}` as `f32` is invalid
--> $DIR/cast-rfc0401.rs:71:30
|
Expand All @@ -250,5 +248,5 @@ LL | vec![0.0].iter().map(|s| *s as f32).collect::<Vec<f32>>();

error: aborting due to 34 previous errors

Some errors have detailed explanations: E0054, E0277, E0604, E0605, E0606, E0607, E0609.
Some errors have detailed explanations: E0054, E0604, E0605, E0606, E0607, E0609.
For more information about an error, try `rustc --explain E0054`.
Loading