Auto merge of #65989 - Aaron1011:fix/normalize-param-env, r=nikomatsakis

Normalize all opaque types when converting ParamEnv to Reveal::All

When we normalize a type using a ParamEnv with a reveal mode of
RevealMode::All, we will normalize opaque types to their underlying
types (e.g. `type MyOpaque = impl Foo` -> `StructThatImplsFoo`).
However, the ParamEnv may still have predicates referring to the
un-normalized opaque type (e.g. `<T as MyTrait<MyOpaque>>`). This can
cause trait projection to fail, since a type containing normalized
opaque types will not match up with the un-normalized type in the
`ParamEnv`.

To fix this, we now explicitly normalize all opaque types in
caller_bounds of a `ParamEnv` when changing its mode to
`RevealMode::All`. This ensures that all predicatse will refer to the
underlying types of any opaque types involved, allowing them to be
matched up properly during projection. To reflect the fact that
normalization is occuring, `ParamEnv::with_reveal_all` is renamed to
`ParamEnv::with_reveal_all_normalized`

Fixes #65918

Author: bors

Diff for: src/librustc_middle/mir/interpret/queries.rs

@@ -18,7 +18,7 @@ impl<'tcx> TyCtxt<'tcx> {
         let substs = InternalSubsts::identity_for_item(self, def_id);
         let instance = ty::Instance::new(def_id, substs);
         let cid = GlobalId { instance, promoted: None };
-        let param_env = self.param_env(def_id).with_reveal_all();
+        let param_env = self.param_env(def_id).with_reveal_all_normalized(self);
         self.const_eval_global_id(param_env, cid, None)
     }
 

Diff for: src/librustc_middle/mir/interpret/value.rs

@@ -78,7 +78,7 @@ impl<'tcx> ConstValue<'tcx> {
         param_env: ParamEnv<'tcx>,
         ty: Ty<'tcx>,
     ) -> Option<u128> {
-        let size = tcx.layout_of(param_env.with_reveal_all().and(ty)).ok()?.size;
+        let size = tcx.layout_of(param_env.with_reveal_all_normalized(tcx).and(ty)).ok()?.size;
         self.try_to_bits(size)
     }
 

Diff for: src/librustc_middle/query/mod.rs

@@ -864,11 +864,17 @@ rustc_queries! {
         /// type-checking etc, and it does not normalize specializable
         /// associated types. This is almost always what you want,
         /// unless you are doing MIR optimizations, in which case you
-        /// might want to use `reveal_all()` method to change modes.
         query param_env(def_id: DefId) -> ty::ParamEnv<'tcx> {
             desc { |tcx| "computing normalized predicates of `{}`", tcx.def_path_str(def_id) }
         }
 
+        /// Like `param_env`, but returns the `ParamEnv in `Reveal::All` mode.
+        /// Prefer this over `tcx.param_env(def_id).with_reveal_all_normalized(tcx)`,
+        /// as this method is more efficient.
+        query param_env_reveal_all_normalized(def_id: DefId) -> ty::ParamEnv<'tcx> {
+            desc { |tcx| "computing revealed normalized predicates of `{}`", tcx.def_path_str(def_id) }
+        }
+
         /// Trait selection queries. These are best used by invoking `ty.is_copy_modulo_regions()`,
         /// `ty.is_copy()`, etc, since that will prune the environment where possible.
         query is_copy_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
@@ -1527,5 +1533,9 @@ rustc_queries! {
                 ty::Instance::new(key.value.0.to_def_id(), key.value.2),
             }
         }
+
+        query normalize_opaque_types(key: &'tcx ty::List<ty::Predicate<'tcx>>) -> &'tcx ty::List<ty::Predicate<'tcx>> {
+            desc { "normalizing opaque types in {:?}", key }
+        }
     }
 }

Diff for: src/librustc_middle/ty/consts.rs

