Skip to content

Commit

Permalink
Auto merge of rust-lang#132418 - lcnr:prepare-for-eval, r=<try>
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
bors committed Oct 31, 2024
2 parents 20c909f + 780bd2f commit 8b79e04
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 54 deletions.
1 change: 0 additions & 1 deletion compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_const_eval/src/const_eval/valtrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
Expand Down
10 changes: 2 additions & 8 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_middle/src/mir/interpret/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_middle/src/ty/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
36 changes: 1 addition & 35 deletions compiler/rustc_middle/src/ty/consts/kind.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down

0 comments on commit 8b79e04

Please sign in to comment.