about summary refs log tree commit diff
diff options
context:
space:
mode:
authorDylan DPC <dylan.dpc@gmail.com>2020-04-05 13:13:14 +0200
committerGitHub <noreply@github.com>2020-04-05 13:13:14 +0200
commit31b8d65803c88e4e267b3aedb81164bf63ef99f3 (patch)
tree63d42c0cdf224a25a8bea5be1c6cfc2cc190555c
parentc185c4fe4740227100c869da44825ca6eb80be74 (diff)
parente52a4519131bccc732f53ec13582016bb2b1d04a (diff)
downloadrust-31b8d65803c88e4e267b3aedb81164bf63ef99f3.tar.gz
rust-31b8d65803c88e4e267b3aedb81164bf63ef99f3.zip
Rollup merge of #70806 - RalfJung:miri-assignment-check, r=eddyb
fix Miri assignment sanity check

Thanks @eddyb for pointing me to the right APIs!

r? @eddyb
Fixes https://github.com/rust-lang/rust/issues/70804
-rw-r--r--src/librustc_mir/interpret/eval_context.rs52
-rw-r--r--src/librustc_mir/interpret/operand.rs2
-rw-r--r--src/librustc_mir/interpret/place.rs4
-rw-r--r--src/test/ui/consts/const-eval/issue-70804-fn-subtyping.rs10
4 files changed, 49 insertions, 19 deletions
diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index 10d3101ebb8..0b182d42287 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -14,11 +14,11 @@ use rustc_middle::mir::interpret::{
     sign_extend, truncate, AllocId, FrameInfo, GlobalId, InterpResult, Pointer, Scalar,
 };
 use rustc_middle::ty::layout::{self, TyAndLayout};
-use rustc_middle::ty::query::TyCtxtAt;
-use rustc_middle::ty::subst::SubstsRef;
-use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable};
+use rustc_middle::ty::{
+    self, fold::BottomUpFolder, query::TyCtxtAt, subst::SubstsRef, Ty, TyCtxt, TypeFoldable,
+};
 use rustc_span::source_map::DUMMY_SP;
-use rustc_target::abi::{Abi, Align, HasDataLayout, LayoutOf, Size, TargetDataLayout};
+use rustc_target::abi::{Align, HasDataLayout, LayoutOf, Size, TargetDataLayout};
 
 use super::{
     Immediate, MPlaceTy, Machine, MemPlace, MemPlaceMeta, Memory, OpTy, Operand, Place, PlaceTy,
@@ -213,6 +213,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> LayoutOf for InterpCx<'mir, 'tcx, M> {
 /// Test if it is valid for a MIR assignment to assign `src`-typed place to `dest`-typed value.
 /// This test should be symmetric, as it is primarily about layout compatibility.
 pub(super) fn mir_assign_valid_types<'tcx>(
+    tcx: TyCtxt<'tcx>,
     src: TyAndLayout<'tcx>,
     dest: TyAndLayout<'tcx>,
 ) -> bool {
@@ -220,23 +221,42 @@ pub(super) fn mir_assign_valid_types<'tcx>(
         // Equal types, all is good.
         return true;
     }
-    // Type-changing assignments can happen for (at least) two reasons:
-    // - `&mut T` -> `&T` gets optimized from a reborrow to a mere assignment.
-    // - Subtyping is used. While all normal lifetimes are erased, higher-ranked lifetime
-    //   bounds are still around and can lead to type differences.
-    // There is no good way to check the latter, so we compare layouts instead -- but only
-    // for values with `Scalar`/`ScalarPair` abi.
-    // FIXME: Do something more accurate, type-based.
-    match &src.abi {
-        Abi::Scalar(..) | Abi::ScalarPair(..) => src.layout == dest.layout,
-        _ => false,
+    if src.layout != dest.layout {
+        // Layout differs, definitely not equal.
+        // We do this here because Miri would *do the wrong thing* if we allowed layout-changing
+        // assignments.
+        return false;
     }
+
+    // Type-changing assignments can happen for (at least) two reasons:
+    // 1. `&mut T` -> `&T` gets optimized from a reborrow to a mere assignment.
+    // 2. Subtyping is used. While all normal lifetimes are erased, higher-ranked types
+    //    with their late-bound lifetimes are still around and can lead to type differences.
+    // Normalize both of them away.
+    let normalize = |ty: Ty<'tcx>| {
+        ty.fold_with(&mut BottomUpFolder {
+            tcx,
+            // Normalize all references to immutable.
+            ty_op: |ty| match ty.kind {
+                ty::Ref(_, pointee, _) => tcx.mk_imm_ref(tcx.lifetimes.re_erased, pointee),
+                _ => ty,
+            },
+            // We just erase all late-bound lifetimes, but this is not fully correct (FIXME):
+            // lifetimes in invariant positions could matter (e.g. through associated types).
+            // We rely on the fact that layout was confirmed to be equal above.
+            lt_op: |_| tcx.lifetimes.re_erased,
+            // Leave consts unchanged.
+            ct_op: |ct| ct,
+        })
+    };
+    normalize(src.ty) == normalize(dest.ty)
 }
 
 /// Use the already known layout if given (but sanity check in debug mode),
 /// or compute the layout.
 #[cfg_attr(not(debug_assertions), inline(always))]
 pub(super) fn from_known_layout<'tcx>(
