about summary refs log tree commit diff
path: root/src
diff options
context:
space:
mode:
authorMazdak Farrokhzad <twingoow@gmail.com>2019-07-13 16:18:37 +0200
committerGitHub <noreply@github.com>2019-07-13 16:18:37 +0200
commit833dada10632814723426f07c53c9913dfec50c3 (patch)
tree8f9aef6b3d38de444e6a3bdd93ccce8898b8f328 /src
parent5e1891c47407bef2e2cc107ed997d557e57edaef (diff)
parent3c8279a2246c8bbb63f7d6aeac144b3a8adbe755 (diff)
downloadrust-833dada10632814723426f07c53c9913dfec50c3.tar.gz
rust-833dada10632814723426f07c53c9913dfec50c3.zip
Rollup merge of #62585 - pnkfelix:issue-60431-make-struct-tail-normalize-when-possible, r=eddyb
Make struct_tail normalize when possible

As noted in commit message: this replaces the existing methods to extract the struct tail(s) with new entry points that make the handling of normalization explicit.

Most of the places that call `struct_tail` are during codegen, post type-checking, and therefore they can get away with using `tcx.normalize_erasing_regions` (this is the entry point `struct_tail_erasing_lifetimes`)

For other cases that may arise, one can use the core method, which is parameterized over the normalization `Ty -> Ty` closure (`struct_tail_with_normalize`).

Or one can use the trivial entry point that does not normalization (`struct_tail_without_normalization`)

----

I spent a little while trying to make a test that exposed the bug via `impl Trait` rather than a projection, but I failed to find something that tripped up the current nightly `rustc`.
 * I have *not* spent any time trying to make tests that trip up the other places where `struct_tail` was previously being called. While I do think the task of making such tests could be worthwhile, I am simply running out of time. (Its also possible that the layout code is always the first point called, and thus it may be pointless to try to come up with such tests.)

I also spent a little time discussing with @eddyb where this code should live. They suggested moving `struct_tail` and its sibling `struct_lockstep_tails` to the `LayoutCx`.  But in the interest of time, I have left that refactoring (which may be questionable at this point) to a follow-up task.

----

Fix #60431
Diffstat (limited to 'src')
-rw-r--r--src/librustc/ty/layout.rs6
-rw-r--r--src/librustc/ty/util.rs102
-rw-r--r--src/librustc_codegen_ssa/base.rs3
-rw-r--r--src/librustc_codegen_ssa/traits/type_.rs5
-rw-r--r--src/librustc_mir/interpret/cast.rs3
-rw-r--r--src/librustc_mir/interpret/intern.rs4
-rw-r--r--src/librustc_mir/interpret/validity.rs3
-rw-r--r--src/librustc_mir/monomorphize/collector.rs7
-rw-r--r--src/librustc_typeck/check/mod.rs2
-rw-r--r--src/librustc_typeck/check/wfcheck.rs3
-rw-r--r--src/test/ui/layout/issue-60431-unsized-tail-behind-projection.rs35
11 files changed, 151 insertions, 22 deletions
diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs
index 4af26e19b37..4ed52a1e966 100644
--- a/src/librustc/ty/layout.rs
+++ b/src/librustc/ty/layout.rs
@@ -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)));
@@ -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());
@@ -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(_, _) => {
diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs
index 56cb89b5144..c3ecc04b12e 100644
--- a/src/librustc/ty/util.rs
+++ b/src/librustc/ty/util.rs
@@ -257,10 +257,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) => {
@@ -281,6 +317,15 @@ impl<'tcx> TyCtxt<'tcx> {
                     }
                 }
 
+                ty::Projection(_) | ty::Opaque(..) => {
+                    let normalized = normalize(ty);
+                    if ty == normalized {
+                        return ty;
+                    } else {
+                        ty = normalized;
+                    }
+                }
+
                 _ => {
                     break;
                 }
@@ -294,10 +339,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.
+    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) {
@@ -319,6 +389,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,
             }
         }
diff --git a/src/librustc_codegen_ssa/base.rs b/src/librustc_codegen_ssa/base.rs
index ca7e17ec97a..00471095f2f 100644
--- a/src/librustc_codegen_ssa/base.rs
+++ b/src/librustc_codegen_ssa/base.rs
@@ -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()))
diff --git a/src/librustc_codegen_ssa/traits/type_.rs b/src/librustc_codegen_ssa/traits/type_.rs
index aa38d8d5184..13f72e23819 100644
--- a/src/librustc_codegen_ssa/traits/type_.rs
+++ b/src/librustc_codegen_ssa/traits/type_.rs
@@ -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,
diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs
index 3ef525979f8..980697360eb 100644
--- a/src/librustc_mir/interpret/cast.rs
+++ b/src/librustc_mir/interpret/cast.rs
@@ -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(_)) => {
diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs
index f0d64e217a2..bcd36ac547c 100644
--- a/src/librustc_mir/interpret/intern.rs
+++ b/src/librustc_mir/interpret/intern.rs
@@ -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
diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs
index 00107a536ba..b1b8e975138 100644
--- a/src/librustc_mir/interpret/validity.rs
+++ b/src/librustc_mir/interpret/validity.rs
@@ -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();
diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs
index bcc6ad4eac2..6e9390f7750 100644
--- a/src/librustc_mir/monomorphize/collector.rs
+++ b/src/librustc_mir/monomorphize/collector.rs
@@ -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,
@@ -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)
         }
     };
 
diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
index d32ee67f745..77d45cfa63c 100644
--- a/src/librustc_typeck/check/mod.rs
+++ b/src/librustc_typeck/check/mod.rs
@@ -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)
             }
diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs
index a41f4ec91a4..68e5e7d4fd2 100644
--- a/src/librustc_typeck/check/wfcheck.rs
+++ b/src/librustc_typeck/check/wfcheck.rs
@@ -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;
             }
         }
diff --git a/src/test/ui/layout/issue-60431-unsized-tail-behind-projection.rs b/src/test/ui/layout/issue-60431-unsized-tail-behind-projection.rs
new file mode 100644
index 00000000000..65845d2c9fe
--- /dev/null
+++ b/src/test/ui/layout/issue-60431-unsized-tail-behind-projection.rs
@@ -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::<&[()]>());
+}