Auto merge of #74717 - davidtwco:issue-74636-polymorphized-closures-i…

…nherited-params, r=oli-obk mir: add `used_generic_parameters_needs_subst` Fixes #74636. This PR adds a `used_generic_parameters_needs_subst` helper function which checks whether a type needs substitution, but only for parameters that the `unused_generic_params` query considers used. This is used in the MIR interpreter to make the check for some pointer casts and for reflection intrinsics more precise. I've opened this as a draft PR because this might not be the approach we want to fix this issue and we have to decide what to do about the reflection case. r? @eddyb cc @lcnr @wesleywiser
rust-lang · Aug 1, 2020 · 22e6099 · 22e6099
2 parents b544b43 + 59e621c
commit 22e6099
Show file tree

Hide file tree

Showing 7 changed files with 108 additions and 20 deletions.
diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs
@@ -8,11 +8,14 @@ use rustc_middle::mir::interpret::{InterpResult, PointerArithmetic, Scalar};
 use rustc_middle::mir::CastKind;
 use rustc_middle::ty::adjustment::PointerCast;
 use rustc_middle::ty::layout::{IntegerExt, TyAndLayout};
-use rustc_middle::ty::{self, Ty, TypeAndMut, TypeFoldable};
+use rustc_middle::ty::{self, Ty, TypeAndMut};
 use rustc_span::symbol::sym;
 use rustc_target::abi::{Integer, LayoutOf, Variants};
 
