Skip to content

Normalize struct tail properly for dyn ptr-to-ptr casting in new solver #128712

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

Merged
merged 3 commits into from
Aug 13, 2024
Merged
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
47 changes: 47 additions & 0 deletions compiler/rustc_borrowck/src/type_check/canonical.rs
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ use rustc_middle::mir::ConstraintCategory;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, Upcast};
use rustc_span::def_id::DefId;
use rustc_span::Span;
use rustc_trait_selection::traits::query::type_op::custom::CustomTypeOp;
use rustc_trait_selection::traits::query::type_op::{self, TypeOpOutput};
use rustc_trait_selection::traits::ObligationCause;

@@ -165,6 +166,52 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
result.unwrap_or(value)
}

#[instrument(skip(self), level = "debug")]
pub(super) fn struct_tail(
&mut self,
ty: Ty<'tcx>,
location: impl NormalizeLocation,
) -> Ty<'tcx> {
let tcx = self.tcx();
if self.infcx.next_trait_solver() {
let body = self.body;
let param_env = self.param_env;
self.fully_perform_op(
Copy link
Member Author

Choose a reason for hiding this comment

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

This is incredibly gross, to the point that it kinda feels unnecessary? 🤔 But I really don't know how to simplify it, given the way that we need to wrap this in a type op and struct_tail doesn't have a fallible normalize fn...

location.to_locations(),
ConstraintCategory::Boring,
CustomTypeOp::new(
|ocx| {
let structurally_normalize = |ty| {
ocx.structurally_normalize(
&ObligationCause::misc(
location.to_locations().span(body),
body.source.def_id().expect_local(),
),
param_env,
ty,
)
.unwrap_or_else(|_| bug!("struct tail should have been computable, since we computed it in HIR"))
};

let tail = tcx.struct_tail_raw(
ty,
structurally_normalize,
|| {},
);

Ok(tail)
},
"normalizing struct tail",
),
)
.unwrap_or_else(|guar| Ty::new_error(tcx, guar))
} else {
let mut normalize = |ty| self.normalize(ty, location);
let tail = tcx.struct_tail_raw(ty, &mut normalize, || {});
normalize(tail)
}
}

