restore the actual leak-check

Author: nikomatsakis

src/librustc/infer/higher_ranked/mod.rs

@@ -116,21 +116,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)
     }
 }

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

src/librustc/infer/region_constraints/mod.rs

@@ -17,6 +17,8 @@ use 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`.

src/librustc/ty/error.rs

@@ -1,5 +1,5 @@
 use hir::def_id::DefId;
-use ty::{self, Region, Ty, TyCtxt};
+use 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")
             }

src/librustc/ty/structural_impls.rs

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

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

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

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