about summary refs log tree commit diff
diff options
context:
space:
mode:
authorDylan DPC <dylan.dpc@gmail.com>2020-01-06 12:00:16 +0530
committerGitHub <noreply@github.com>2020-01-06 12:00:16 +0530
commit36920750499f5ad03cd2c35dabf09094fba7b679 (patch)
treeb031b79a5a4da555152caeb52b07fa3694bb6f7e
parent33640f0e03af2fb31ce380d5389d5545f24ce29a (diff)
parent336b902f669167d09782702dffcc12513ebc8950 (diff)
Rollup merge of #67800 - Aaron1011:fix/mir-generic-instance, r=oli-obk
Fix ICE involving calling `Instance.ty` during const evaluation

Fixes #67639

`Instance.ty` assumes that we are in a fully monomorphic context (e.g.
codegen), and can therefore use an empty `ParamEnv` when performing
normalization. Howver, the MIR constant evaluator code ends up calling
`Instance.ty` as a result of us attemptign to 'speculatively'
const-evaluate generic functions during const propagation.

As a result,
we may end up with projections involving type parameters
(e.g. <T as MyTrait>::Bar>) in the type we are trying to normalize.
Normalization expects us to have proper predicates in the `ParamEnv` for
such projections, and will ICE if we don't.

This commit adds a new method `Instance.ty_env`, which takes a
`ParamEnv` for use during normalization. The MIR const-evaluator code is
changed to use this method, passing in the proper `ParamEnv` for the
context at hand.
-rw-r--r--src/librustc/ty/instance.rs27
-rw-r--r--src/librustc/ty/layout.rs2
-rw-r--r--src/librustc_codegen_llvm/callee.rs2
-rw-r--r--src/librustc_codegen_llvm/consts.rs4
-rw-r--r--src/librustc_codegen_llvm/debuginfo/metadata.rs2
-rw-r--r--src/librustc_codegen_llvm/intrinsic.rs2
-rw-r--r--src/librustc_codegen_llvm/mono_item.rs2
-rw-r--r--src/librustc_mir/const_eval/eval_queries.rs2
-rw-r--r--src/librustc_mir/interpret/terminator.rs2
-rw-r--r--src/librustc_mir/interpret/traits.rs2
-rw-r--r--src/librustc_mir/monomorphize/collector.rs5
-rw-r--r--src/test/ui/mir/issue-67639-normalization-ice.rs34
12 files changed, 73 insertions, 13 deletions
diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs
index a7e716ad7b7..e315de11568 100644
--- a/src/librustc/ty/instance.rs
+++ b/src/librustc/ty/instance.rs
@@ -62,10 +62,35 @@ pub enum InstanceDef<'tcx> {
 }
 
 impl<'tcx> Instance<'tcx> {
-    pub fn ty(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> {
+    /// Returns the `Ty` corresponding to this `Instance`,
+    /// with generic substitutions applied and lifetimes erased.
+    ///
+    /// This method can only be called when the 'substs' for this Instance
+    /// are fully monomorphic (no `ty::Param`'s are present).
+    /// This is usually the case (e.g. during codegen).
+    /// However, during constant evaluation, we may want
+    /// to try to resolve a `Instance` using generic parameters
+    /// (e.g. when we are attempting to to do const-propagation).
+    /// In this case, `Instance.ty_env` should be used to provide
+    /// the `ParamEnv` for our generic context.
+    pub fn monomorphic_ty(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> {
         let ty = tcx.type_of(self.def.def_id());
+        // There shouldn't be any params - if there are, then
+        // Instance.ty_env should have been used to provide the proper
+        // ParamEnv
+        if self.substs.has_param_types() {
+            bug!("Instance.ty called for type {:?} with params in substs: {:?}", ty, self.substs);
+        }
         tcx.subst_and_normalize_erasing_regions(self.substs, ty::ParamEnv::reveal_all(), &ty)
     }
+
+    /// Like `Instance.ty`, but allows a `ParamEnv` to be specified for use during
+    /// normalization. This method is only really useful during constant evaluation,
+    /// where we are dealing with potentially generic types.
+    pub fn ty_env(&self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> Ty<'tcx> {
+        let ty = tcx.type_of(self.def.def_id());
+        tcx.subst_and_normalize_erasing_regions(self.substs, param_env, &ty)
+    }
 }
 
 impl<'tcx> InstanceDef<'tcx> {
diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs
index 63345f828e0..5f599034e7d 100644
--- a/src/librustc/ty/layout.rs
+++ b/src/librustc/ty/layout.rs
@@ -2301,7 +2301,7 @@ impl<'tcx> ty::Instance<'tcx> {
     // or should go through `FnAbi` instead, to avoid losing any
     // adjustments `FnAbi::of_instance` might be performing.
     fn fn_sig_for_fn_abi(&self, tcx: TyCtxt<'tcx>) -> ty::PolyFnSig<'tcx> {
-        let ty = self.ty(tcx);
+        let ty = self.monomorphic_ty(tcx);
         match ty.kind {
             ty::FnDef(..) |
             // Shims currently have type FnPtr. Not sure this should remain.
diff --git a/src/librustc_codegen_llvm/callee.rs b/src/librustc_codegen_llvm/callee.rs
index 7be179a2098..78dd6fc8ffe 100644
--- a/src/librustc_codegen_llvm/callee.rs
+++ b/src/librustc_codegen_llvm/callee.rs
@@ -36,7 +36,7 @@ pub fn get_fn(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) -> &'ll Value
     }
 
     let sym = tcx.symbol_name(instance).name.as_str();
-    debug!("get_fn({:?}: {:?}) => {}", instance, instance.ty(cx.tcx()), sym);
+    debug!("get_fn({:?}: {:?}) => {}", instance, instance.monomorphic_ty(cx.tcx()), sym);
 
     let fn_abi = FnAbi::of_instance(cx, instance, &[]);
 
diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs
index 4b4fbd0e1ad..bccf3f5735c 100644
--- a/src/librustc_codegen_llvm/consts.rs
+++ b/src/librustc_codegen_llvm/consts.rs
@@ -204,7 +204,7 @@ impl CodegenCx<'ll, 'tcx> {
             def_id
         );
 
-        let ty = instance.ty(self.tcx);
+        let ty = instance.monomorphic_ty(self.tcx);
         let sym = self.tcx.symbol_name(instance).name;
 
         debug!("get_static: sym={} instance={:?}", sym, instance);
@@ -361,7 +361,7 @@ impl StaticMethods for CodegenCx<'ll, 'tcx> {
             };
 
             let instance = Instance::mono(self.tcx, def_id);
-            let ty = instance.ty(self.tcx);
+            let ty = instance.monomorphic_ty(self.tcx);
             let llty = self.layout_of(ty).llvm_type(self);
             let g = if val_llty == llty {
                 g
diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs
index 88e692cb117..a1e81c10b41 100644
--- a/src/librustc_codegen_llvm/debuginfo/metadata.rs
+++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs
@@ -2287,7 +2287,7 @@ pub fn create_global_var_metadata(cx: &CodegenCx<'ll, '_>, def_id: DefId, global
     };
 
     let is_local_to_unit = is_node_local_to_unit(cx, def_id);
-    let variable_type = Instance::mono(cx.tcx, def_id).ty(cx.tcx);
+    let variable_type = Instance::mono(cx.tcx, def_id).monomorphic_ty(cx.tcx);
     let type_metadata = type_metadata(cx, variable_type, span);
     let var_name = SmallCStr::new(&tcx.item_name(def_id).as_str());
     let linkage_name = if no_mangle {
diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs
index 0e121df1d25..1c714630838 100644
--- a/src/librustc_codegen_llvm/intrinsic.rs
+++ b/src/librustc_codegen_llvm/intrinsic.rs
@@ -89,7 +89,7 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> {
         span: Span,
     ) {
         let tcx = self.tcx;
-        let callee_ty = instance.ty(tcx);
+        let callee_ty = instance.monomorphic_ty(tcx);
 
         let (def_id, substs) = match callee_ty.kind {
             ty::FnDef(def_id, substs) => (def_id, substs),
diff --git a/src/librustc_codegen_llvm/mono_item.rs b/src/librustc_codegen_llvm/mono_item.rs
index e0a946a38e4..681bc1f2dcb 100644
--- a/src/librustc_codegen_llvm/mono_item.rs
+++ b/src/librustc_codegen_llvm/mono_item.rs
@@ -22,7 +22,7 @@ impl PreDefineMethods<'tcx> for CodegenCx<'ll, 'tcx> {
         symbol_name: &str,
     ) {
         let instance = Instance::mono(self.tcx, def_id);
-        let ty = instance.ty(self.tcx);
+        let ty = instance.monomorphic_ty(self.tcx);
         let llty = self.layout_of(ty).llvm_type(self);
 
         let g = self.define_global(symbol_name, llty).unwrap_or_else(|| {
diff --git a/src/librustc_mir/const_eval/eval_queries.rs b/src/librustc_mir/const_eval/eval_queries.rs
index d55620f657b..46e76512d35 100644
--- a/src/librustc_mir/const_eval/eval_queries.rs
+++ b/src/librustc_mir/const_eval/eval_queries.rs
@@ -221,7 +221,7 @@ pub fn const_eval_validated_provider<'tcx>(
     // We call `const_eval` for zero arg intrinsics, too, in order to cache their value.
     // Catch such calls and evaluate them instead of trying to load a constant's MIR.
     if let ty::InstanceDef::Intrinsic(def_id) = key.value.instance.def {
-        let ty = key.value.instance.ty(tcx);
+        let ty = key.value.instance.ty_env(tcx, key.param_env);
         let substs = match ty.kind {
             ty::FnDef(_, substs) => substs,
             _ => bug!("intrinsic with type {:?}", ty),
diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs
index 032062d6360..a28bb539fd0 100644
--- a/src/librustc_mir/interpret/terminator.rs
+++ b/src/librustc_mir/interpret/terminator.rs
@@ -204,7 +204,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // ABI check
         {
             let callee_abi = {
-                let instance_ty = instance.ty(*self.tcx);
+                let instance_ty = instance.ty_env(*self.tcx, self.param_env);
                 match instance_ty.kind {
                     ty::FnDef(..) => instance_ty.fn_sig(*self.tcx).abi(),
                     ty::Closure(..) => Abi::RustCall,
diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs
index d6fd48cc89f..efbbca53485 100644
--- a/src/librustc_mir/interpret/traits.rs
+++ b/src/librustc_mir/interpret/traits.rs
@@ -140,7 +140,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // to determine the type.
         let drop_instance = self.memory.get_fn(drop_fn)?.as_instance()?;
         trace!("Found drop fn: {:?}", drop_instance);
-        let fn_sig = drop_instance.ty(*self.tcx).fn_sig(*self.tcx);
+        let fn_sig = drop_instance.ty_env(*self.tcx, self.param_env).fn_sig(*self.tcx);
         let fn_sig = self.tcx.normalize_erasing_late_bound_regions(self.param_env, &fn_sig);
         // The drop function takes `*mut T` where `T` is the type being dropped, so get that.
         let args = fn_sig.inputs();
diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs
index 231ee792918..dca850462ce 100644
--- a/src/librustc_mir/monomorphize/collector.rs
+++ b/src/librustc_mir/monomorphize/collector.rs
@@ -358,7 +358,7 @@ fn collect_items_rec<'tcx>(
             // Sanity check whether this ended up being collected accidentally
             debug_assert!(should_monomorphize_locally(tcx, &instance));
 
-            let ty = instance.ty(tcx);
+            let ty = instance.monomorphic_ty(tcx);
             visit_drop_use(tcx, ty, true, &mut neighbors);
 
             recursion_depth_reset = None;
@@ -1002,7 +1002,8 @@ impl ItemLikeVisitor<'v> for RootCollector<'_, 'v> {
                             def_id_to_string(self.tcx, def_id)
                         );
 
-                        let ty = Instance::new(def_id, InternalSubsts::empty()).ty(self.tcx);
+                        let ty =
+                            Instance::new(def_id, InternalSubsts::empty()).monomorphic_ty(self.tcx);
                         visit_drop_use(self.tcx, ty, true, self.output);
                     }
                 }
diff --git a/src/test/ui/mir/issue-67639-normalization-ice.rs b/src/test/ui/mir/issue-67639-normalization-ice.rs
new file mode 100644
index 00000000000..21851a72525
--- /dev/null
+++ b/src/test/ui/mir/issue-67639-normalization-ice.rs
@@ -0,0 +1,34 @@
+// compile-flags: -Z mir-opt-level=3
+// build-pass
+
+// This used to ICE in const-prop due
+// to an empty ParamEnv being used during normalization
+// of a generic type
+
+
+fn main() {
+    join_all::<u32>();
+}
+
+trait Foo {
+    type Item;
+}
+
+impl Foo for u32 {
+    type Item = u8;
+}
+
+trait Bar {
+    type Item2;
+}
+
+impl Bar for u8 {
+    type Item2 = u64;
+}
+
+fn join_all<I>()
+where I: Foo,
+    I::Item: Bar
+{
+    Vec::<<I::Item as Bar>::Item2>::new(); // ICE occurs processing this line
+}