-use super::{truncate, FnVal, ImmTy, Immediate, InterpCx, Machine, OpTy, PlaceTy};
+use super::{
+    truncate, util::ensure_monomorphic_enough, FnVal, ImmTy, Immediate, InterpCx, Machine, OpTy,
+    PlaceTy,
+};
 
 impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     pub fn cast(
@@ -47,9 +50,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 match src.layout.ty.kind {
                     ty::FnDef(def_id, substs) => {
                         // All reifications must be monomorphic, bail out otherwise.
-                        if src.layout.ty.needs_subst() {
-                            throw_inval!(TooGeneric);
-                        }
+                        ensure_monomorphic_enough(*self.tcx, src.layout.ty)?;
 
                         if self.tcx.has_attr(def_id, sym::rustc_args_required_const) {
                             span_bug!(
@@ -89,9 +90,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 match src.layout.ty.kind {
                     ty::Closure(def_id, substs) => {
                         // All reifications must be monomorphic, bail out otherwise.
-                        if src.layout.ty.needs_subst() {
-                            throw_inval!(TooGeneric);
-                        }
+                        ensure_monomorphic_enough(*self.tcx, src.layout.ty)?;
 
                         let instance = ty::Instance::resolve_closure(
                             *self.tcx,

diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs
@@ -12,11 +12,13 @@ use rustc_middle::mir::{
 };
 use rustc_middle::ty;
 use rustc_middle::ty::subst::SubstsRef;
-use rustc_middle::ty::{Ty, TyCtxt, TypeFoldable};
+use rustc_middle::ty::{Ty, TyCtxt};
 use rustc_span::symbol::{sym, Symbol};
 use rustc_target::abi::{Abi, LayoutOf as _, Primitive, Size};
 
-use super::{CheckInAllocMsg, ImmTy, InterpCx, Machine, OpTy, PlaceTy};
+use super::{
+    util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, Machine, OpTy, PlaceTy,
+};
 
 mod caller_location;
 mod type_name;
@@ -54,9 +56,7 @@ crate fn eval_nullary_intrinsic<'tcx>(
     let name = tcx.item_name(def_id);
     Ok(match name {
         sym::type_name => {
-            if tp_ty.needs_subst() {
-                throw_inval!(TooGeneric);
-            }
+            ensure_monomorphic_enough(tcx, tp_ty)?;
             let alloc = type_name::alloc_type_name(tcx, tp_ty);
             ConstValue::Slice { data: alloc, start: 0, end: alloc.len() }
         }
@@ -72,9 +72,7 @@ crate fn eval_nullary_intrinsic<'tcx>(
             ConstValue::from_machine_usize(n, &tcx)
         }
         sym::type_id => {
-            if tp_ty.needs_subst() {
-                throw_inval!(TooGeneric);
-            }
+            ensure_monomorphic_enough(tcx, tp_ty)?;
             ConstValue::from_u64(tcx.type_id_hash(tp_ty))
         }
         sym::variant_count => {

diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs
@@ -12,6 +12,7 @@ mod place;
 mod step;
 mod terminator;
 mod traits;
+mod util;
 mod validity;
 mod visitor;
 

diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs
@@ -1,9 +1,10 @@
 use std::convert::TryFrom;
 
 use rustc_middle::mir::interpret::{InterpResult, Pointer, PointerArithmetic, Scalar};
-use rustc_middle::ty::{self, Instance, Ty, TypeFoldable};
+use rustc_middle::ty::{self, Instance, Ty};
 use rustc_target::abi::{Align, LayoutOf, Size};
 
+use super::util::ensure_monomorphic_enough;
 use super::{FnVal, InterpCx, Machine, MemoryKind};
 
 impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
@@ -23,9 +24,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         let (ty, poly_trait_ref) = self.tcx.erase_regions(&(ty, poly_trait_ref));
 
         // All vtables must be monomorphic, bail out otherwise.
-        if ty.needs_subst() || poly_trait_ref.needs_subst() {
-            throw_inval!(TooGeneric);
-        }
+        ensure_monomorphic_enough(*self.tcx, ty)?;
+        ensure_monomorphic_enough(*self.tcx, poly_trait_ref)?;
 
         if let Some(&vtable) = self.vtables.get(&(ty, poly_trait_ref)) {
             // This means we guarantee that there are no duplicate vtables, we will

diff --git a/src/librustc_mir/interpret/util.rs b/src/librustc_mir/interpret/util.rs
@@ -0,0 +1,73 @@
+use rustc_middle::mir::interpret::InterpResult;
+use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, TypeVisitor};
+use std::convert::TryInto;
+
+/// Returns `true` if a used generic parameter requires substitution.
+crate fn ensure_monomorphic_enough<'tcx, T>(tcx: TyCtxt<'tcx>, ty: T) -> InterpResult<'tcx>
+where
+    T: TypeFoldable<'tcx>,
+{
+    debug!("ensure_monomorphic_enough: ty={:?}", ty);
+    if !ty.needs_subst() {
+        return Ok(());
+    }
+
+    struct UsedParamsNeedSubstVisitor<'tcx> {
+        tcx: TyCtxt<'tcx>,
+    };
+
+    impl<'tcx> TypeVisitor<'tcx> for UsedParamsNeedSubstVisitor<'tcx> {
+        fn visit_const(&mut self, c: &'tcx ty::Const<'tcx>) -> bool {
+            if !c.needs_subst() {
+                return false;
+            }
+
+            match c.val {
+                ty::ConstKind::Param(..) => true,
+                _ => c.super_visit_with(self),
+            }
+        }
+
+        fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool {
+            if !ty.needs_subst() {
+                return false;
+            }
+
+            match ty.kind {
+                ty::Param(_) => true,
+                ty::Closure(def_id, substs)
+                | ty::Generator(def_id, substs, ..)
+                | ty::FnDef(def_id, substs) => {
+                    let unused_params = self.tcx.unused_generic_params(def_id);
+                    for (index, subst) in substs.into_iter().enumerate() {
+                        let index = index
+                            .try_into()
+                            .expect("more generic parameters than can fit into a `u32`");
+                        let is_used =
+                            unused_params.contains(index).map(|unused| !unused).unwrap_or(true);
+                        // Only recurse when generic parameters in fns, closures and generators
+                        // are used and require substitution.
+                        if is_used && subst.needs_subst() {
+                            // Just in case there are closures or generators within this subst,
+                            // recurse.
+                            if subst.super_visit_with(self) {
+                                // Only return when we find a parameter so the remaining substs
+                                // are not skipped.
+                                return true;
+                            }
+                        }
+                    }
+                    false
+                }
+                _ => ty.super_visit_with(self),
+            }
+        }
+    }
+
+    let mut vis = UsedParamsNeedSubstVisitor { tcx };
+    if ty.visit_with(&mut vis) {
+        throw_inval!(TooGeneric);
+    } else {
+        Ok(())
+    }
+}
diff --git a/src/test/ui/issues/issue-74614.rs b/src/test/ui/issues/issue-74614.rs
@@ -1,3 +1,4 @@
+// compile-flags:-Zpolymorphize=on
 // build-pass
 
 fn test<T>() {

diff --git a/src/test/ui/polymorphization/issue-74636.rs b/src/test/ui/polymorphization/issue-74636.rs
@@ -0,0 +1,16 @@
+// compile-flags:-Zpolymorphize=on
+// build-pass
+
+use std::any::TypeId;
+
+pub fn foo<T: 'static>(_: T) -> TypeId {
+    TypeId::of::<T>()
+}
+
+fn outer<T: 'static>() {
+    foo(|| ());
+}
+
+fn main() {
+    outer::<u8>();
+}