Auto merge of rust-lang#132418 - lcnr:prepare-for-eval, r=<try>

do not eagerly use `Reveal::All` in `const_eval_resolve_for_typeck`

We should not use `RevealAll` when resolving the instance; only when evaluating its body.

r? `@BoxyUwU`

Author: bors

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -348,7 +348,6 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
     // Const eval always happens in Reveal::All mode in order to be able to use the hidden types of
     // opaque types. This is needed for trivial things like `size_of`, but also for using associated
     // types that are not specified in the opaque type.
-
     assert_eq!(key.param_env.reveal(), Reveal::All);
     if cfg!(debug_assertions) {
         // Make sure we format the instance even if we do not print it.

compiler/rustc_const_eval/src/const_eval/valtrees.rs

@@ -2,6 +2,7 @@ use rustc_abi::{BackendRepr, VariantIdx};
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_middle::mir::interpret::{EvalToValTreeResult, GlobalId};
 use rustc_middle::ty::layout::{LayoutCx, LayoutOf, TyAndLayout};
+use rustc_middle::ty::solve::Reveal;
 use rustc_middle::ty::{self, ScalarInt, Ty, TyCtxt};
 use rustc_middle::{bug, mir};
 use rustc_span::DUMMY_SP;
@@ -231,6 +232,7 @@ pub(crate) fn eval_to_valtree<'tcx>(
     param_env: ty::ParamEnv<'tcx>,
     cid: GlobalId<'tcx>,
 ) -> EvalToValTreeResult<'tcx> {
+    debug_assert_eq!(param_env.reveal(), Reveal::All);
     let const_alloc = tcx.eval_to_allocation_raw(param_env.and(cid))?;
 
     // FIXME Need to provide a span to `eval_to_valtree`

compiler/rustc_infer/src/infer/mod.rs

@@ -1345,16 +1345,10 @@ impl<'tcx> InferCtxt<'tcx> {
             }
         }
 
-        let param_env_erased = tcx.erase_regions(param_env);
-        let args_erased = tcx.erase_regions(args);
-        debug!(?param_env_erased);
-        debug!(?args_erased);
-
-        let unevaluated = ty::UnevaluatedConst { def: unevaluated.def, args: args_erased };
-
+        let unevaluated = ty::UnevaluatedConst { def: unevaluated.def, args };
         // The return value is the evaluated value which doesn't contain any reference to inference
         // variables, thus we don't need to instantiate back the original values.
-        tcx.const_eval_resolve_for_typeck(param_env_erased, unevaluated, span)
+        tcx.const_eval_resolve_for_typeck(param_env, unevaluated, span)
     }
 
     /// The returned function is used in a fast path. If it returns `true` the variable is

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

@@ -41,7 +41,7 @@ impl<'tcx> TyCtxt<'tcx> {
         let args = GenericArgs::identity_for_item(self, def_id);
         let instance = ty::Instance::new(def_id, args);
         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);
         let inputs = self.erase_regions(param_env.and(cid));
         self.eval_to_allocation_raw(inputs)
     }
@@ -72,11 +72,7 @@ impl<'tcx> TyCtxt<'tcx> {
             bug!("did not expect inference variables here");
         }
 
-        match ty::Instance::try_resolve(
-            self, param_env,
-            // FIXME: maybe have a separate version for resolving mir::UnevaluatedConst?
-            ct.def, ct.args,
-        ) {
+        match ty::Instance::try_resolve(self, param_env, ct.def, ct.args) {
             Ok(Some(instance)) => {
                 let cid = GlobalId { instance, promoted: ct.promoted };
                 self.const_eval_global_id(param_env, cid, span)
@@ -108,6 +104,10 @@ impl<'tcx> TyCtxt<'tcx> {
         match ty::Instance::try_resolve(self, param_env, ct.def, ct.args) {
             Ok(Some(instance)) => {
                 let cid = GlobalId { instance, promoted: None };
+                // We always evaluate with `Reveal::All` as evaluation should be able
+                // to observe the hidden types of opaques. Note that we've still succeeded to
+                // typeck the body of the constant using `Reveal::UserFacing`.
+                let param_env = self.erase_regions(param_env).with_reveal_all_normalized(self);
                 self.const_eval_global_id_for_typeck(param_env, cid, span).inspect(|_| {
                     // We are emitting the lint here instead of in `is_const_evaluatable`
                     // as we normalize obligations before checking them, and normalization

compiler/rustc_middle/src/ty/consts.rs

@@ -376,11 +376,14 @@ impl<'tcx> Const<'tcx> {
         assert!(!self.has_escaping_bound_vars(), "escaping vars in {self:?}");
         match self.kind() {
             ConstKind::Unevaluated(unevaluated) => {
-                // FIXME(eddyb) maybe the `const_eval_*` methods should take
-                // `ty::ParamEnvAnd` instead of having them separate.
-                let (param_env, unevaluated) = unevaluated.prepare_for_eval(tcx, param_env);
-                // try to resolve e.g. associated constants to their definition on an impl, and then
+                // Try to resolve e.g. associated constants to their definition on an impl, and then
                 // evaluate the const.
+                //
+                // In case the query key would contain unconstrained inference variables, we bail instead.
+                if (unevaluated, param_env).has_non_region_infer() {
+                    return Err(Either::Right(ErrorHandled::TooGeneric(span)));
+                }
+
                 match tcx.const_eval_resolve_for_typeck(param_env, unevaluated, span) {
                     Ok(Ok(c)) => {
                         Ok((tcx.type_of(unevaluated.def).instantiate(tcx, unevaluated.args), c))

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

@@ -1,46 +1,12 @@
 use std::assert_matches::assert_matches;
 
-use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisitable, extension};
+use rustc_macros::{HashStable, TyDecodable, TyEncodable, TypeFoldable, TypeVisitable};
 
 use super::Const;
 use crate::mir;
 use crate::ty::abstract_const::CastKind;
-use crate::ty::visit::TypeVisitableExt as _;
 use crate::ty::{self, Ty, TyCtxt};
 
-#[extension(pub(crate) trait UnevaluatedConstEvalExt<'tcx>)]
-impl<'tcx> ty::UnevaluatedConst<'tcx> {
-    /// FIXME(RalfJung): I cannot explain what this does or why it makes sense, but not doing this
-    /// hurts performance.
-    #[inline]
-    fn prepare_for_eval(
-        self,
-        tcx: TyCtxt<'tcx>,
-        param_env: ty::ParamEnv<'tcx>,
-    ) -> (ty::ParamEnv<'tcx>, Self) {
-        // 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.
-
-        // HACK(eddyb) when the query key would contain inference variables,
-        // attempt using identity args and `ParamEnv` instead, that will succeed
-        // when the expression doesn't depend on any parameters.
-        // FIXME(eddyb, skinny121) pass `InferCtxt` into here when it's available, so that
-        // we can call `infcx.const_eval_resolve` which handles inference variables.
-        if (param_env, self).has_non_region_infer() {
-            (tcx.param_env(self.def), ty::UnevaluatedConst {
-                def: self.def,
-                args: ty::GenericArgs::identity_for_item(tcx, self.def),
-            })
-        } else {
-            (tcx.erase_regions(param_env).with_reveal_all_normalized(tcx), tcx.erase_regions(self))
-        }
-    }
-}
-
 #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)]
 #[derive(HashStable, TyEncodable, TyDecodable, TypeVisitable, TypeFoldable)]
 pub enum ExprKind {