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

Re-implement leak check in terms of universes #58592

Merged
merged 7 commits into from
Feb 22, 2019
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
66 changes: 40 additions & 26 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 @@ -97,16 +103,24 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {

debug!(
"replace_bound_vars_with_placeholders(\
next_universe={:?}, \
binder={:?}, \
result={:?}, \
map={:?})",
next_universe,
binder,
result,
map,
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)
}
}
47 changes: 28 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
174 changes: 174 additions & 0 deletions src/librustc/infer/region_constraints/leak_check.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
use super::*;
use crate::infer::{CombinedSnapshot, PlaceholderMap};
use crate::ty::error::TypeError;
use crate::ty::relate::RelateResult;

impl<'tcx> RegionConstraintCollector<'tcx> {
/// Searches region constraints created since `snapshot` that
/// affect one of the placeholders in `placeholder_map`, returning
/// an error if any of the placeholders are related to another
/// placeholder or would have to escape into some parent universe
/// that cannot name them.
///
/// This is a temporary backwards compatibility measure to try and
/// retain the older (arguably incorrect) behavior of the
/// compiler.
///
/// NB. The use of snapshot here is mostly an efficiency thing --
/// we could search *all* region constraints, but that'd be a
/// bigger set and the data structures are not setup for that. If
/// we wind up keeping some form of this check long term, it would
/// probably be better to remove the snapshot parameter and to
/// refactor the constraint set.
pub fn leak_check(
&mut self,
tcx: TyCtxt<'_, '_, 'tcx>,
overly_polymorphic: bool,
placeholder_map: &PlaceholderMap<'tcx>,
_snapshot: &CombinedSnapshot<'_, 'tcx>,
Copy link
Member

Choose a reason for hiding this comment

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

Just to confirm: this parameter is basically acting as a static check that we're in the scope of a snapshot?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirm, I should perhaps write that more explicitly as a comment.

) -> RelateResult<'tcx, ()> {
debug!("leak_check(placeholders={:?})", placeholder_map);

assert!(self.in_snapshot());

// If the user gave `-Zno-leak-check`, then skip the leak
// check completely. This is wildly unsound and also not
// unlikely to cause an ICE or two. It is intended for use
// only during a transition period, in which the MIR typeck
// uses the "universe-style" check, and the rest of typeck
// uses the more conservative leak check. Since the leak
// check is more conservative, we can't test the
// universe-style check without disabling it.
if tcx.sess.opts.debugging_opts.no_leak_check {
return Ok(());
}

// Go through each placeholder that we created.
for (_, &placeholder_region) in placeholder_map {
// Find the universe this placeholder inhabits.
let placeholder = match placeholder_region {
ty::RePlaceholder(p) => p,
_ => bug!(
"leak_check: expected placeholder found {:?}",
placeholder_region,
),
};

// Find all regions that are related to this placeholder
// in some way. This means any region that either outlives
// or is outlived by a placeholder.
let mut taint_set = TaintSet::new(
TaintDirections::both(),
placeholder_region,
);
taint_set.fixed_point(tcx, &self.undo_log, &self.data.verifys);
let tainted_regions = taint_set.into_set();

// Report an error if two placeholders in the same universe
// are related to one another, or if a placeholder is related
// to something from a parent universe.
for &tainted_region in &tainted_regions {
if let ty::RePlaceholder(_) = tainted_region {
// Two placeholders cannot be related:
if tainted_region == placeholder_region {
continue;
}
} else if self.universe(tainted_region).can_name(placeholder.universe) {
continue;
}

return Err(if overly_polymorphic {
debug!("Overly polymorphic!");
TypeError::RegionsOverlyPolymorphic(placeholder.name, tainted_region)
} else {
debug!("Not as polymorphic!");
TypeError::RegionsInsufficientlyPolymorphic(placeholder.name, tainted_region)
});
}
}

Ok(())
}
}

#[derive(Debug)]
struct TaintSet<'tcx> {
directions: TaintDirections,
regions: FxHashSet<ty::Region<'tcx>>,
}

