Skip to content

do not eagerly use Reveal::All in const_eval_resolve_for_typeck #132418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anything that cares about infer vars should use infcx.try_const_eval_resolve :<

}

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
Loading