Skip to content
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

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

Merged
merged 3 commits into from
Aug 13, 2024
Merged
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
47 changes: 47 additions & 0 deletions compiler/rustc_borrowck/src/type_check/canonical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down
13 changes: 2 additions & 11 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
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
Expand Up @@ -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 {}",
Expand Down
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
Expand Up @@ -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,
|| {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_hir_typeck/src/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_hir_typeck/src/expectation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand Down
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
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(_))
Expand All @@ -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),

Expand Down
22 changes: 8 additions & 14 deletions compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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>,
Expand Down Expand Up @@ -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)
})
}
Expand All @@ -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>,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_ty_utils/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
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
Expand Down
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.
Expand Down
Loading