@@ -158,7 +158,7 @@ impl<'tcx> Const<'tcx> {
         ty: Ty<'tcx>,
     ) -> Option<u128> {
         assert_eq!(self.ty, ty);
-        let size = tcx.layout_of(param_env.with_reveal_all().and(ty)).ok()?.size;
+        let size = tcx.layout_of(param_env.with_reveal_all_normalized(tcx).and(ty)).ok()?.size;
         // if `ty` does not depend on generic parameters, use an empty param_env
         self.val.eval(tcx, param_env).try_to_bits(size)
     }

Diff for: src/librustc_middle/ty/consts/kind.rs

@@ -96,12 +96,16 @@ impl<'tcx> ConstKind<'tcx> {
         if let ConstKind::Unevaluated(def, substs, promoted) = self {
             use crate::mir::interpret::ErrorHandled;
 
-            let param_env_and_substs = param_env.with_reveal_all().and(substs);
-
             // HACK(eddyb) this erases lifetimes even though `const_eval_resolve`
             // also does later, but we want to do it before checking for
             // inference variables.
-            let param_env_and_substs = tcx.erase_regions(&param_env_and_substs);
+            // Note that we erase regions *before* calling `with_reveal_all_normalized`,
+            // so that we don't try to invoke this query with
+            // any region variables.
+            let param_env_and_substs = tcx
+                .erase_regions(&param_env)
+                .with_reveal_all_normalized(tcx)
+                .and(tcx.erase_regions(&substs));
 
             // HACK(eddyb) when the query key would contain inference variables,
             // attempt using identity substs and `ParamEnv` instead, that will succeed

Diff for: src/librustc_middle/ty/layout.rs

@@ -1903,7 +1903,7 @@ impl<'tcx> LayoutOf for LayoutCx<'tcx, TyCtxt<'tcx>> {
     /// Computes the layout of a type. Note that this implicitly
     /// executes in "reveal all" mode.
     fn layout_of(&self, ty: Ty<'tcx>) -> Self::TyAndLayout {
-        let param_env = self.param_env.with_reveal_all();
+        let param_env = self.param_env.with_reveal_all_normalized(self.tcx);
         let ty = self.tcx.normalize_erasing_regions(param_env, ty);
         let layout = self.tcx.layout_raw(param_env.and(ty))?;
         let layout = TyAndLayout { ty, layout };
@@ -1927,7 +1927,7 @@ impl LayoutOf for LayoutCx<'tcx, ty::query::TyCtxtAt<'tcx>> {
     /// Computes the layout of a type. Note that this implicitly
     /// executes in "reveal all" mode.
     fn layout_of(&self, ty: Ty<'tcx>) -> Self::TyAndLayout {
-        let param_env = self.param_env.with_reveal_all();
+        let param_env = self.param_env.with_reveal_all_normalized(*self.tcx);
         let ty = self.tcx.normalize_erasing_regions(param_env, ty);
         let layout = self.tcx.layout_raw(param_env.and(ty))?;
         let layout = TyAndLayout { ty, layout };

Diff for: src/librustc_middle/ty/mod.rs

@@ -1,5 +1,5 @@
 // ignore-tidy-filelength
-pub use self::fold::{TypeFoldable, TypeVisitor};
+pub use self::fold::{TypeFoldable, TypeFolder, TypeVisitor};
 pub use self::AssocItemContainer::*;
 pub use self::BorrowKind::*;
 pub use self::IntVarValue::*;
@@ -1874,9 +1874,15 @@ impl<'tcx> ParamEnv<'tcx> {
     /// the desired behavior during codegen and certain other special
     /// contexts; normally though we want to use `Reveal::UserFacing`,
     /// which is the default.
-    pub fn with_reveal_all(mut self) -> Self {
-        self.packed_data |= 1;
-        self
+    /// All opaque types in the caller_bounds of the `ParamEnv`
+    /// will be normalized to their underlying types.
+    /// See PR #65989 and issue #65918 for more details
+    pub fn with_reveal_all_normalized(self, tcx: TyCtxt<'tcx>) -> Self {
+        if self.packed_data & 1 == 1 {
+            return self;
+        }
+
+        ParamEnv::new(tcx.normalize_opaque_types(self.caller_bounds()), Reveal::All, self.def_id)
     }
 
     /// Returns this same environment but with no caller bounds.
@@ -3116,6 +3122,7 @@ pub fn provide(providers: &mut ty::query::Providers) {
     context::provide(providers);
     erase_regions::provide(providers);
     layout::provide(providers);
+    util::provide(providers);
     super::util::bug::provide(providers);
     *providers = ty::query::Providers {
         trait_impls_of: trait_def::trait_impls_of_provider,

Diff for: src/librustc_middle/ty/query/keys.rs

@@ -270,6 +270,17 @@ impl<'tcx> Key for Ty<'tcx> {
     }
 }
 
+impl<'tcx> Key for &'tcx ty::List<ty::Predicate<'tcx>> {
+    type CacheSelector = DefaultCacheSelector;
+
+    fn query_crate(&self) -> CrateNum {
+        LOCAL_CRATE
+    }
+    fn default_span(&self, _: TyCtxt<'_>) -> Span {
+        DUMMY_SP
+    }
+}
+
 impl<'tcx> Key for ty::ParamEnv<'tcx> {
     type CacheSelector = DefaultCacheSelector;
 

Diff for: src/librustc_middle/ty/util.rs

@@ -3,11 +3,12 @@
 use crate::ich::NodeIdHashingMode;
 use crate::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use crate::mir::interpret::{sign_extend, truncate};
+use crate::ty::fold::TypeFolder;
 use crate::ty::layout::IntegerExt;
 use crate::ty::query::TyCtxtAt;
 use crate::ty::subst::{GenericArgKind, InternalSubsts, Subst, SubstsRef};
 use crate::ty::TyKind::*;
-use crate::ty::{self, DefIdTree, GenericParamDefKind, Ty, TyCtxt, TypeFoldable};
+use crate::ty::{self, DefIdTree, GenericParamDefKind, List, Ty, TyCtxt, TypeFoldable};
 use rustc_apfloat::Float as _;
 use rustc_ast::ast;
 use rustc_attr::{self as attr, SignedInt, UnsignedInt};
@@ -557,82 +558,84 @@ impl<'tcx> TyCtxt<'tcx> {
         def_id: DefId,
         substs: SubstsRef<'tcx>,
     ) -> Result<Ty<'tcx>, Ty<'tcx>> {
-        use crate::ty::fold::TypeFolder;
-
-        struct OpaqueTypeExpander<'tcx> {
-            // Contains the DefIds of the opaque types that are currently being
-            // expanded. When we expand an opaque type we insert the DefId of
-            // that type, and when we finish expanding that type we remove the
-            // its DefId.
-            seen_opaque_tys: FxHashSet<DefId>,
-            // Cache of all expansions we've seen so far. This is a critical
-            // optimization for some large types produced by async fn trees.
-            expanded_cache: FxHashMap<(DefId, SubstsRef<'tcx>), Ty<'tcx>>,
-            primary_def_id: DefId,
-            found_recursion: bool,
-            tcx: TyCtxt<'tcx>,
-        }
-
-        impl<'tcx> OpaqueTypeExpander<'tcx> {
-            fn expand_opaque_ty(
-                &mut self,
-                def_id: DefId,
-                substs: SubstsRef<'tcx>,
-            ) -> Option<Ty<'tcx>> {
-                if self.found_recursion {
-                    return None;
-                }
-                let substs = substs.fold_with(self);
-                if self.seen_opaque_tys.insert(def_id) {
-                    let expanded_ty = match self.expanded_cache.get(&(def_id, substs)) {
-                        Some(expanded_ty) => expanded_ty,
-                        None => {
-                            let generic_ty = self.tcx.type_of(def_id);
-                            let concrete_ty = generic_ty.subst(self.tcx, substs);
-                            let expanded_ty = self.fold_ty(concrete_ty);
-                            self.expanded_cache.insert((def_id, substs), expanded_ty);
-                            expanded_ty
-                        }
-                    };
-                    self.seen_opaque_tys.remove(&def_id);
-                    Some(expanded_ty)
-                } else {
-                    // If another opaque type that we contain is recursive, then it
-                    // will report the error, so we don't have to.
-                    self.found_recursion = def_id == self.primary_def_id;
-                    None
-                }
-            }
-        }
-
-        impl<'tcx> TypeFolder<'tcx> for OpaqueTypeExpander<'tcx> {
-            fn tcx(&self) -> TyCtxt<'tcx> {
-                self.tcx
-            }
-
-            fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
-                if let ty::Opaque(def_id, substs) = t.kind {
-                    self.expand_opaque_ty(def_id, substs).unwrap_or(t)
-                } else if t.has_opaque_types() {
-                    t.super_fold_with(self)
-                } else {
-                    t
-                }
-            }
-        }
-
         let mut visitor = OpaqueTypeExpander {
             seen_opaque_tys: FxHashSet::default(),
             expanded_cache: FxHashMap::default(),
-            primary_def_id: def_id,
+            primary_def_id: Some(def_id),
             found_recursion: false,
+            check_recursion: true,
             tcx: self,
         };
+
         let expanded_type = visitor.expand_opaque_ty(def_id, substs).unwrap();
         if visitor.found_recursion { Err(expanded_type) } else { Ok(expanded_type) }
     }
 }
 
+struct OpaqueTypeExpander<'tcx> {
+    // Contains the DefIds of the opaque types that are currently being
+    // expanded. When we expand an opaque type we insert the DefId of
+    // that type, and when we finish expanding that type we remove the
+    // its DefId.
+    seen_opaque_tys: FxHashSet<DefId>,
+    // Cache of all expansions we've seen so far. This is a critical
+    // optimization for some large types produced by async fn trees.
+    expanded_cache: FxHashMap<(DefId, SubstsRef<'tcx>), Ty<'tcx>>,
+    primary_def_id: Option<DefId>,
+    found_recursion: bool,
+    /// Whether or not to check for recursive opaque types.
+    /// This is `true` when we're explicitly checking for opaque type
+    /// recursion, and 'false' otherwise to avoid unecessary work.
+    check_recursion: bool,
+    tcx: TyCtxt<'tcx>,
+}
+
+impl<'tcx> OpaqueTypeExpander<'tcx> {
+    fn expand_opaque_ty(&mut self, def_id: DefId, substs: SubstsRef<'tcx>) -> Option<Ty<'tcx>> {
+        if self.found_recursion {
+            return None;
+        }
+        let substs = substs.fold_with(self);
+        if !self.check_recursion || self.seen_opaque_tys.insert(def_id) {
+            let expanded_ty = match self.expanded_cache.get(&(def_id, substs)) {
+                Some(expanded_ty) => expanded_ty,
+                None => {
+                    let generic_ty = self.tcx.type_of(def_id);
+                    let concrete_ty = generic_ty.subst(self.tcx, substs);
+                    let expanded_ty = self.fold_ty(concrete_ty);
+                    self.expanded_cache.insert((def_id, substs), expanded_ty);
+                    expanded_ty
+                }
+            };
+            if self.check_recursion {
+                self.seen_opaque_tys.remove(&def_id);
+            }
+            Some(expanded_ty)
+        } else {
+            // If another opaque type that we contain is recursive, then it
+            // will report the error, so we don't have to.
+            self.found_recursion = def_id == *self.primary_def_id.as_ref().unwrap();
+            None
+        }
+    }
+}
+
+impl<'tcx> TypeFolder<'tcx> for OpaqueTypeExpander<'tcx> {
+    fn tcx(&self) -> TyCtxt<'tcx> {
+        self.tcx
+    }
+
+    fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
+        if let ty::Opaque(def_id, substs) = t.kind {
+            self.expand_opaque_ty(def_id, substs).unwrap_or(t)
+        } else if t.has_opaque_types() {
+            t.super_fold_with(self)
+        } else {
+            t
+        }
+    }
+}
+
 impl<'tcx> ty::TyS<'tcx> {
     /// Returns the maximum value for the given numeric type (including `char`s)
     /// or returns `None` if the type is not numeric.
@@ -1142,3 +1145,24 @@ pub fn needs_drop_components(
 
 #[derive(Copy, Clone, Debug, HashStable, RustcEncodable, RustcDecodable)]
 pub struct AlwaysRequiresDrop;
+
+/// Normalizes all opaque types in the given value, replacing them
+/// with their underlying types.
+pub fn normalize_opaque_types(
+    tcx: TyCtxt<'tcx>,
+    val: &'tcx List<ty::Predicate<'tcx>>,
+) -> &'tcx List<ty::Predicate<'tcx>> {
+    let mut visitor = OpaqueTypeExpander {
+        seen_opaque_tys: FxHashSet::default(),
+        expanded_cache: FxHashMap::default(),
+        primary_def_id: None,
+        found_recursion: false,
+        check_recursion: false,
+        tcx,
+    };
+    val.fold_with(&mut visitor)
+}
+
+pub fn provide(providers: &mut ty::query::Providers) {
+    *providers = ty::query::Providers { normalize_opaque_types, ..*providers }
+}

Diff for: src/librustc_mir/shim.rs

@@ -193,7 +193,7 @@ fn build_drop_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, ty: Option<Ty<'tcx>>)
             );
         }
         let patch = {
-            let param_env = tcx.param_env(def_id).with_reveal_all();
+            let param_env = tcx.param_env_reveal_all_normalized(def_id);
             let mut elaborator =
                 DropShimElaborator { body: &body, patch: MirPatch::new(&body), tcx, param_env };
             let dropee = tcx.mk_place_deref(dropee_ptr);

Diff for: src/librustc_mir/transform/const_prop.rs

@@ -328,7 +328,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
     ) -> ConstPropagator<'mir, 'tcx> {
         let def_id = source.def_id();
         let substs = &InternalSubsts::identity_for_item(tcx, def_id);
-        let param_env = tcx.param_env(def_id).with_reveal_all();
+        let param_env = tcx.param_env_reveal_all_normalized(def_id);
 
         let span = tcx.def_span(def_id);
         let can_const_prop = CanConstProp::check(body);

Diff for: src/librustc_mir/transform/elaborate_drops.rs

@@ -25,7 +25,7 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops {
         debug!("elaborate_drops({:?} @ {:?})", src, body.span);
 
         let def_id = src.def_id();
-        let param_env = tcx.param_env(src.def_id()).with_reveal_all();
+        let param_env = tcx.param_env_reveal_all_normalized(src.def_id());
         let move_data = match MoveData::gather_moves(body, tcx, param_env) {
             Ok(move_data) => move_data,
             Err((move_data, _)) => {

Diff for: src/librustc_mir/transform/inline.rs

@@ -72,7 +72,7 @@ impl Inliner<'tcx> {
 
         let mut callsites = VecDeque::new();
 
-        let param_env = self.tcx.param_env(self.source.def_id()).with_reveal_all();
+        let param_env = self.tcx.param_env_reveal_all_normalized(self.source.def_id());
 
         // Only do inlining into fn bodies.
         let id = self.tcx.hir().as_local_hir_id(self.source.def_id().expect_local());

Diff for: src/librustc_mir/transform/validate.rs

@@ -189,7 +189,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
         // Normalize projections and things like that.
         // FIXME: We need to reveal_all, as some optimizations change types in ways
         // that require unfolding opaque types.
-        let param_env = self.param_env.with_reveal_all();
+        let param_env = self.param_env.with_reveal_all_normalized(self.tcx);
         let src = self.tcx.normalize_erasing_regions(param_env, src);
         let dest = self.tcx.normalize_erasing_regions(param_env, dest);
 

Diff for: src/librustc_mir_build/hair/pattern/mod.rs

@@ -776,7 +776,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
 
         // Use `Reveal::All` here because patterns are always monomorphic even if their function
         // isn't.
-        let param_env_reveal_all = self.param_env.with_reveal_all();
+        let param_env_reveal_all = self.param_env.with_reveal_all_normalized(self.tcx);
         let substs = self.typeck_results.node_substs(id);
         let instance = match ty::Instance::resolve(self.tcx, param_env_reveal_all, def_id, substs) {
             Ok(Some(i)) => i,