stop upgrading param envs to Reveal::All

Author: lcnr

compiler/rustc_const_eval/src/transform/validate.rs

@@ -2,6 +2,7 @@
 
 use rustc_data_structures::fx::FxHashSet;
 use rustc_index::bit_set::BitSet;
+use rustc_infer::infer::DefiningAnchor;
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_middle::mir::interpret::Scalar;
 use rustc_middle::mir::visit::NonUseContext::VarDebugInfo;
@@ -11,13 +12,14 @@ use rustc_middle::mir::{
     MirPass, MirPhase, Operand, Place, PlaceElem, PlaceRef, ProjectionElem, RuntimePhase, Rvalue,
     SourceScope, Statement, StatementKind, Terminator, TerminatorKind, UnOp, START_BLOCK,
 };
-use rustc_middle::ty::fold::BottomUpFolder;
+use rustc_middle::traits::ObligationCause;
 use rustc_middle::ty::subst::Subst;
-use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeFoldable, TypeVisitable};
+use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeVisitable};
 use rustc_mir_dataflow::impls::MaybeStorageLive;
 use rustc_mir_dataflow::storage::always_storage_live_locals;
 use rustc_mir_dataflow::{Analysis, ResultsCursor};
 use rustc_target::abi::{Size, VariantIdx};
+use rustc_trait_selection::traits::ObligationCtxt;
 
 #[derive(Copy, Clone, Debug)]
 enum EdgeKind {
@@ -87,24 +89,36 @@ pub fn equal_up_to_regions<'tcx>(
         return true;
     }
 
-    // Normalize lifetimes away on both sides, then compare.
-    let normalize = |ty: Ty<'tcx>| {
-        let ty = ty.fold_with(&mut BottomUpFolder {
-            tcx,
-            // FIXME: We erase all late-bound lifetimes, but this is not fully correct.
-            // If you have a type like `<for<'a> fn(&'a u32) as SomeTrait>::Assoc`,
-            // this is not necessarily equivalent to `<fn(&'static u32) as SomeTrait>::Assoc`,
-            // since one may have an `impl SomeTrait for fn(&32)` and
-            // `impl SomeTrait for fn(&'static u32)` at the same time which
-            // specify distinct values for Assoc. (See also #56105)
-            lt_op: |_| tcx.lifetimes.re_erased,
-            // Leave consts and types unchanged.
-            ct_op: |ct| ct,
-            ty_op: |ty| ty,
-        });
-        tcx.try_normalize_erasing_regions(param_env, ty).unwrap_or(ty)
-    };
-    tcx.infer_ctxt().enter(|infcx| infcx.can_eq(param_env, normalize(src), normalize(dest)).is_ok())
+    may_subtype_ignoring_regions(tcx, param_env, src, dest)
+        || may_subtype_ignoring_regions(tcx, param_env, dest, src)
+}
+
+fn may_subtype_ignoring_regions<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    param_env: ParamEnv<'tcx>,
+    src: Ty<'tcx>,
+    dest: Ty<'tcx>,
+) -> bool {
+    let mut builder =
+        tcx.infer_ctxt().ignoring_regions().with_opaque_type_inference(DefiningAnchor::Bubble);
+    builder.enter(|infcx| {
+        let ocx = ObligationCtxt::new(&infcx);
+        let cause = ObligationCause::dummy();
+        let src = ocx.normalize(cause.clone(), param_env, src);
+        let dest = ocx.normalize(cause.clone(), param_env, dest);
+        let Ok(infer_ok) = infcx.at(&cause, param_env).eq(src, dest) else {
+            return false;
+        };
+        let () = ocx.register_infer_ok_obligations(infer_ok);
+        let errors = ocx.select_all_or_error();
+        // With `Reveal::All`, opaque types get normalized away, with `Reveal::UserFacing`
+        // we would get unification errors because we're unable to look into opaque types,
+        // even if they're constrained in our current function.
+        //
+        // It seems very unlikely that this hides any bugs.
+        let _ = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types();
+        errors.is_empty()
+    })
 }
 
 struct TypeChecker<'a, 'tcx> {
@@ -187,16 +201,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
         // all normal lifetimes are erased, higher-ranked types with their
         // late-bound lifetimes are still around and can lead to type
         // differences. So we compare ignoring lifetimes.
-
-        // First, try with reveal_all. This might not work in some cases, as the predicates
-        // can be cleared in reveal_all mode. We try the reveal first anyways as it is used
-        // by some other passes like inlining as well.
-        let param_env = self.param_env.with_reveal_all_normalized(self.tcx);
-        if equal_up_to_regions(self.tcx, param_env, src, dest) {
-            return true;
-        }
-
-        // If this fails, we can try it without the reveal.
         equal_up_to_regions(self.tcx, self.param_env, src, dest)
     }
 }
@@ -283,7 +287,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                         this.fail(
                         location,
                         format!(
-                            "Field projection `{:?}.{:?}` specified type `{:?}`, but actual type is {:?}",
+                            "Field projection `{:?}.{:?}` specified type `{:?}`, but actual type is `{:?}`",
                             parent, f, ty, f_ty
                         )
                     )

compiler/rustc_middle/src/mir/interpret/queries.rs