#[instrument(skip(self), level = "debug")]
pub(super) fn ascribe_user_type(
&mut self,
13 changes: 2 additions & 11 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
@@ -2329,17 +2329,8 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
let cast_ty_to = CastTy::from_ty(*ty);
match (cast_ty_from, cast_ty_to) {
(Some(CastTy::Ptr(src)), Some(CastTy::Ptr(dst))) => {
let mut normalize = |t| self.normalize(t, location);

// N.B. `struct_tail_with_normalize` only "structurally resolves"
// the type. It is not fully normalized, so we have to normalize it
// afterwards.
let src_tail =
tcx.struct_tail_with_normalize(src.ty, &mut normalize, || ());
let src_tail = normalize(src_tail);
let dst_tail =
tcx.struct_tail_with_normalize(dst.ty, &mut normalize, || ());
let dst_tail = normalize(dst_tail);
let src_tail = self.struct_tail(src.ty, location);
let dst_tail = self.struct_tail(dst.ty, location);

// This checks (lifetime part of) vtable validity for pointer casts,
// which is irrelevant when there are aren't principal traits on both sides (aka only auto traits).
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
@@ -226,7 +226,7 @@ pub(super) fn op_to_const<'tcx>(
let pointee_ty = imm.layout.ty.builtin_deref(false).unwrap(); // `false` = no raw ptrs
debug_assert!(
matches!(
ecx.tcx.struct_tail_without_normalization(pointee_ty).kind(),
ecx.tcx.struct_tail_for_codegen(pointee_ty, ecx.param_env).kind(),
Copy link
Member Author

Choose a reason for hiding this comment

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

Can't actually create a test for this, since we only allow matching on arrays and str (not tails), and I don't know how to trigger this outside of MIR match lowering.

ty::Str | ty::Slice(..),
),
"`ConstValue::Slice` is for slice-tailed types only, but got {}",
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/valtrees.rs
Original file line number Diff line number Diff line change
@@ -195,7 +195,7 @@ fn reconstruct_place_meta<'tcx>(

let mut last_valtree = valtree;
// Traverse the type, and update `last_valtree` as we go.
let tail = tcx.struct_tail_with_normalize(
let tail = tcx.struct_tail_raw(
layout.ty,
|ty| ty,
|| {
2 changes: 2 additions & 0 deletions compiler/rustc_hir_typeck/src/cast.rs
Original file line number Diff line number Diff line change
@@ -97,6 +97,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return Ok(Some(PointerKind::Thin));
}

let t = self.try_structurally_resolve_type(span, t);

Ok(match *t.kind() {
ty::Slice(_) | ty::Str => Some(PointerKind::Length),
ty::Dynamic(tty, _, ty::Dyn) => Some(PointerKind::VTable(tty)),
3 changes: 2 additions & 1 deletion compiler/rustc_hir_typeck/src/expectation.rs
Original file line number Diff line number Diff line change
@@ -70,7 +70,8 @@ impl<'a, 'tcx> Expectation<'tcx> {
/// See the test case `test/ui/coerce-expect-unsized.rs` and #20169
/// for examples of where this comes up,.
pub(super) fn rvalue_hint(fcx: &FnCtxt<'a, 'tcx>, ty: Ty<'tcx>) -> Expectation<'tcx> {
match fcx.tcx.struct_tail_without_normalization(ty).kind() {
// FIXME: This is not right, even in the old solver...
match fcx.tcx.struct_tail_raw(ty, |ty| ty, || {}).kind() {
ty::Slice(_) | ty::Str | ty::Dynamic(..) => ExpectRvalueLikeUnsized(ty),
_ => ExpectHasType(ty),
}
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
@@ -404,7 +404,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
code: traits::ObligationCauseCode<'tcx>,
) {
if !ty.references_error() {
let tail = self.tcx.struct_tail_with_normalize(
let tail = self.tcx.struct_tail_raw(
ty,
|ty| {
if self.next_trait_solver() {
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
@@ -362,7 +362,7 @@ impl<'tcx> SizeSkeleton<'tcx> {
ty::Ref(_, pointee, _) | ty::RawPtr(pointee, _) => {
let non_zero = !ty.is_unsafe_ptr();

let tail = tcx.struct_tail_with_normalize(
let tail = tcx.struct_tail_raw(
pointee,
|ty| match tcx.try_normalize_erasing_regions(param_env, ty) {
Ok(ty) => ty,
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
@@ -1590,7 +1590,7 @@ impl<'tcx> Ty<'tcx> {
tcx: TyCtxt<'tcx>,
normalize: impl FnMut(Ty<'tcx>) -> Ty<'tcx>,
) -> Result<Ty<'tcx>, Ty<'tcx>> {
let tail = tcx.struct_tail_with_normalize(self, normalize, || {});
let tail = tcx.struct_tail_raw(self, normalize, || {});
match tail.kind() {
// Sized types
ty::Infer(ty::IntVar(_) | ty::FloatVar(_))
@@ -1614,10 +1614,10 @@ impl<'tcx> Ty<'tcx> {
| ty::Foreign(..)
// `dyn*` has metadata = ().
| ty::Dynamic(_, _, ty::DynStar)
// If returned by `struct_tail_with_normalize` this is a unit struct
// If returned by `struct_tail_raw` this is a unit struct
// without any fields, or not a struct, and therefore is Sized.
| ty::Adt(..)
// If returned by `struct_tail_with_normalize` this is the empty tuple,
// If returned by `struct_tail_raw` this is the empty tuple,
// a.k.a. unit type, which is Sized
| ty::Tuple(..) => Ok(tcx.types.unit),

22 changes: 8 additions & 14 deletions compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
@@ -171,14 +171,6 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

/// Attempts to returns the deeply last field of nested structures, but
/// does not apply any normalization in its search. Returns the same type
/// if input `ty` is not a structure at all.
pub fn struct_tail_without_normalization(self, ty: Ty<'tcx>) -> Ty<'tcx> {
let tcx = self;
tcx.struct_tail_with_normalize(ty, |ty| ty, || {})
}

/// Returns the deeply last field of nested structures, or the same type if
/// not a structure at all. Corresponds to the only possible unsized field,
/// and its type can be used to determine unsizing strategy.
@@ -188,20 +180,22 @@ impl<'tcx> TyCtxt<'tcx> {
/// normalization attempt may cause compiler bugs.
pub fn struct_tail_for_codegen(self, ty: Ty<'tcx>, param_env: ty::ParamEnv<'tcx>) -> Ty<'tcx> {
let tcx = self;
tcx.struct_tail_with_normalize(ty, |ty| tcx.normalize_erasing_regions(param_env, ty), || {})
tcx.struct_tail_raw(ty, |ty| tcx.normalize_erasing_regions(param_env, ty), || {})
}

/// Returns the deeply last field of nested structures, or the same type if
/// not a structure at all. Corresponds to the only possible unsized field,
/// and its type can be used to determine unsizing strategy.
///
/// This is parameterized over the normalization strategy (i.e. how to
/// handle `<T as Trait>::Assoc` and `impl Trait`); pass the identity
/// function to indicate no normalization should take place.
/// handle `<T as Trait>::Assoc` and `impl Trait`). You almost certainly do
/// **NOT** want to pass the identity function here, unless you know what
/// you're doing, or you're within normalization code itself and will handle
/// an unnormalized tail recursively.
///
/// See also `struct_tail_for_codegen`, which is suitable for use
/// during codegen.
pub fn struct_tail_with_normalize(
pub fn struct_tail_raw(
self,
mut ty: Ty<'tcx>,
mut normalize: impl FnMut(Ty<'tcx>) -> Ty<'tcx>,
@@ -281,7 +275,7 @@ impl<'tcx> TyCtxt<'tcx> {
param_env: ty::ParamEnv<'tcx>,
) -> (Ty<'tcx>, Ty<'tcx>) {
let tcx = self;
tcx.struct_lockstep_tails_with_normalize(source, target, |ty| {
tcx.struct_lockstep_tails_raw(source, target, |ty| {
tcx.normalize_erasing_regions(param_env, ty)
})
}
@@ -294,7 +288,7 @@ impl<'tcx> TyCtxt<'tcx> {
///
/// See also `struct_lockstep_tails_for_codegen`, which is suitable for use
/// during codegen.
pub fn struct_lockstep_tails_with_normalize(
pub fn struct_lockstep_tails_raw(
self,
source: Ty<'tcx>,
target: Ty<'tcx>,
6 changes: 3 additions & 3 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
@@ -1110,7 +1110,7 @@ fn assemble_candidates_from_impls<'cx, 'tcx>(
| ty::Error(_) => false,
}
} else if tcx.is_lang_item(trait_ref.def_id, LangItem::PointeeTrait) {
let tail = selcx.tcx().struct_tail_with_normalize(
let tail = selcx.tcx().struct_tail_raw(
self_ty,
|ty| {
// We throw away any obligations we get from this, since we normalize
@@ -1149,10 +1149,10 @@ fn assemble_candidates_from_impls<'cx, 'tcx>(
| ty::Never
// Extern types have unit metadata, according to RFC 2850
| ty::Foreign(_)
// If returned by `struct_tail_without_normalization` this is a unit struct
// If returned by `struct_tail` this is a unit struct
// without any fields, or not a struct, and therefore is Sized.
| ty::Adt(..)
// If returned by `struct_tail_without_normalization` this is the empty tuple.
// If returned by `struct_tail` this is the empty tuple.
| ty::Tuple(..)
// Integers and floats are always Sized, and so have unit type metadata.
| ty::Infer(ty::InferTy::IntVar(_) | ty::InferTy::FloatVar(..)) => true,
6 changes: 5 additions & 1 deletion compiler/rustc_ty_utils/src/layout.rs
Original file line number Diff line number Diff line change
@@ -219,9 +219,13 @@ fn layout_of_uncached<'tcx>(
// its struct tail cannot be normalized either, so try to get a
// more descriptive layout error here, which will lead to less confusing
// diagnostics.
//
// We use the raw struct tail function here to get the first tail
// that is an alias, which is likely the cause of the normalization
// error.
match tcx.try_normalize_erasing_regions(
param_env,
tcx.struct_tail_without_normalization(pointee),
tcx.struct_tail_raw(pointee, |ty| ty, || {}),
) {
Ok(_) => {}
Err(better_err) => {
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: lifetime may not live long enough
--> $DIR/ptr-to-trait-obj-different-regions-id-trait.rs:21:17
--> $DIR/ptr-to-trait-obj-different-regions-id-trait.rs:24:17
|
LL | fn m<'a>() {
| -- lifetime `'a` defined here
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: lifetime may not live long enough
--> $DIR/ptr-to-trait-obj-different-regions-id-trait.rs:24:17
|
LL | fn m<'a>() {
| -- lifetime `'a` defined here
LL | let unsend: *const dyn Cat<'a> = &();
LL | let _send = unsend as *const S<dyn Cat<'static>>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type annotation requires that `'a` must outlive `'static`
|
= note: requirement occurs because of the type `S<dyn Cat<'_>>`, which makes the generic argument `dyn Cat<'_>` invariant
= note: the struct `S<T>` is invariant over the parameter `T`
= help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance

error: aborting due to 1 previous error

3 changes: 3 additions & 0 deletions tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
//@ revisions: current next
//@ ignore-compare-mode-next-solver (explicit revisions)
//@[next] compile-flags: -Znext-solver
//@ check-fail
//
// Make sure we can't trick the compiler by using a projection.