Skip to content

Commit

Permalink
Auto merge of #58592 - nikomatsakis:universe-leak-check, r=<try>
Browse files Browse the repository at this point in the history
[WIP] Re-implement leak check in terms of universes

This PR temporarily restores the leak-check, but implemented in terms of universes. This is not because the leak check behavior was necessarily **correct**, but because (a) we may want to have a transition period and because (b) we want to have more breathing room to work through the full implications of handling higher-ranked types correctly. Note that this PR builds atop #58056.

Fixes #58451
Fixes #46989
Fixes #57639

r? @aturon
cc @arielb1, @lqd

~~Temporary note: I've not finished running `./x.py test` locally -- I'm confident a lot of error messages in tests will need updating. I sort of expect them to revert to the older, (imo) less good error messages, which is mildly unfortunate. There might be a way to preserve the new error messages, not sure.~~
  • Loading branch information
bors committed Feb 20, 2019
2 parents f66e469 + f156ac2 commit c3b2220
Show file tree
Hide file tree
Showing 93 changed files with 1,389 additions and 415 deletions.
71 changes: 51 additions & 20 deletions src/librustc/infer/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,10 +255,24 @@ impl<'infcx, 'gcx, 'tcx> CombineFields<'infcx, 'gcx, 'tcx> {
RelationDir::SupertypeOf => ty::Contravariant,
};

debug!("generalize: ambient_variance = {:?}", ambient_variance);

let for_universe = match self.infcx.type_variables.borrow_mut().probe(for_vid) {
v @ TypeVariableValue::Known { .. } => panic!(
"instantiating {:?} which has a known value {:?}",
for_vid,
v,
),
TypeVariableValue::Unknown { universe } => universe,
};

debug!("generalize: for_universe = {:?}", for_universe);

