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

Unify region variables when projecting associated types #73452

Merged
merged 3 commits into from
Jun 20, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
11 changes: 9 additions & 2 deletions src/librustc_infer/infer/canonical/canonicalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,18 +314,25 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for Canonicalizer<'cx, 'tcx> {
}

ty::ReVar(vid) => {
let r = self
let resolved_vid = self
.infcx
.unwrap()
.inner
.borrow_mut()
.unwrap_region_constraints()
.opportunistic_resolve_var(self.tcx, vid);
.opportunistic_resolve_var(vid);
debug!(
"canonical: region var found with vid {:?}, \
opportunistically resolved to {:?}",
vid, r
);
// micro-optimize -- avoid an interner look-up if the vid
// hasn't changed.
let r = if vid == resolved_vid {
r
} else {
self.tcx.mk_region(ty::ReVar(resolved_vid))
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: can we refactor with a helper like

self.tcx.reuse_or_mk_region(r, ty::ReVar(resolved_vid))

I guess it wouldn't be quite as fast. I'd like to see that pattern used in more places, though.

};
self.canonicalize_region_mode.canonicalize_free_region(self, r)
}

Expand Down
17 changes: 6 additions & 11 deletions src/librustc_infer/infer/region_constraints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ pub struct RegionConstraintStorage<'tcx> {
/// R1 <= R2 and R2 <= R1 and (b) we unify the two regions in this
/// table. You can then call `opportunistic_resolve_var` early
/// which will map R1 and R2 to some common region (i.e., either
/// R1 or R2). This is important when dropck and other such code
/// is iterating to a fixed point, because otherwise we sometimes
/// would wind up with a fresh stream of region variables that
/// have been equated but appear distinct.
/// R1 or R2). This is important when fulfillment, dropck and other such
/// code is iterating to a fixed point, because otherwise we sometimes
/// would wind up with a fresh stream of region variables that have been
/// equated but appear distinct.
pub(super) unification_table: ut::UnificationTableStorage<ty::RegionVid>,

/// a flag set to true when we perform any unifications; this is used
Expand Down Expand Up @@ -714,13 +714,8 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
}
}

pub fn opportunistic_resolve_var(
&mut self,
tcx: TyCtxt<'tcx>,
rid: RegionVid,
) -> ty::Region<'tcx> {
let vid = self.unification_table().probe_value(rid).min_vid;
tcx.mk_region(ty::ReVar(vid))
pub fn opportunistic_resolve_var(&mut self, rid: RegionVid) -> ty::RegionVid {
self.unification_table().probe_value(rid).min_vid
}

fn combine_map(&mut self, t: CombineMapType) -> &mut CombineMap<'tcx> {
Expand Down
43 changes: 24 additions & 19 deletions src/librustc_infer/infer/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,51 +46,56 @@ impl<'a, 'tcx> TypeFolder<'tcx> for OpportunisticVarResolver<'a, 'tcx> {
}
}

/// The opportunistic type and region resolver is similar to the
/// opportunistic type resolver, but also opportunistically resolves
/// regions. It is useful for canonicalization.
pub struct OpportunisticTypeAndRegionResolver<'a, 'tcx> {
/// The opportunistic region resolver opportunistically resolves regions
/// variables to the variable with the least variable id. It is used when
/// normlizing projections to avoid hitting the recursion limit by creating
/// many versions of a predicate for types that in the end have to unify.
///
/// If you want to resolve type and const variables as well, call
/// [InferCtxt::resolve_vars_if_possible] first.
pub struct OpportunisticRegionResolver<'a, 'tcx> {
infcx: &'a InferCtxt<'a, 'tcx>,
}

impl<'a, 'tcx> OpportunisticTypeAndRegionResolver<'a, 'tcx> {
impl<'a, 'tcx> OpportunisticRegionResolver<'a, 'tcx> {
pub fn new(infcx: &'a InferCtxt<'a, 'tcx>) -> Self {
OpportunisticTypeAndRegionResolver { infcx }
OpportunisticRegionResolver { infcx }
}
}

impl<'a, 'tcx> TypeFolder<'tcx> for OpportunisticTypeAndRegionResolver<'a, 'tcx> {
impl<'a, 'tcx> TypeFolder<'tcx> for OpportunisticRegionResolver<'a, 'tcx> {
fn tcx<'b>(&'b self) -> TyCtxt<'tcx> {
self.infcx.tcx
}

fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
if !t.needs_infer() {
if !t.has_infer_regions() {
t // micro-optimize -- if there is nothing in this type that this fold affects...
} else {
let t0 = self.infcx.shallow_resolve(t);
t0.super_fold_with(self)
t.super_fold_with(self)
}
}

fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> {
match *r {
ty::ReVar(rid) => self
.infcx
.inner
.borrow_mut()
.unwrap_region_constraints()
.opportunistic_resolve_var(self.tcx(), rid),
ty::ReVar(rid) => {
let resolved = self
.infcx
.inner
.borrow_mut()
.unwrap_region_constraints()
.opportunistic_resolve_var(rid);
if resolved == rid { r } else { self.tcx().mk_region(ty::ReVar(resolved)) }
}
_ => r,
}
}