+    tcx: TyCtxt<'tcx>,
     known_layout: Option<TyAndLayout<'tcx>>,
     compute: impl FnOnce() -> InterpResult<'tcx, TyAndLayout<'tcx>>,
 ) -> InterpResult<'tcx, TyAndLayout<'tcx>> {
@@ -246,7 +266,7 @@ pub(super) fn from_known_layout<'tcx>(
             if cfg!(debug_assertions) {
                 let check_layout = compute()?;
                 assert!(
-                    mir_assign_valid_types(check_layout, known_layout),
+                    mir_assign_valid_types(tcx, check_layout, known_layout),
                     "expected type differs from actual type.\nexpected: {:?}\nactual: {:?}",
                     known_layout.ty,
                     check_layout.ty,
@@ -424,7 +444,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // have to support that case (mostly by skipping all caching).
         match frame.locals.get(local).and_then(|state| state.layout.get()) {
             None => {
-                let layout = from_known_layout(layout, || {
+                let layout = from_known_layout(self.tcx.tcx, layout, || {
                     let local_ty = frame.body.local_decls[local].ty;
                     let local_ty =
                         self.subst_from_frame_and_normalize_erasing_regions(frame, local_ty);
diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index 12595e4e4d9..03614b2803f 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -529,7 +529,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             ty::ConstKind::Value(val_val) => val_val,
         };
         // Other cases need layout.
-        let layout = from_known_layout(layout, || self.layout_of(val.ty))?;
+        let layout = from_known_layout(self.tcx.tcx, layout, || self.layout_of(val.ty))?;
         let op = match val_val {
             ConstValue::ByRef { alloc, offset } => {
                 let id = self.tcx.alloc_map.lock().create_memory_alloc(alloc);
diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs
index ec299cdd213..716c7c7d933 100644
--- a/src/librustc_mir/interpret/place.rs
+++ b/src/librustc_mir/interpret/place.rs
@@ -868,7 +868,7 @@ where
         // We do NOT compare the types for equality, because well-typed code can
         // actually "transmute" `&mut T` to `&T` in an assignment without a cast.
         assert!(
-            mir_assign_valid_types(src.layout, dest.layout),
+            mir_assign_valid_types(self.tcx.tcx, src.layout, dest.layout),
             "type mismatch when copying!\nsrc: {:?},\ndest: {:?}",
             src.layout.ty,
             dest.layout.ty,
@@ -922,7 +922,7 @@ where
         src: OpTy<'tcx, M::PointerTag>,
         dest: PlaceTy<'tcx, M::PointerTag>,
     ) -> InterpResult<'tcx> {
-        if mir_assign_valid_types(src.layout, dest.layout) {
+        if mir_assign_valid_types(self.tcx.tcx, src.layout, dest.layout) {
             // Fast path: Just use normal `copy_op`
             return self.copy_op(src, dest);
         }
diff --git a/src/test/ui/consts/const-eval/issue-70804-fn-subtyping.rs b/src/test/ui/consts/const-eval/issue-70804-fn-subtyping.rs
new file mode 100644
index 00000000000..59d46ea66c9
--- /dev/null
+++ b/src/test/ui/consts/const-eval/issue-70804-fn-subtyping.rs
@@ -0,0 +1,10 @@
+// check-pass
+#![feature(const_fn)]
+
+const fn nested(x: (for<'a> fn(&'a ()), String)) -> (fn(&'static ()), String) {
+    x
+}
+
+pub const TEST: (fn(&'static ()), String) = nested((|_x| (), String::new()));
+
+fn main() {}