Skip to content

Commit

Permalink
restore the actual leak-check
Browse files Browse the repository at this point in the history
  • Loading branch information
nikomatsakis committed Feb 21, 2019
1 parent 0c94ea0 commit 561ce44
Show file tree
Hide file tree
Showing 8 changed files with 277 additions and 14 deletions.
19 changes: 6 additions & 13 deletions src/librustc/infer/higher_ranked/mod.rs
Expand Up @@ -113,21 +113,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
(result, map)
}

/// 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.
/// See `infer::region_constraints::RegionConstraintCollector::leak_check`.
pub fn leak_check(
&self,
_overly_polymorphic: bool,
_placeholder_map: &PlaceholderMap<'tcx>,
_snapshot: &CombinedSnapshot<'_, 'tcx>,
overly_polymorphic: bool,
placeholder_map: &PlaceholderMap<'tcx>,
snapshot: &CombinedSnapshot<'_, 'tcx>,
) -> RelateResult<'tcx, ()> {
Ok(())
self.borrow_region_constraints()
.leak_check(self.tcx, overly_polymorphic, placeholder_map, snapshot)
}
}
162 changes: 162 additions & 0 deletions src/librustc/infer/region_constraints/leak_check.rs
@@ -0,0 +1,162 @@
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>,
) -> RelateResult<'tcx, ()> {
debug!("leak_check(placeholders={:?})", placeholder_map);

assert!(self.in_snapshot());

// 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
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
16 changes: 15 additions & 1 deletion src/librustc/ty/error.rs
@@ -1,5 +1,5 @@
use crate::hir::def_id::DefId;
use crate::ty::{self, Region, Ty, TyCtxt};
use crate::ty::{self, BoundRegion, Region, Ty, TyCtxt};
use std::borrow::Cow;
use std::fmt;
use rustc_target::spec::abi;
Expand Down Expand Up @@ -27,6 +27,8 @@ pub enum TypeError<'tcx> {
ArgCount,

RegionsDoesNotOutlive(Region<'tcx>, Region<'tcx>),
RegionsInsufficientlyPolymorphic(BoundRegion, Region<'tcx>),
RegionsOverlyPolymorphic(BoundRegion, Region<'tcx>),
RegionsPlaceholderMismatch,

Sorts(ExpectedFound<Ty<'tcx>>),
Expand Down Expand Up @@ -101,6 +103,18 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {
RegionsDoesNotOutlive(..) => {
write!(f, "lifetime mismatch")
}
RegionsInsufficientlyPolymorphic(br, _) => {
write!(f,
"expected bound lifetime parameter{}{}, found concrete lifetime",
if br.is_named() { " " } else { "" },
br)
}
RegionsOverlyPolymorphic(br, _) => {
write!(f,
"expected concrete lifetime, found bound lifetime parameter{}{}",
if br.is_named() { " " } else { "" },
br)
}
RegionsPlaceholderMismatch => {
write!(f, "one type is more general than the other")
}
Expand Down
8 changes: 8 additions & 0 deletions src/librustc/ty/structural_impls.rs
Expand Up @@ -434,6 +434,12 @@ impl<'a, 'tcx> Lift<'tcx> for ty::error::TypeError<'a> {
RegionsDoesNotOutlive(a, b) => {
return tcx.lift(&(a, b)).map(|(a, b)| RegionsDoesNotOutlive(a, b))
}
RegionsInsufficientlyPolymorphic(a, b) => {
return tcx.lift(&b).map(|b| RegionsInsufficientlyPolymorphic(a, b))
}
RegionsOverlyPolymorphic(a, b) => {
return tcx.lift(&b).map(|b| RegionsOverlyPolymorphic(a, b))
}
RegionsPlaceholderMismatch => RegionsPlaceholderMismatch,
IntMismatch(x) => IntMismatch(x),
FloatMismatch(x) => FloatMismatch(x),
Expand Down Expand Up @@ -1021,6 +1027,8 @@ EnumTypeFoldableImpl! {
(ty::error::TypeError::FixedArraySize)(x),
(ty::error::TypeError::ArgCount),
(ty::error::TypeError::RegionsDoesNotOutlive)(a, b),
(ty::error::TypeError::RegionsInsufficientlyPolymorphic)(a, b),
(ty::error::TypeError::RegionsOverlyPolymorphic)(a, b),
(ty::error::TypeError::RegionsPlaceholderMismatch),
(ty::error::TypeError::IntMismatch)(x),
(ty::error::TypeError::FloatMismatch)(x),
Expand Down
42 changes: 42 additions & 0 deletions src/test/ui/hrtb/issue-46989.rs
@@ -0,0 +1,42 @@
// Regression test for #46989:
//
// In the move to universes, this test started passing.
// It is not necessarily WRONG to do so, but it was a bit
// surprising. The reason that it passed is that when we were
// asked to prove that
//
// for<'a> fn(&'a i32): Foo
//
// we were able to use the impl below to prove
//
// fn(&'empty i32): Foo
//
// and then we were able to prove that
//
// fn(&'empty i32) = for<'a> fn(&'a i32)
//
// This last fact is somewhat surprising, but essentially "falls out"
// from handling variance correctly. In particular, consider the subtyping
// relations. First:
//
// fn(&'empty i32) <: for<'a> fn(&'a i32)
//
// This holds because -- intuitively -- a fn that takes a reference but doesn't use
// it can be given a reference with any lifetime. Similarly, the opposite direction:
//
// for<'a> fn(&'a i32) <: fn(&'empty i32)
//
// holds because 'a can be instantiated to 'empty.

trait Foo {

}

impl<A> Foo for fn(A) { }

fn assert_foo<T: Foo>() {}

fn main() {
assert_foo::<fn(&i32)>();
//~^ ERROR the trait bound `for<'r> fn(&'r i32): Foo` is not satisfied
}
29 changes: 29 additions & 0 deletions src/test/ui/hrtb/issue-57639.rs
@@ -0,0 +1,29 @@
// Regression test for #57639:
//
// In the move to universes, this test stopped working. The problem
// was that when the trait solver was asked to prove `for<'a> T::Item:
// Foo<'a>` as part of WF checking, it wound up "eagerly committing"
// to the where clause, which says that `T::Item: Foo<'a>`, but it
// should instead have been using the bound found in the trait
// declaration. Pre-universe, this used to work out ok because we got
// "eager errors" due to the leak check.
//
// See [this comment on GitHub][c] for more details.
//
// run-pass
//
// [c]: https://github.com/rust-lang/rust/issues/57639#issuecomment-455685861

trait Foo<'a> {}

trait Bar {
type Item: for<'a> Foo<'a>;
}

fn foo<'a, T>(_: T)
where
T: Bar,
T::Item: Foo<'a>,
{}

fn main() { }
13 changes: 13 additions & 0 deletions src/test/ui/hrtb/issue-58451.rs
@@ -0,0 +1,13 @@
// Regression test for #58451:
//
// Error reporting here encountered an ICE in the shift to universes.

fn f<I>(i: I)
where
I: IntoIterator,
I::Item: for<'a> Into<&'a ()>,
{}

fn main() {
f(&[f()]);
}

0 comments on commit 561ce44

Please sign in to comment.