fn fold_const(&mut self, ct: &'tcx ty::Const<'tcx>) -> &'tcx ty::Const<'tcx> {
if !ct.needs_infer() {
if !ct.has_infer_regions() {
ct // micro-optimize -- if there is nothing in this const that this fold affects...
} else {
let c0 = self.infcx.shallow_resolve(ct);
c0.super_fold_with(self)
ct.super_fold_with(self)
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_middle/ty/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ pub trait TypeFoldable<'tcx>: fmt::Debug + Clone {
fn has_param_types_or_consts(&self) -> bool {
self.has_type_flags(TypeFlags::HAS_TY_PARAM | TypeFlags::HAS_CT_PARAM)
}
fn has_infer_regions(&self) -> bool {
self.has_type_flags(TypeFlags::HAS_RE_INFER)
}
fn has_infer_types(&self) -> bool {
self.has_type_flags(TypeFlags::HAS_TY_INFER)
}
Expand Down
12 changes: 11 additions & 1 deletion src/librustc_trait_selection/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_errors::ErrorReported;
use rustc_hir::def_id::DefId;
use rustc_hir::lang_items::{FnOnceTraitLangItem, GeneratorTraitLangItem};
use rustc_infer::infer::resolve::OpportunisticRegionResolver;
use rustc_middle::ty::fold::{TypeFoldable, TypeFolder};
use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::util::IntTypeExt;
Expand Down Expand Up @@ -1146,7 +1147,7 @@ fn confirm_candidate<'cx, 'tcx>(
) -> Progress<'tcx> {
debug!("confirm_candidate(candidate={:?}, obligation={:?})", candidate, obligation);

match candidate {
let mut progress = match candidate {
ProjectionTyCandidate::ParamEnv(poly_projection)
| ProjectionTyCandidate::TraitDef(poly_projection) => {
confirm_param_env_candidate(selcx, obligation, poly_projection)
Expand All @@ -1155,7 +1156,16 @@ fn confirm_candidate<'cx, 'tcx>(
ProjectionTyCandidate::Select(impl_source) => {
confirm_select_candidate(selcx, obligation, obligation_trait_ref, impl_source)
}
};
// When checking for cycle during evaluation, we compare predicates with
// "syntactic" equality. Since normalization generally introduces a type
// with new region variables, we need to resolve them to existing variables
// when possible for this to work. See `auto-trait-projection-recursion.rs`
// for a case where this matters.
if progress.ty.has_infer_regions() {
progress.ty = OpportunisticRegionResolver::new(selcx.infcx()).fold_ty(progress.ty);
}
progress
}

fn confirm_select_candidate<'cx, 'tcx>(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0495]: cannot infer an appropriate lifetime due to conflicting requirements
--> $DIR/project-fn-ret-invariant.rs:48:8
--> $DIR/project-fn-ret-invariant.rs:48:4
|
LL | bar(foo, x)
| ^^^
| ^^^^^^^^^^^
|
note: first, the lifetime cannot outlive the lifetime `'a` as defined on the function body at 44:8...
--> $DIR/project-fn-ret-invariant.rs:44:8
Expand Down
34 changes: 34 additions & 0 deletions src/test/ui/auto-traits/auto-trait-projection-recursion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Checking the `Send` bound in `main` requires:
//
// checking <C<'static> as Y>::P: Send
// which normalizes to Box<X<C<'?1>>>: Send
// which needs X<C<'?1>>: Send
// which needs <C<'?1> as Y>::P: Send
//
// At this point we used to normalize the predicate to `Box<X<C<'?2>>>: Send`
// and continue in a loop where we created new region variables to the
// recursion limit. To avoid this we now "canonicalize" region variables to
// lowest unified region vid. This means we instead have to prove
// `Box<X<C<'?1>>>: Send`, which we can because auto traits are coinductive.

// check-pass

// Avoid a really long error message if this regresses.
#![recursion_limit="20"]

trait Y {
type P;
}

impl<'a> Y for C<'a> {
type P = Box<X<C<'a>>>;
}

struct C<'a>(&'a ());
struct X<T: Y>(T::P);

fn is_send<S: Send>() {}

fn main() {
is_send::<X<C<'static>>>();
}
30 changes: 30 additions & 0 deletions src/test/ui/traits/traits-inductive-overflow-lifetime.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Test that we don't hit the recursion limit for short cycles involving lifetimes.

// Shouldn't hit this, we should realize that we're in a cycle sooner.
#![recursion_limit="20"]

trait NotAuto {}
trait Y {
type P;
}

impl<'a> Y for C<'a> {
type P = Box<X<C<'a>>>;
}

struct C<'a>(&'a ());
struct X<T: Y>(T::P);

impl<T: NotAuto> NotAuto for Box<T> {}
impl<T: Y> NotAuto for X<T> where T::P: NotAuto {}
impl<'a> NotAuto for C<'a> {}

fn is_send<S: NotAuto>() {}
//~^ NOTE: required

fn main() {
// Should only be a few notes.
is_send::<X<C<'static>>>();
//~^ ERROR overflow evaluating
//~| NOTE: required
}
14 changes: 14 additions & 0 deletions src/test/ui/traits/traits-inductive-overflow-lifetime.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0275]: overflow evaluating the requirement `std::boxed::Box<X<C<'_>>>: NotAuto`
--> $DIR/traits-inductive-overflow-lifetime.rs:27:5
|
LL | fn is_send<S: NotAuto>() {}
| ------- required by this bound in `is_send`
...
LL | is_send::<X<C<'static>>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: required because of the requirements on the impl of `NotAuto` for `X<C<'static>>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0275`.