@@ -20,7 +20,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_normalized(self);
+        let param_env = self.param_env_reveal_all_normalized(def_id);
         self.const_eval_global_id(param_env, cid, None)
     }
     /// Resolves and evaluates a constant.
@@ -184,8 +184,7 @@ impl<'tcx> TyCtxtEnsure<'tcx> {
         let substs = InternalSubsts::identity_for_item(self.tcx, def_id);
         let instance = ty::Instance::new(def_id, substs);
         let cid = GlobalId { instance, promoted: None };
-        let param_env =
-            self.tcx.param_env(def_id).with_reveal_all_normalized(self.tcx).with_const();
+        let param_env = self.tcx.param_env_reveal_all_normalized(def_id).with_const();
         // Const-eval shouldn't depend on lifetimes at all, so we can erase them, which should
         // improve caching of queries.
         let inputs = self.tcx.erase_regions(param_env.and(cid));

compiler/rustc_middle/src/mir/interpret/value.rs

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

compiler/rustc_middle/src/mir/mod.rs

@@ -2190,8 +2190,7 @@ impl<'tcx> ConstantKind<'tcx> {
             Self::Ty(ct) => ct.try_eval_bits(tcx, param_env, ty),
             Self::Val(val, t) => {
                 assert_eq!(*t, ty);
-                let size =
-                    tcx.layout_of(param_env.with_reveal_all_normalized(tcx).and(ty)).ok()?.size;
+                let size = tcx.layout_of(param_env.and(ty)).ok()?.size;
                 val.try_to_bits(size)
             }
         }

compiler/rustc_middle/src/query/mod.rs

@@ -1236,8 +1236,6 @@ rustc_queries! {
     }
 
     /// 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) }
     }

compiler/rustc_middle/src/traits/mod.rs

@@ -70,13 +70,17 @@ pub enum Reveal {
     /// Also, `impl Trait` is normalized to the concrete type,
     /// which has to be already collected by type-checking.
     ///
-    /// NOTE: as `impl Trait`'s concrete type should *never*
-    /// be observable directly by the user, `Reveal::All`
-    /// should not be used by checks which may expose
-    /// type equality or type contents to the user.
-    /// There are some exceptions, e.g., around auto traits and
-    /// transmute-checking, which expose some details, but
-    /// not the whole concrete type of the `impl Trait`.
+    /// **This should not be used at any point before borrowck**
+    ///
+    /// The concrete type of an opaque type should *never*
+    /// be observable directly by the user and doing so can cause
+    /// cycles if done before borrowck. Therefore, `Reveal::All`
+    /// should not be used by checks which may expose type equality
+    /// or type contents to the user.
+    ///
+    /// There are some places where we do observe some details about
+    /// the concrete type of opaque types, e.g., around auto traits and
+    /// transmute-checking, but these shouldn't rely on `Reveal::All`.
     All,
 }
 

compiler/rustc_middle/src/ty/consts.rs

@@ -250,7 +250,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_normalized(tcx).and(ty)).ok()?.size;
+        let size = tcx.layout_of(param_env.and(ty)).ok()?.size;
         // if `ty` does not depend on generic parameters, use an empty param_env
         self.kind().eval(tcx, param_env).try_to_bits(size)
     }

compiler/rustc_middle/src/ty/consts/kind.rs

@@ -183,16 +183,7 @@ impl<'tcx> ConstKind<'tcx> {
         if let ConstKind::Unevaluated(unevaluated) = self {
             use crate::mir::interpret::ErrorHandled;
 
-            // HACK(eddyb) this erases lifetimes even though `const_eval_resolve`
-            // also does later, but we want to do it before checking for
-            // inference variables.
-            // 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 = tcx
-                .erase_regions(param_env)
-                .with_reveal_all_normalized(tcx)
-                .and(tcx.erase_regions(unevaluated));
+            let param_env_and = tcx.erase_regions(param_env).and(tcx.erase_regions(unevaluated));
 
             // HACK(eddyb) when the query key would contain inference variables,
             // attempt using identity substs and `ParamEnv` instead, that will succeed

compiler/rustc_middle/src/ty/layout.rs

@@ -232,7 +232,6 @@ fn layout_of<'tcx>(
     let (param_env, ty) = query.into_parts();
     debug!(?ty);
 
-    let param_env = param_env.with_reveal_all_normalized(tcx);
     let unnormalized_ty = ty;
 
     // FIXME: We might want to have two different versions of `layout_of`:

compiler/rustc_middle/src/ty/mod.rs

@@ -1522,27 +1522,6 @@ impl<'tcx> ParamEnv<'tcx> {
         *self = self.with_constness(constness.and(self.constness()))
     }
 
-    /// Returns a new parameter environment with the same clauses, but
-    /// which "reveals" the true results of projections in all cases
-    /// (even for associated types that are specializable). This is
-    /// the desired behavior during codegen and certain other special
-    /// contexts; normally though we want to use `Reveal::UserFacing`,
-    /// which is the default.
-    /// 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.tag().reveal == traits::Reveal::All {
-            return self;
-        }
-
-        ParamEnv::new(
-            tcx.normalize_opaque_types(self.caller_bounds()),
-            Reveal::All,
-            self.constness(),
-        )
-    }
-
     /// Returns this same environment but with no caller bounds.
     #[inline]
     pub fn without_caller_bounds(self) -> Self {