impl<'tcx> TaintSet<'tcx> {
fn new(directions: TaintDirections, initial_region: ty::Region<'tcx>) -> Self {
let mut regions = FxHashSet::default();
regions.insert(initial_region);
TaintSet {
directions: directions,
regions: regions,
}
}

fn fixed_point(
&mut self,
tcx: TyCtxt<'_, '_, 'tcx>,
undo_log: &[UndoLog<'tcx>],
verifys: &[Verify<'tcx>],
) {
let mut prev_len = 0;
while prev_len < self.len() {
debug!(
"tainted: prev_len = {:?} new_len = {:?}",
prev_len,
self.len()
);

prev_len = self.len();

for undo_entry in undo_log {
match undo_entry {
&AddConstraint(Constraint::VarSubVar(a, b)) => {
self.add_edge(tcx.mk_region(ReVar(a)), tcx.mk_region(ReVar(b)));
}
&AddConstraint(Constraint::RegSubVar(a, b)) => {
self.add_edge(a, tcx.mk_region(ReVar(b)));
}
&AddConstraint(Constraint::VarSubReg(a, b)) => {
self.add_edge(tcx.mk_region(ReVar(a)), b);
}
&AddConstraint(Constraint::RegSubReg(a, b)) => {
self.add_edge(a, b);
}
&AddGiven(a, b) => {
self.add_edge(a, tcx.mk_region(ReVar(b)));
}
&AddVerify(i) => span_bug!(
verifys[i].origin.span(),
"we never add verifications while doing higher-ranked things",
),
&Purged | &AddCombination(..) | &AddVar(..) => {}
}
}
}
}

fn into_set(self) -> FxHashSet<ty::Region<'tcx>> {
self.regions
}

fn len(&self) -> usize {
self.regions.len()
}

fn add_edge(&mut self, source: ty::Region<'tcx>, target: ty::Region<'tcx>) {
if self.directions.incoming {
if self.regions.contains(&target) {
self.regions.insert(source);
}
}

if self.directions.outgoing {
if self.regions.contains(&source) {
self.regions.insert(target);
}
}
}
}
2 changes: 2 additions & 0 deletions src/librustc/infer/region_constraints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ use crate::ty::{Region, RegionVid};
use std::collections::BTreeMap;
use std::{cmp, fmt, mem, u32};

mod leak_check;

#[derive(Default)]
pub struct RegionConstraintCollector<'tcx> {
/// For each `RegionVid`, the corresponding `RegionVariableOrigin`.
Expand Down
8 changes: 7 additions & 1 deletion src/librustc/traits/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,13 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
}
}
&ty::Predicate::RegionOutlives(ref binder) => {
let () = select.infcx().region_outlives_predicate(&dummy_cause, binder);
if select
.infcx()
.region_outlives_predicate(&dummy_cause, binder)
.is_err()
{
return false;
}
}
&ty::Predicate::TypeOutlives(ref binder) => {
match (
Expand Down
11 changes: 8 additions & 3 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,9 +730,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}

ty::Predicate::RegionOutlives(ref predicate) => {
// These errors should show up as region
// inference failures.
panic!("region outlives {:?} failed", predicate);
let predicate = self.resolve_type_vars_if_possible(predicate);
let err = self.region_outlives_predicate(&obligation.cause,
&predicate).err().unwrap();
struct_span_err!(
self.tcx.sess, span, E0279,
"the requirement `{}` is not satisfied (`{}`)",
predicate, err,
)
}

ty::Predicate::Projection(..) | ty::Predicate::TypeOutlives(..) => {
Expand Down
6 changes: 4 additions & 2 deletions src/librustc/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,10 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
}

ty::Predicate::RegionOutlives(ref binder) => {
let () = self.selcx.infcx().region_outlives_predicate(&obligation.cause, binder);
ProcessResult::Changed(vec![])
match self.selcx.infcx().region_outlives_predicate(&obligation.cause, binder) {
Ok(()) => ProcessResult::Changed(vec![]),
Err(_) => ProcessResult::Error(CodeSelectionError(Unimplemented)),
}
}

ty::Predicate::TypeOutlives(ref binder) => {
Expand Down
Loading