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

Make struct_tail normalize when possible #62585

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
6 changes: 3 additions & 3 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
return Ok(tcx.intern_layout(LayoutDetails::scalar(self, data_ptr)));
}

let unsized_part = tcx.struct_tail(pointee);
let unsized_part = tcx.struct_tail_erasing_lifetimes(pointee, param_env);
let metadata = match unsized_part.sty {
ty::Foreign(..) => {
return Ok(tcx.intern_layout(LayoutDetails::scalar(self, data_ptr)));
Expand Down Expand Up @@ -1664,7 +1664,7 @@ impl<'tcx> SizeSkeleton<'tcx> {
ty::Ref(_, pointee, _) |
ty::RawPtr(ty::TypeAndMut { ty: pointee, .. }) => {
let non_zero = !ty.is_unsafe_ptr();
let tail = tcx.struct_tail(pointee);
let tail = tcx.struct_tail_erasing_lifetimes(pointee, param_env);
match tail.sty {
ty::Param(_) | ty::Projection(_) => {
debug_assert!(tail.has_param_types() || tail.has_self_ty());
Expand Down Expand Up @@ -2015,7 +2015,7 @@ where
}));
}

match tcx.struct_tail(pointee).sty {
match tcx.struct_tail_erasing_lifetimes(pointee, cx.param_env()).sty {
ty::Slice(_) |
ty::Str => tcx.types.usize,
ty::Dynamic(_, _) => {
Expand Down
102 changes: 94 additions & 8 deletions src/librustc/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,46 @@ impl<'tcx> TyCtxt<'tcx> {
false
}

/// 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.
pub fn struct_tail(self, mut ty: Ty<'tcx>) -> Ty<'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.
///
/// Should only be called if `ty` has no inference variables and does not
/// need its lifetimes preserved (e.g. as part of codegen); otherwise
/// normalization attempt may cause compiler bugs.
pub fn struct_tail_erasing_lifetimes(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))
}

/// 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.
///
/// See also `struct_tail_erasing_lifetimes`, which is suitable for use
/// during codegen.
pub fn struct_tail_with_normalize(self,
mut ty: Ty<'tcx>,
normalize: impl Fn(Ty<'tcx>) -> Ty<'tcx>)
-> Ty<'tcx>
{
loop {
match ty.sty {
ty::Adt(def, substs) => {
Expand All @@ -282,6 +318,15 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

ty::Projection(_) | ty::Opaque(..) => {
let normalized = normalize(ty);
if ty == normalized {
return ty;
} else {
ty = normalized;
}
}

_ => {
break;
}
Expand All @@ -295,10 +340,35 @@ impl<'tcx> TyCtxt<'tcx> {
/// structure definitions.
/// For `(Foo<Foo<T>>, Foo<dyn Trait>)`, the result will be `(Foo<T>, Trait)`,
/// whereas struct_tail produces `T`, and `Trait`, respectively.
pub fn struct_lockstep_tails(self,
source: Ty<'tcx>,
target: Ty<'tcx>)
-> (Ty<'tcx>, Ty<'tcx>) {
///
/// Should only be called if the types have no inference variables and do
/// not need their lifetimes preserved (e.g. as part of codegen); otherwise
/// normalization attempt may cause compiler bugs.
pnkfelix marked this conversation as resolved.
Show resolved Hide resolved
pub fn struct_lockstep_tails_erasing_lifetimes(self,
source: Ty<'tcx>,
target: Ty<'tcx>,
param_env: ty::ParamEnv<'tcx>)
-> (Ty<'tcx>, Ty<'tcx>)
{
let tcx = self;
tcx.struct_lockstep_tails_with_normalize(
source, target, |ty| tcx.normalize_erasing_regions(param_env, ty))
}

/// Same as applying struct_tail on `source` and `target`, but only
/// keeps going as long as the two types are instances of the same
/// structure definitions.
/// For `(Foo<Foo<T>>, Foo<dyn Trait>)`, the result will be `(Foo<T>, Trait)`,
/// whereas struct_tail produces `T`, and `Trait`, respectively.
///
/// See also `struct_lockstep_tails_erasing_lifetimes`, which is suitable for use
/// during codegen.
pub fn struct_lockstep_tails_with_normalize(self,
source: Ty<'tcx>,
target: Ty<'tcx>,
normalize: impl Fn(Ty<'tcx>) -> Ty<'tcx>)
-> (Ty<'tcx>, Ty<'tcx>)
{
let (mut a, mut b) = (source, target);
loop {
match (&a.sty, &b.sty) {
Expand All @@ -320,6 +390,22 @@ impl<'tcx> TyCtxt<'tcx> {
break;
}
},
(ty::Projection(_), _) | (ty::Opaque(..), _) |
(_, ty::Projection(_)) | (_, ty::Opaque(..)) => {
// If either side is a projection, attempt to
// progress via normalization. (Should be safe to
// apply to both sides as normalization is
// idempotent.)
let a_norm = normalize(a);
let b_norm = normalize(b);
if a == a_norm && b == b_norm {
break;
} else {
a = a_norm;
b = b_norm;
}
}

_ => break,
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_codegen_ssa/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ pub fn unsized_info<'tcx, Cx: CodegenMethods<'tcx>>(
target: Ty<'tcx>,
old_info: Option<Cx::Value>,
) -> Cx::Value {
let (source, target) = cx.tcx().struct_lockstep_tails(source, target);
let (source, target) =
cx.tcx().struct_lockstep_tails_erasing_lifetimes(source, target, cx.param_env());
match (&source.sty, &target.sty) {
(&ty::Array(_, len), &ty::Slice(_)) => {
cx.const_usize(len.unwrap_usize(cx.tcx()))
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_codegen_ssa/traits/type_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,12 @@ pub trait DerivedTypeMethods<'tcx>: BaseTypeMethods<'tcx> + MiscMethods<'tcx> {
}

fn type_has_metadata(&self, ty: Ty<'tcx>) -> bool {
if ty.is_sized(self.tcx().at(DUMMY_SP), ty::ParamEnv::reveal_all()) {
let param_env = ty::ParamEnv::reveal_all();
if ty.is_sized(self.tcx().at(DUMMY_SP), param_env) {
return false;
}

let tail = self.tcx().struct_tail(ty);
let tail = self.tcx().struct_tail_erasing_lifetimes(ty, param_env);
match tail.sty {
ty::Foreign(..) => false,
ty::Str | ty::Slice(..) | ty::Dynamic(..) => true,
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
dty: Ty<'tcx>,
) -> InterpResult<'tcx> {
// A<Struct> -> A<Trait> conversion
let (src_pointee_ty, dest_pointee_ty) = self.tcx.struct_lockstep_tails(sty, dty);
let (src_pointee_ty, dest_pointee_ty) =
self.tcx.struct_lockstep_tails_erasing_lifetimes(sty, dty, self.param_env);

match (&src_pointee_ty.sty, &dest_pointee_ty.sty) {
(&ty::Array(_, length), &ty::Slice(_)) => {
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_mir/interpret/intern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ for
let value = self.ecx.read_immediate(mplace.into())?;
// Handle trait object vtables
if let Ok(meta) = value.to_meta() {
if let ty::Dynamic(..) = self.ecx.tcx.struct_tail(referenced_ty).sty {
if let ty::Dynamic(..) =
self.ecx.tcx.struct_tail_erasing_lifetimes(referenced_ty, self.param_env).sty
{
if let Ok(vtable) = meta.unwrap().to_ptr() {
// explitly choose `Immutable` here, since vtables are immutable, even
// if the reference of the fat pointer is mutable
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,8 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
"uninitialized data in fat pointer metadata", self.path);
let layout = self.ecx.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?;
if layout.is_unsized() {
let tail = self.ecx.tcx.struct_tail(layout.ty);
let tail = self.ecx.tcx.struct_tail_erasing_lifetimes(layout.ty,
self.ecx.param_env);
match tail.sty {
ty::Dynamic(..) => {
let vtable = meta.unwrap();
Expand Down
7 changes: 4 additions & 3 deletions src/librustc_mir/monomorphize/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -851,12 +851,13 @@ fn find_vtable_types_for_unsizing<'tcx>(
target_ty: Ty<'tcx>,
) -> (Ty<'tcx>, Ty<'tcx>) {
let ptr_vtable = |inner_source: Ty<'tcx>, inner_target: Ty<'tcx>| {
let param_env = ty::ParamEnv::reveal_all();
let type_has_metadata = |ty: Ty<'tcx>| -> bool {
use syntax_pos::DUMMY_SP;
if ty.is_sized(tcx.at(DUMMY_SP), ty::ParamEnv::reveal_all()) {
if ty.is_sized(tcx.at(DUMMY_SP), param_env) {
return false;
}
let tail = tcx.struct_tail(ty);
let tail = tcx.struct_tail_erasing_lifetimes(ty, param_env);
match tail.sty {
ty::Foreign(..) => false,
ty::Str | ty::Slice(..) | ty::Dynamic(..) => true,
Expand All @@ -866,7 +867,7 @@ fn find_vtable_types_for_unsizing<'tcx>(
if type_has_metadata(inner_source) {
(inner_source, inner_target)
} else {
tcx.struct_lockstep_tails(inner_source, inner_target)
tcx.struct_lockstep_tails_erasing_lifetimes(inner_source, inner_target, param_env)
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ impl<'a, 'tcx> Expectation<'tcx> {
/// See the test case `test/run-pass/coerce-expect-unsized.rs` and #20169
/// for examples of where this comes up,.
fn rvalue_hint(fcx: &FnCtxt<'a, 'tcx>, ty: Ty<'tcx>) -> Expectation<'tcx> {
match fcx.tcx.struct_tail(ty).sty {
match fcx.tcx.struct_tail_without_normalization(ty).sty {
ty::Slice(_) | ty::Str | ty::Dynamic(..) => {
ExpectRvalueLikeUnsized(ty)
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,8 @@ fn check_item_type(

let mut forbid_unsized = true;
if allow_foreign_ty {
if let ty::Foreign(_) = fcx.tcx.struct_tail(item_ty).sty {
let tail = fcx.tcx.struct_tail_erasing_lifetimes(item_ty, fcx.param_env);
if let ty::Foreign(_) = tail.sty {
forbid_unsized = false;
}
}
Expand Down
35 changes: 35 additions & 0 deletions src/test/ui/layout/issue-60431-unsized-tail-behind-projection.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// rust-lang/rust#60431: This is a scenario where to determine the size of
// `&Ref<Obstack>`, we need to know the concrete type of the last field in
// `Ref<Obstack>` (i.e. its "struct tail"), and determining that concrete type
// requires normalizing `Obstack::Dyn`.
//
// The old "struct tail" computation did not perform such normalization, and so
// the compiler would ICE when trying to figure out if `Ref<Obstack>` is a
// dynamically-sized type (DST).

// run-pass

use std::mem;

pub trait Arena {
type Dyn : ?Sized;
}

pub struct DynRef {
_dummy: [()],
}

pub struct Ref<A: Arena> {
_value: u8,
_dyn_arena: A::Dyn,
}

pub struct Obstack;

impl Arena for Obstack {
type Dyn = DynRef;
}

fn main() {
assert_eq!(mem::size_of::<&Ref<Obstack>>(), mem::size_of::<&[()]>());
}