replace constant regions with a post-inference check

Rather than declaring some region variables to be constant, and
reporting errors when they would have to change, we instead populate
each free region X with a minimal set of points (the CFG plus end(X)),
and then we let inference do its thing. This may add other `end(Y)`
points into X; we can then check after the fact that indeed `X: Y`
holds.

This requires a bit of "blame" detection to find where the bad
constraint came from: we are currently using a pretty dumb
algorithm. Good place for later expansion.

src/librustc/middle/free_region.rs

@@ -63,28 +63,28 @@ pub fn is_subregion_of(&self,
                            -> bool {
         let result = sub_region == super_region || {
             match (sub_region, super_region) {
-                (&ty::ReEmpty, _) |
-                (_, &ty::ReStatic) =>
+                (ty::ReEmpty, _) |
+                (_, ty::ReStatic) =>
                     true,
 
-                (&ty::ReScope(sub_scope), &ty::ReScope(super_scope)) =>
-                    self.region_scope_tree.is_subscope_of(sub_scope, super_scope),
+                (ty::ReScope(sub_scope), ty::ReScope(super_scope)) =>
+                    self.region_scope_tree.is_subscope_of(*sub_scope, *super_scope),
 
-                (&ty::ReScope(sub_scope), &ty::ReEarlyBound(ref br)) => {
+                (ty::ReScope(sub_scope), ty::ReEarlyBound(ref br)) => {
                     let fr_scope = self.region_scope_tree.early_free_scope(self.tcx, br);
-                    self.region_scope_tree.is_subscope_of(sub_scope, fr_scope)
+                    self.region_scope_tree.is_subscope_of(*sub_scope, fr_scope)
                 }
 
-                (&ty::ReScope(sub_scope), &ty::ReFree(ref fr)) => {
+                (ty::ReScope(sub_scope), ty::ReFree(fr)) => {
                     let fr_scope = self.region_scope_tree.free_scope(self.tcx, fr);
-                    self.region_scope_tree.is_subscope_of(sub_scope, fr_scope)
+                    self.region_scope_tree.is_subscope_of(*sub_scope, fr_scope)
                 }
 
-                (&ty::ReEarlyBound(_), &ty::ReEarlyBound(_)) |
-                (&ty::ReFree(_), &ty::ReEarlyBound(_)) |
-                (&ty::ReEarlyBound(_), &ty::ReFree(_)) |
-                (&ty::ReFree(_), &ty::ReFree(_)) =>
-                    self.free_regions.sub_free_regions(&sub_region, &super_region),
+                (ty::ReEarlyBound(_), ty::ReEarlyBound(_)) |
+                (ty::ReFree(_), ty::ReEarlyBound(_)) |
+                (ty::ReEarlyBound(_), ty::ReFree(_)) |
+                (ty::ReFree(_), ty::ReFree(_)) =>
+                    self.free_regions.sub_free_regions(sub_region, super_region),
 
                 _ =>
                     false,
@@ -162,23 +162,23 @@ pub fn relate_free_regions_from_predicates(&mut self,
     /// (with the exception that `'static: 'x` is not notable)
     pub fn relate_regions(&mut self, sub: Region<'tcx>, sup: Region<'tcx>) {
         debug!("relate_regions(sub={:?}, sup={:?})", sub, sup);
-        if (is_free(sub) || *sub == ty::ReStatic) && is_free(sup) {
+        if is_free_or_static(sub) && is_free(sup) {
             self.relation.add(sub, sup)
         }
     }
 
-    /// True if `r_a <= r_b` is known to hold. Both `r_a` and `r_b`
-    /// must be free regions from the function header.
+    /// Tests whether `r_a <= sup`. Both must be free regions or
+    /// `'static`.
     pub fn sub_free_regions<'a, 'gcx>(&self,
                                       r_a: Region<'tcx>,
                                       r_b: Region<'tcx>)
                                       -> bool {
-        debug!("sub_free_regions(r_a={:?}, r_b={:?})", r_a, r_b);
-        assert!(is_free(r_a));
-        assert!(is_free(r_b));
-        let result = r_a == r_b || self.relation.contains(&r_a, &r_b);
-        debug!("sub_free_regions: result={}", result);
-        result
+        assert!(is_free_or_static(r_a) && is_free_or_static(r_b));
+        if let ty::ReStatic = r_b {
+            true // `'a <= 'static` is just always true, and not stored in the relation explicitly
+        } else {
+            r_a == r_b || self.relation.contains(&r_a, &r_b)
+        }
     }
 
     /// Compute the least-upper-bound of two free regions. In some
@@ -224,6 +224,13 @@ fn is_free(r: Region) -> bool {
     }
 }
 
+fn is_free_or_static(r: Region) -> bool {
+    match *r {
+        ty::ReStatic => true,
+        _ => is_free(r),
+    }
+}
+
 impl_stable_hash_for!(struct FreeRegionMap<'tcx> {
     relation
 });

src/librustc_mir/transform/nll/region_infer.rs

@@ -13,6 +13,7 @@
 use rustc::infer::RegionVariableOrigin;
 use rustc::infer::NLLRegionVariableOrigin;
 use rustc::infer::region_constraints::VarOrigins;
+use rustc::middle::free_region::FreeRegionMap;
 use rustc::mir::{Location, Mir};
 use rustc::ty::{self, RegionVid};
 use rustc_data_structures::indexed_vec::IndexVec;
@@ -40,6 +41,8 @@ pub struct RegionInferenceContext<'tcx> {
 
     /// The constraints we have accumulated and used during solving.
     constraints: Vec<Constraint>,
+
+    free_region_map: &'tcx FreeRegionMap<'tcx>,
 }
 
 struct RegionDefinition<'tcx> {
@@ -52,10 +55,6 @@ struct RegionDefinition<'tcx> {
     /// If this is a free-region, then this is `Some(X)` where `X` is
     /// the name of the region.
     name: Option<ty::Region<'tcx>>,
-
-    /// If true, this is a constant region which cannot grow larger.
-    /// This is used for named regions as well as `'static`.
-    constant: bool,
 }
 
 /// The value of an individual region variable. Region variables
@@ -100,7 +99,6 @@ pub struct Constraint {
     // it is for convenience. Before we dump the constraints in the
     // debugging logs, we sort them, and we'd like the "super region"
     // to be first, etc. (In particular, span should remain last.)
-
     /// The region SUP must outlive SUB...
     sup: RegionVid,
 
@@ -114,7 +112,7 @@ pub struct Constraint {
     span: Span,
 }
 
-impl<'a, 'gcx, 'tcx> RegionInferenceContext<'tcx> {
+impl<'tcx> RegionInferenceContext<'tcx> {
     /// Creates a new region inference context with a total of
     /// `num_region_variables` valid inference variables; the first N
     /// of those will be constant regions representing the free
@@ -133,6 +131,7 @@ pub fn new(var_origins: VarOrigins, free_regions: &FreeRegions<'tcx>, mir: &Mir<
             liveness_constraints: IndexVec::from_elem_n(Region::default(), num_region_variables),
             inferred_values: None,
             constraints: Vec::new(),
+            free_region_map: free_regions.free_region_map,
         };
 
         result.init_free_regions(free_regions, mir);
@@ -162,7 +161,7 @@ pub fn new(var_origins: VarOrigins, free_regions: &FreeRegions<'tcx>, mir: &Mir<
     fn init_free_regions(&mut self, free_regions: &FreeRegions<'tcx>, mir: &Mir<'tcx>) {
         let FreeRegions {
             indices,
-            free_region_map,
+            free_region_map: _,
         } = free_regions;
 
         // For each free region X:
@@ -175,7 +174,6 @@ fn init_free_regions(&mut self, free_regions: &FreeRegions<'tcx>, mir: &Mir<'tcx
 
             // Initialize the name and a few other details.
             self.definitions[variable].name = Some(free_region);
-            self.definitions[variable].constant = true;
 
             // Add all nodes in the CFG to `definition.value`.
             for (block, block_data) in mir.basic_blocks().iter_enumerated() {
@@ -192,21 +190,6 @@ fn init_free_regions(&mut self, free_regions: &FreeRegions<'tcx>, mir: &Mir<'tcx
             // Add `end(X)` into the set for X.
             self.liveness_constraints[variable].add_free_region(variable);
 
-            // `'static` outlives all other free regions as well.
-            if let ty::ReStatic = free_region {
-                for &other_variable in indices.values() {
-                    self.liveness_constraints[variable]
-                        .add_free_region(other_variable);
-                }
-            }
-
-            // Go through each region Y that outlives X (i.e., where
-            // Y: X is true). Add `end(X)` into the set for `Y`.
-            for superregion in free_region_map.regions_that_outlive(&free_region) {
-                let superregion_index = indices[superregion];
-                self.liveness_constraints[superregion_index].add_free_region(variable);
-            }
-
             debug!(
                 "init_free_regions: region variable for `{:?}` is `{:?}` with value `{:?}`",
                 free_region,
@@ -265,20 +248,72 @@ pub(super) fn add_outlives(
     }
 
     /// Perform region inference.
-    pub(super) fn solve(&mut self, infcx: &InferCtxt<'a, 'gcx, 'tcx>, mir: &Mir<'tcx>) {
+    pub(super) fn solve(&mut self, infcx: &InferCtxt<'_, '_, 'tcx>, mir: &Mir<'tcx>) {
         assert!(self.inferred_values.is_none(), "values already inferred");
-        let errors = self.propagate_constraints(mir);
-
-        // worst error msg ever
-        for (fr1, span, fr2) in errors {
-            infcx.tcx.sess.span_err(
-                span,
-                &format!(
-                    "free region `{}` does not outlive `{}`",
-                    self.definitions[fr1].name.unwrap(),
-                    self.definitions[fr2].name.unwrap()
-                ),
-            );
+
+        // Find the minimal regions that can solve the constraints. This is infallible.
+        self.propagate_constraints(mir);
+
+        // Now, see whether any of the constraints were too strong. In particular,
+        // we want to check for a case where a free region exceeded its bounds.
+        // Consider:
+        //
+        //     fn foo<'a, 'b>(x: &'a u32) -> &'b u32 { x }
+        //
+        // In this case, returning `x` requires `&'a u32 <: &'b u32`
+        // and hence we establish (transitively) a constraint that
+        // `'a: 'b`. The `propagate_constraints` code above will
+        // therefore add `end('a)` into the region for `'b` -- but we
+        // have no evidence that `'b` outlives `'a`, so we want to report
+        // an error.
+
+        // The free regions are always found in a prefix of the full list.
+        let free_region_definitions = self.definitions
+            .iter_enumerated()
+            .take_while(|(_, fr_definition)| fr_definition.name.is_some());
+
+        for (fr, fr_definition) in free_region_definitions {
+            self.check_free_region(infcx, fr, fr_definition);
+        }
+    }
+
+    fn check_free_region(
+        &self,
+        infcx: &InferCtxt<'_, '_, 'tcx>,
+        fr: RegionVid,
+        fr_definition: &RegionDefinition<'tcx>,
+    ) {
+        let inferred_values = self.inferred_values.as_ref().unwrap();
+        let fr_name = fr_definition.name.unwrap();
+        let fr_value = &inferred_values[fr];
+
+        // Find every region `o` such that `fr: o`
+        // (because `fr` includes `end(o)`).
+        for &outlived_fr in &fr_value.free_regions {
+            // `fr` includes `end(fr)`, that's not especially
+            // interesting.
+            if fr == outlived_fr {
+                continue;
+            }
+
+            let outlived_fr_definition = &self.definitions[outlived_fr];
+            let outlived_fr_name = outlived_fr_definition.name.unwrap();
+
+            // Check that `o <= fr`. If not, report an error.
+            if !self.free_region_map
+                .sub_free_regions(outlived_fr_name, fr_name)
+            {
+                // worst error msg ever
+                let blame_span = self.blame_span(fr, outlived_fr);
+                infcx.tcx.sess.span_err(
+                    blame_span,
+                    &format!(
+                        "free region `{}` does not outlive `{}`",
+                        fr_name,
+                        outlived_fr_name
+                    ),
+                );
+            }
         }
     }
 
@@ -286,11 +321,9 @@ pub(super) fn solve(&mut self, infcx: &InferCtxt<'a, 'gcx, 'tcx>, mir: &Mir<'tcx
     /// for each region variable until all the constraints are
     /// satisfied. Note that some values may grow **too** large to be
     /// feasible, but we check this later.
-    fn propagate_constraints(&mut self, mir: &Mir<'tcx>) -> Vec<(RegionVid, Span, RegionVid)> {
+    fn propagate_constraints(&mut self, mir: &Mir<'tcx>) {
         let mut changed = true;
         let mut dfs = Dfs::new(mir);
-        let mut error_regions = FxHashSet();
-        let mut errors = vec![];
 
         debug!("propagate_constraints()");
         debug!("propagate_constraints: constraints={:#?}", {
@@ -305,51 +338,78 @@ fn propagate_constraints(&mut self, mir: &Mir<'tcx>) -> Vec<(RegionVid, Span, Re
 
         while changed {
             changed = false;
+            debug!("propagate_constraints: --------------------");
             for constraint in &self.constraints {
                 debug!("propagate_constraints: constraint={:?}", constraint);
+
                 let sub = &inferred_values[constraint.sub].clone();
                 let sup_value = &mut inferred_values[constraint.sup];
 
-                debug!("propagate_constraints:    sub (before): {:?}", sub);
-                debug!("propagate_constraints:    sup (before): {:?}", sup_value);
+                // Grow the value as needed to accommodate the
+                // outlives constraint.
 
-                if !self.definitions[constraint.sup].constant {
-                    // If this is not a constant, then grow the value as needed to
-                    // accommodate the outlives constraint.
+                if dfs.copy(sub, sup_value, constraint.point) {
+                    debug!("propagate_constraints:   sub={:?}", sub);
+                    debug!("propagate_constraints:   sup={:?}", sup_value);
+                    changed = true;
+                }
+            }
+            debug!("\n");
+        }
 
-                    if dfs.copy(sub, sup_value, constraint.point) {
-                        changed = true;
-                    }
+        self.inferred_values = Some(inferred_values);
+    }
 
-                    debug!("propagate_constraints:    sup (after) : {:?}", sup_value);
-                    debug!("propagate_constraints:    changed     : {:?}", changed);
-                } else {
-                    // If this is a constant, check whether it *would
-                    // have* to grow in order for the constraint to be
-                    // satisfied. If so, create an error.
-
-                    let sup_value = &mut sup_value.clone();
-                    if dfs.copy(sub, sup_value, constraint.point) {
-                        // Constant values start out with the entire
-                        // CFG, so it must be some new free region
-                        // that was added. Find one.
-                        let &new_region = sup_value
-                            .free_regions
-                            .difference(&sup_value.free_regions)
-                            .next()
-                            .unwrap();
-                        debug!("propagate_constraints:    new_region : {:?}", new_region);
-                        if error_regions.insert(constraint.sup) {
-                            errors.push((constraint.sup, constraint.span, new_region));
-                        }
+    /// Tries to finds a good span to blame for the fact that `fr1`
+    /// contains `fr2`.
+    fn blame_span(&self, fr1: RegionVid, fr2: RegionVid) -> Span {
+        // Find everything that influenced final value of `fr`.
+        let influenced_fr1 = self.dependencies(fr1);
+
+        // Try to find some outlives constraint `'X: fr2` where `'X`
+        // influenced `fr1`. Blame that.
+        //
+        // NB, this is a pretty bad choice most of the time. In
+        // particular, the connection between `'X` and `fr1` may not
+        // be obvious to the user -- not to mention the naive notion
+        // of dependencies, which doesn't account for the locations of
+        // contraints at all. But it will do for now.
+        for constraint in &self.constraints {
+            if constraint.sub == fr2 && influenced_fr1[constraint.sup] {
+                return constraint.span;
+            }
+        }
+
+        bug!(
+            "could not find any constraint to blame for {:?}: {:?}",
+            fr1,
+            fr2
+        );
+    }
+
+    /// Finds all regions whose values `'a` may depend on in some way.
+    /// Basically if there exists a constraint `'a: 'b @ P`, then `'b`
+    /// and `dependencies('b)` will be in the final set.
+    ///
+    /// Used during error reporting, extremely naive and inefficient.
+    fn dependencies(&self, r0: RegionVid) -> IndexVec<RegionVid, bool> {
+        let mut result_set = IndexVec::from_elem(false, &self.definitions);
+        let mut changed = true;
+        result_set[r0] = true;
+
+        while changed {
+            changed = false;
+            for constraint in &self.constraints {
+                if result_set[constraint.sup] {
+                    if !result_set[constraint.sub] {
+                        result_set[constraint.sub] = true;
+                        changed = true;
                     }
                 }
             }
-            debug!("\n");
         }
 
-        self.inferred_values = Some(inferred_values);
-        errors
+        result_set
     }
 }
 
@@ -434,11 +494,7 @@ fn new(origin: RegionVariableOrigin) -> Self {
         // Create a new region definition. Note that, for free
         // regions, these fields get updated later in
         // `init_free_regions`.
-        Self {
-            origin,
-            name: None,
-            constant: false,
-        }
+        Self { origin, name: None }
     }
 }
 

src/test/mir-opt/nll/named-lifetimes-basic.rs

@@ -26,9 +26,9 @@ fn main() {
 
 // END RUST SOURCE
 // START rustc.use_x.nll.0.mir
-// | '_#0r: {bb0[0], bb0[1], '_#0r, '_#1r, '_#2r, '_#3r}
+// | '_#0r: {bb0[0], bb0[1], '_#0r}
 // | '_#1r: {bb0[0], bb0[1], '_#1r}
-// | '_#2r: {bb0[0], bb0[1], '_#1r, '_#2r}
+// | '_#2r: {bb0[0], bb0[1], '_#2r}
 // | '_#3r: {bb0[0], bb0[1], '_#3r}
 // fn use_x(_1: &'_#1r mut i32, _2: &'_#2r u32, _3: &'_#1r u32, _4: &'_#3r u32) -> bool {
 // END rustc.use_x.nll.0.mir