Skip to content
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

Add eval hack in super_relate_consts back #103279

Merged
merged 2 commits into from
Oct 26, 2022
Merged
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
9 changes: 8 additions & 1 deletion compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
val: &mir::ConstantKind<'tcx>,
layout: Option<TyAndLayout<'tcx>>,
) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
// FIXME(const_prop): normalization needed b/c const prop lint in
// `mir_drops_elaborated_and_const_checked`, which happens before
// optimized MIR. Only after optimizing the MIR can we guarantee
// that the `RevealAll` pass has happened and that the body's consts
// are normalized, so any call to resolve before that needs to be
// manually normalized.
let val = self.tcx.normalize_erasing_regions(self.param_env, *val);
match val {
mir::ConstantKind::Ty(ct) => {
match ct.kind() {
Expand Down Expand Up @@ -585,7 +592,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}
}
mir::ConstantKind::Val(val, ty) => self.const_val_to_op(*val, *ty, layout),
mir::ConstantKind::Val(val, ty) => self.const_val_to_op(val, ty, layout),
mir::ConstantKind::Unevaluated(uv, _) => {
let instance = self.resolve(uv.def, uv.substs)?;
Ok(self.eval_to_allocation(GlobalId { instance, promoted: uv.promoted })?.into())
Expand Down
15 changes: 13 additions & 2 deletions compiler/rustc_middle/src/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,8 +574,8 @@ pub fn super_relate_tys<'tcx, R: TypeRelation<'tcx>>(
/// it.
pub fn super_relate_consts<'tcx, R: TypeRelation<'tcx>>(
relation: &mut R,
a: ty::Const<'tcx>,
b: ty::Const<'tcx>,
mut a: ty::Const<'tcx>,
mut b: ty::Const<'tcx>,
) -> RelateResult<'tcx, ty::Const<'tcx>> {
debug!("{}.super_relate_consts(a = {:?}, b = {:?})", relation.tag(), a, b);
let tcx = relation.tcx();
Expand All @@ -596,6 +596,17 @@ pub fn super_relate_consts<'tcx, R: TypeRelation<'tcx>>(
);
}

// HACK(const_generics): We still need to eagerly evaluate consts when
// relating them because during `normalize_param_env_or_error`,
// we may relate an evaluated constant in a obligation against
// an unnormalized (i.e. unevaluated) const in the param-env.
// FIXME(generic_const_exprs): Once we always lazily unify unevaluated constants
// these `eval` calls can be removed.
if !relation.tcx().features().generic_const_exprs {
a = a.eval(tcx, relation.param_env());
b = b.eval(tcx, relation.param_env());
}

// Currently, the values that can be unified are primitive types,
// and those that derive both `PartialEq` and `Eq`, corresponding
// to structural-match types.
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_mir_build/src/thir/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,12 @@ 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_normalized(self.tcx);
let substs = self.typeck_results.node_substs(id);
// N.B. There is no guarantee that substs collected in typeck results are fully normalized,
// so they need to be normalized in order to pass to `Instance::resolve`, which will ICE
// if given unnormalized types.
let substs = self
.tcx
.normalize_erasing_regions(param_env_reveal_all, self.typeck_results.node_substs(id));
Copy link
Contributor

@lcnr lcnr Oct 20, 2022

Choose a reason for hiding this comment

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

that normalize call seems correct and useful. Considering that we want to backport this, is this call necessary to fix these regressions? I guess I am fine with keeping this even if not 🤷

let instance = match ty::Instance::resolve(self.tcx, param_env_reveal_all, def_id, substs) {
Ok(Some(i)) => i,
Ok(None) => {
Expand Down
31 changes: 31 additions & 0 deletions src/test/ui/consts/unnormalized-param-env.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// check-pass

pub trait CSpace<const N: usize> {
type Traj;
}

pub struct Const<const R: usize>;

pub trait Obstacle<CS, const N: usize> {
fn trajectory_free<FT, S1>(&self, t: &FT)
where
CS::Traj: Sized,
CS: CSpace<N>;
}

// -----

const N: usize = 4;

struct ObstacleSpace2df32;

impl<CS> Obstacle<CS, N> for ObstacleSpace2df32 {
fn trajectory_free<TF, S1>(&self, t: &TF)
where
CS::Traj: Sized,
CS: CSpace<N>,
{
}
}

fn main() {}