Auto merge of #58592 - nikomatsakis:universe-leak-check, r=aturon

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.~~

Author: bors

src/librustc/infer/higher_ranked/mod.rs

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

src/librustc/infer/mod.rs

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

src/librustc/infer/region_constraints/leak_check.rs

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

src/librustc/infer/region_constraints/mod.rs

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

src/librustc/traits/auto_trait.rs

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

src/librustc/traits/error_reporting.rs

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

src/librustc/traits/fulfill.rs

@@ -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) => {