let mut generalize = Generalizer {
infcx: self.infcx,
span: self.trace.cause.span,
for_vid_sub_root: self.infcx.type_variables.borrow_mut().sub_root_var(for_vid),
for_universe,
ambient_variance,
needs_wf: false,
root_ty: ty,
Expand Down Expand Up @@ -288,6 +302,11 @@ struct Generalizer<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> {
/// that means we would have created a cyclic type.
for_vid_sub_root: ty::TyVid,

/// The universe of the type variable that is in the process of
/// being instantiated. Any fresh variables that we create in this
/// process should be in that same universe.
for_universe: ty::UniverseIndex,

/// Track the variance as we descend into the type.
ambient_variance: ty::Variance,

Expand Down Expand Up @@ -386,6 +405,8 @@ impl<'cx, 'gcx, 'tcx> TypeRelation<'cx, 'gcx, 'tcx> for Generalizer<'cx, 'gcx, '
fn tys(&mut self, t: Ty<'tcx>, t2: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> {
assert_eq!(t, t2); // we are abusing TypeRelation here; both LHS and RHS ought to be ==

debug!("generalize: t={:?}", t);

// Check to see whether the type we are genealizing references
// any other type variable related to `vid` via
// subtyping. This is basically our "occurs check", preventing
Expand All @@ -403,12 +424,17 @@ impl<'cx, 'gcx, 'tcx> TypeRelation<'cx, 'gcx, 'tcx> for Generalizer<'cx, 'gcx, '
match variables.probe(vid) {
TypeVariableValue::Known { value: u } => {
drop(variables);
debug!("generalize: known value {:?}", u);
self.relate(&u, &u)
}
TypeVariableValue::Unknown { universe } => {
match self.ambient_variance {
// Invariant: no need to make a fresh type variable.
ty::Invariant => return Ok(t),
ty::Invariant => {
if self.for_universe.can_name(universe) {
return Ok(t);
}
}

// Bivariant: make a fresh var, but we
// may need a WF predicate. See
Expand All @@ -422,7 +448,7 @@ impl<'cx, 'gcx, 'tcx> TypeRelation<'cx, 'gcx, 'tcx> for Generalizer<'cx, 'gcx, '
}

let origin = *variables.var_origin(vid);
let new_var_id = variables.new_var(universe, false, origin);
let new_var_id = variables.new_var(self.for_universe, false, origin);
let u = self.tcx().mk_var(new_var_id);
debug!("generalize: replacing original vid={:?} with new={:?}",
vid, u);
Expand All @@ -448,6 +474,8 @@ impl<'cx, 'gcx, 'tcx> TypeRelation<'cx, 'gcx, 'tcx> for Generalizer<'cx, 'gcx, '
-> RelateResult<'tcx, ty::Region<'tcx>> {
assert_eq!(r, r2); // we are abusing TypeRelation here; both LHS and RHS ought to be ==

debug!("generalize: regions r={:?}", r);

match *r {
// Never make variables for regions bound within the type itself,
// nor for erased regions.
Expand All @@ -456,37 +484,40 @@ impl<'cx, 'gcx, 'tcx> TypeRelation<'cx, 'gcx, 'tcx> for Generalizer<'cx, 'gcx, '
return Ok(r);
}

// Always make a fresh region variable for placeholder
// regions; the higher-ranked decision procedures rely on
// this.
ty::RePlaceholder(..) => { }
ty::ReClosureBound(..) => {
span_bug!(
self.span,
"encountered unexpected ReClosureBound: {:?}",
r,
);
}

// For anything else, we make a region variable, unless we
// are *equating*, in which case it's just wasteful.
ty::RePlaceholder(..) |
ty::ReVar(..) |
ty::ReEmpty |
ty::ReStatic |
ty::ReScope(..) |
ty::ReVar(..) |
ty::ReEarlyBound(..) |
ty::ReFree(..) => {
match self.ambient_variance {
ty::Invariant => return Ok(r),
ty::Bivariant | ty::Covariant | ty::Contravariant => (),
}
// see common code below
}
}

ty::ReClosureBound(..) => {
span_bug!(
self.span,
"encountered unexpected ReClosureBound: {:?}",
r,
);
// If we are in an invariant context, we can re-use the region
// as is, unless it happens to be in some universe that we
// can't name. (In the case of a region *variable*, we could
// use it if we promoted it into our universe, but we don't
// bother.)
if let ty::Invariant = self.ambient_variance {
let r_universe = self.infcx.universe_of_region(r);
if self.for_universe.can_name(r_universe) {
return Ok(r);
}
}

// FIXME: This is non-ideal because we don't give a
// very descriptive origin for this region variable.
Ok(self.infcx.next_region_var(MiscVariable(self.span)))
Ok(self.infcx.next_region_var_in_universe(MiscVariable(self.span), self.for_universe))
}
}

Expand Down
63 changes: 41 additions & 22 deletions src/librustc/infer/higher_ranked/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use super::combine::CombineFields;
use super::{HigherRankedType, InferCtxt, PlaceholderMap};

use crate::infer::CombinedSnapshot;
use crate::ty::relate::{Relate, RelateResult, TypeRelation};
use crate::ty::{self, Binder, TypeFoldable};

Expand All @@ -29,27 +30,32 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {

let span = self.trace.cause.span;

// First, we instantiate each bound region in the supertype with a
// fresh placeholder region.
let (b_prime, _) = self.infcx.replace_bound_vars_with_placeholders(b);
return self.infcx.commit_if_ok(|snapshot| {
// First, we instantiate each bound region in the supertype with a
// fresh placeholder region.
let (b_prime, placeholder_map) = self.infcx.replace_bound_vars_with_placeholders(b);

// Next, we instantiate each bound region in the subtype
// with a fresh region variable. These region variables --
// but no other pre-existing region variables -- can name
// the placeholders.
let (a_prime, _) =
self.infcx
.replace_bound_vars_with_fresh_vars(span, HigherRankedType, a);
// Next, we instantiate each bound region in the subtype
// with a fresh region variable. These region variables --
// but no other pre-existing region variables -- can name
// the placeholders.
let (a_prime, _) =
self.infcx
.replace_bound_vars_with_fresh_vars(span, HigherRankedType, a);

debug!("a_prime={:?}", a_prime);
debug!("b_prime={:?}", b_prime);

debug!("a_prime={:?}", a_prime);
debug!("b_prime={:?}", b_prime);
// Compare types now that bound regions have been replaced.
let result = self.sub(a_is_expected).relate(&a_prime, &b_prime)?;

// Compare types now that bound regions have been replaced.
let result = self.sub(a_is_expected).relate(&a_prime, &b_prime)?;
self.infcx
.leak_check(!a_is_expected, &placeholder_map, snapshot)?;

debug!("higher_ranked_sub: OK result={:?}", result);
debug!("higher_ranked_sub: OK result={:?}", result);

Ok(ty::Binder::bind(result))
Ok(ty::Binder::bind(result))
});
}
}

Expand All @@ -72,10 +78,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
/// [rustc guide]: https://rust-lang.github.io/rustc-guide/traits/hrtb.html
pub fn replace_bound_vars_with_placeholders<T>(
&self,
binder: &ty::Binder<T>
binder: &ty::Binder<T>,
) -> (T, PlaceholderMap<'tcx>)
where
T: TypeFoldable<'tcx>
T: TypeFoldable<'tcx>,
{
let next_universe = self.create_next_universe();

Expand All @@ -96,12 +102,25 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
let (result, map) = self.tcx.replace_bound_vars(binder, fld_r, fld_t);

debug!(
"replace_bound_vars_with_placeholders(binder={:?}, result={:?}, map={:?})",
binder,
result,
map
"replace_bound_vars_with_placeholders(\
next_universe={:?}, \
binder={:?}, \
result={:?}, \
map={:?})",
next_universe, binder, result, map,
);

(result, map)
}

/// See `infer::region_constraints::RegionConstraintCollector::leak_check`.
pub fn leak_check(
&self,
overly_polymorphic: bool,
placeholder_map: &PlaceholderMap<'tcx>,
snapshot: &CombinedSnapshot<'_, 'tcx>,
) -> RelateResult<'tcx, ()> {
self.borrow_region_constraints()
.leak_check(self.tcx, overly_polymorphic, placeholder_map, snapshot)
}
}
59 changes: 40 additions & 19 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -937,32 +937,41 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
return None;
}

let (
ty::SubtypePredicate {
a_is_expected,
a,
b,
},
_,
) = self.replace_bound_vars_with_placeholders(predicate);
Some(self.commit_if_ok(|snapshot| {
let (
ty::SubtypePredicate {
a_is_expected,
a,
b,
},
placeholder_map,
) = self.replace_bound_vars_with_placeholders(predicate);

Some(
self.at(cause, param_env)
.sub_exp(a_is_expected, a, b)
.map(|ok| ok.unit()),
)
let ok = self.at(cause, param_env)
.sub_exp(a_is_expected, a, b)?;

self.leak_check(false, &placeholder_map, snapshot)?;

Ok(ok.unit())
}))
}

