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

revert #75443, update mir validator #78410

Merged
merged 2 commits into from
Nov 8, 2020
Merged
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
99 changes: 24 additions & 75 deletions compiler/rustc_mir/src/transform/validate.rs
Original file line number Diff line number Diff line change
@@ -6,15 +6,16 @@ use crate::util::storage::AlwaysLiveLocals;

use super::MirPass;
use rustc_index::bit_set::BitSet;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::mir::interpret::Scalar;
use rustc_middle::mir::traversal;
use rustc_middle::mir::visit::{PlaceContext, Visitor};
use rustc_middle::mir::{
AggregateKind, BasicBlock, Body, BorrowKind, Local, Location, MirPhase, Operand, PlaceRef,
Rvalue, SourceScope, Statement, StatementKind, Terminator, TerminatorKind, VarDebugInfo,
};
use rustc_middle::ty::relate::{Relate, RelateResult, TypeRelation};
use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt};
use rustc_middle::ty::fold::BottomUpFolder;
use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt, TypeFoldable};
use rustc_target::abi::Size;

#[derive(Copy, Clone, Debug)]
@@ -77,79 +78,27 @@ pub fn equal_up_to_regions(
return true;
}

struct LifetimeIgnoreRelation<'tcx> {
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
}

impl TypeRelation<'tcx> for LifetimeIgnoreRelation<'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
}

fn param_env(&self) -> ty::ParamEnv<'tcx> {
self.param_env
}

fn tag(&self) -> &'static str {
"librustc_mir::transform::validate"
}

fn a_is_expected(&self) -> bool {
true
}

fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
_: ty::Variance,
a: T,
b: T,
) -> RelateResult<'tcx, T> {
// Ignore variance, require types to be exactly the same.
self.relate(a, b)
}

fn tys(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> {
if a == b {
// Short-circuit.
return Ok(a);
}
ty::relate::super_relate_tys(self, a, b)
}

fn regions(
&mut self,
a: ty::Region<'tcx>,
_b: ty::Region<'tcx>,
) -> RelateResult<'tcx, ty::Region<'tcx>> {
// Ignore regions.
Ok(a)
}

fn consts(
&mut self,
a: &'tcx ty::Const<'tcx>,
b: &'tcx ty::Const<'tcx>,
) -> RelateResult<'tcx, &'tcx ty::Const<'tcx>> {
ty::relate::super_relate_consts(self, a, b)
}

fn binders<T>(
&mut self,
a: ty::Binder<T>,
b: ty::Binder<T>,
) -> RelateResult<'tcx, ty::Binder<T>>
where
T: Relate<'tcx>,
{
self.relate(a.skip_binder(), b.skip_binder())?;
Ok(a)
}
}

// Instantiate and run relation.
let mut relator: LifetimeIgnoreRelation<'tcx> = LifetimeIgnoreRelation { tcx: tcx, param_env };
relator.relate(src, dest).is_ok()
// Normalize lifetimes away on both sides, then compare.
let param_env = param_env.with_reveal_all_normalized(tcx);
let normalize = |ty: Ty<'tcx>| {
tcx.normalize_erasing_regions(
param_env,
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.infer_ctxt().enter(|infcx| infcx.can_eq(param_env, normalize(src), normalize(dest)).is_ok())
Copy link
Member

Choose a reason for hiding this comment

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

This seems reasonable -- it is pretty close to what I had before we went for TypeRelation, except for this infcx.can_eq thing which I never saw before so I cannot comment on it.^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using normalize(src) == normalize(dest) would fail to unify 2 generic consts, so the following would fail mir validation

#[feature(const_generics, lazy_normalization_consts)]
fn test<const N: usize>() -> [u8; N + 1] {
    [0; N + 1]
}

Copy link
Member

@RalfJung RalfJung Oct 27, 2020

Choose a reason for hiding this comment

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

So generic consts are not interned the same way as other types? Or what does the difference stem from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the 2 N + 1 are two distinct mir bodies so we have ConstKind::Unevaluated(DefId1, substs: [N]) and ConstKind::Unevaluated(DefId2, substs: [N]) which should still be considered equal. This doesn't really exist for types afaik as we can partially normalize them.

We may want to change this in the future but I don't yet know how we would do that

Copy link
Contributor

Choose a reason for hiding this comment

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

"which would still be considered unequal", you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ConstKind::Unevaluated(DefId1, substs: [N]) and ConstKind::Unevaluated(DefId2, substs: [N]) are considered unequal when using Ty == Ty atm but are - and should be - considered equal with feature(const_evaluatable_checked).

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Yes, that makes sense. The point here is that normalize doesn't do any kind of evaluation of unevaluated constants, so simple equality is not especially meaningful.

}

struct TypeChecker<'a, 'tcx> {
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
@@ -338,7 +338,7 @@ impl<'a, 'b, 'tcx> TypeFolder<'tcx> for AssocTypeNormalizer<'a, 'b, 'tcx> {

let ty = ty.super_fold_with(self);
match *ty.kind() {
ty::Opaque(def_id, substs) => {
ty::Opaque(def_id, substs) if !substs.has_escaping_bound_vars() => {
// Only normalize `impl Trait` after type-checking, usually in codegen.
match self.param_env.reveal() {
Reveal::UserFacing => ty,
Original file line number Diff line number Diff line change
@@ -108,7 +108,7 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for QueryNormalizer<'cx, 'tcx> {

let ty = ty.super_fold_with(self);
let res = (|| match *ty.kind() {
ty::Opaque(def_id, substs) => {
ty::Opaque(def_id, substs) if !substs.has_escaping_bound_vars() => {
// Only normalize `impl Trait` after type-checking, usually in codegen.
match self.param_env.reveal() {
Reveal::UserFacing => ty,
27 changes: 0 additions & 27 deletions src/test/ui/type-alias-impl-trait/issue-72793.rs

This file was deleted.