restore the actual leak-check

This commit is contained in:
Niko Matsakis 2019-02-20 05:39:04 -05:00
parent 0c94ea0bf1
commit 561ce442de
8 changed files with 277 additions and 14 deletions

View file

@ -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)
}
}

View file

@ -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);
}
}
}
}

View file

@ -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`.

View file

@ -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;
@ -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>>),
@ -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")
}

View file

@ -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),
@ -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),

View file

@ -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
}

View file

@ -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() { }

View file

@ -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()]);
}