pub fn region_outlives_predicate(
&self,
cause: &traits::ObligationCause<'tcx>,
predicate: &ty::PolyRegionOutlivesPredicate<'tcx>,
) {
let (ty::OutlivesPredicate(r_a, r_b), _) =
self.replace_bound_vars_with_placeholders(predicate);
let origin =
SubregionOrigin::from_obligation_cause(cause, || RelateRegionParamBound(cause.span));
self.sub_regions(origin, r_b, r_a); // `b : a` ==> `a <= b`
) -> UnitResult<'tcx> {
self.commit_if_ok(|snapshot| {
let (ty::OutlivesPredicate(r_a, r_b), placeholder_map) =
self.replace_bound_vars_with_placeholders(predicate);
let origin = SubregionOrigin::from_obligation_cause(
cause,
|| RelateRegionParamBound(cause.span),
);
self.sub_regions(origin, r_b, r_a); // `b : a` ==> `a <= b`
self.leak_check(false, &placeholder_map, snapshot)?;
Ok(())
})
}

pub fn next_ty_var_id(&self, diverging: bool, origin: TypeVariableOrigin) -> TyVid {
Expand Down Expand Up @@ -1018,6 +1027,18 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
self.tcx.mk_region(ty::ReVar(region_var))
}

/// Return the universe that the region `r` was created in. For
/// most regions (e.g., `'static`, named regions from the user,
/// etc) this is the root universe U0. For inference variables or
/// placeholders, however, it will return the universe which which
/// they are associated.
fn universe_of_region(
&self,
r: ty::Region<'tcx>,
) -> ty::UniverseIndex {
self.borrow_region_constraints().universe(r)
}

/// Number of region variables created so far.
pub fn num_region_vars(&self) -> usize {
self.borrow_region_constraints().num_region_vars()
Expand Down
Loading

0 comments on commit c3b2220

Please sign in to comment.