Always check upper bounds when choosing member regions

Also correctly calculate what the upper bounds are.

Author: matthewjasper

src/librustc_mir/borrow_check/region_infer/mod.rs

@@ -11,8 +11,6 @@ use rustc::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable};
 use rustc_data_structures::binary_search_util;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::graph::scc::Sccs;
-use rustc_data_structures::graph::vec_graph::VecGraph;
-use rustc_data_structures::graph::WithSuccessors;
 use rustc_hir::def_id::DefId;
 use rustc_index::bit_set::BitSet;
 use rustc_index::vec::IndexVec;
@@ -25,6 +23,7 @@ use crate::borrow_check::{
     diagnostics::{RegionErrorKind, RegionErrors},
     member_constraints::{MemberConstraintSet, NllMemberConstraintIndex},
     nll::{PoloniusOutput, ToRegionVid},
+    region_infer::reverse_sccs::ReverseSccGraph,
     region_infer::values::{
         LivenessValues, PlaceholderIndices, RegionElement, RegionValueElements, RegionValues,
         ToElementIndex,
@@ -36,6 +35,7 @@ use crate::borrow_check::{
 mod dump_mir;
 mod graphviz;
 mod opaque_types;
+mod reverse_sccs;
 
 pub mod values;
 
@@ -65,9 +65,10 @@ pub struct RegionInferenceContext<'tcx> {
     /// compute the values of each region.
     pub(in crate::borrow_check) constraint_sccs: Rc<Sccs<RegionVid, ConstraintSccIndex>>,
 
-    /// Reverse of the SCC constraint graph -- i.e., an edge `A -> B`
-    /// exists if `B: A`. Computed lazilly.
-    pub(in crate::borrow_check) rev_constraint_graph: Option<Rc<VecGraph<ConstraintSccIndex>>>,
+    /// Reverse of the SCC constraint graph --  i.e., an edge `A -> B` exists if
+    /// `B: A`. This is used to compute the universal regions that are required
+    /// to outlive a given SCC. Computed lazily.
+    pub(in crate::borrow_check) rev_scc_graph: Option<Rc<ReverseSccGraph>>,
 
     /// The "R0 member of [R1..Rn]" constraints, indexed by SCC.
     pub(in crate::borrow_check) member_constraints:
@@ -281,7 +282,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             constraints,
             constraint_graph,
             constraint_sccs,
-            rev_constraint_graph: None,
+            rev_scc_graph: None,
             member_constraints,
             member_constraints_applied: Vec::new(),
             closure_bounds_mapping,
@@ -673,15 +674,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // free region that must outlive the member region `R0` (`UB:
         // R0`). Therefore, we need only keep an option `O` if `UB: O`
         // for all UB.
-        if choice_regions.len() > 1 {
-            let universal_region_relations = self.universal_region_relations.clone();
-            let rev_constraint_graph = self.rev_constraint_graph();
-            for ub in self.upper_bounds(scc, &rev_constraint_graph) {
-                debug!("apply_member_constraint: ub={:?}", ub);
-                choice_regions.retain(|&o_r| universal_region_relations.outlives(ub, o_r));
-            }
-            debug!("apply_member_constraint: after ub, choice_regions={:?}", choice_regions);
+        let rev_scc_graph = self.reverse_scc_graph();
+        let universal_region_relations = &self.universal_region_relations;
+        for ub in rev_scc_graph.upper_bounds(scc) {
+            debug!("apply_member_constraint: ub={:?}", ub);
+            choice_regions.retain(|&o_r| universal_region_relations.outlives(ub, o_r));
         }
+        debug!("apply_member_constraint: after ub, choice_regions={:?}", choice_regions);
 
         // If we ruled everything out, we're done.
         if choice_regions.is_empty() {
@@ -737,32 +736,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         }
     }
 
-    /// Compute and return the reverse SCC-based constraint graph (lazilly).
-    fn upper_bounds(
-        &'a mut self,
-        scc0: ConstraintSccIndex,
-        rev_constraint_graph: &'a VecGraph<ConstraintSccIndex>,
-    ) -> impl Iterator<Item = RegionVid> + 'a {
-        let scc_values = &self.scc_values;
-        let mut duplicates = FxHashSet::default();
-        rev_constraint_graph
-            .depth_first_search(scc0)
-            .skip(1)
-            .flat_map(move |scc1| scc_values.universal_regions_outlived_by(scc1))
-            .filter(move |&r| duplicates.insert(r))
-    }
-
-    /// Compute and return the reverse SCC-based constraint graph (lazilly).
-    fn rev_constraint_graph(&mut self) -> Rc<VecGraph<ConstraintSccIndex>> {
-        if let Some(g) = &self.rev_constraint_graph {
-            return g.clone();
-        }
-
-        let rev_graph = Rc::new(self.constraint_sccs.reverse());
-        self.rev_constraint_graph = Some(rev_graph.clone());
-        rev_graph
-    }
-
     /// Returns `true` if all the elements in the value of `scc_b` are nameable
     /// in `scc_a`. Used during constraint propagation, and only once
     /// the value of `scc_b` has been computed.

src/librustc_mir/borrow_check/region_infer/reverse_sccs.rs

@@ -0,0 +1,68 @@
+use crate::borrow_check::constraints::ConstraintSccIndex;
+use crate::borrow_check::RegionInferenceContext;
+use itertools::Itertools;
+use rustc::ty::RegionVid;
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_data_structures::graph::vec_graph::VecGraph;
+use rustc_data_structures::graph::WithSuccessors;
+use std::ops::Range;
+use std::rc::Rc;
+
+crate struct ReverseSccGraph {
+    graph: VecGraph<ConstraintSccIndex>,
+    /// For each SCC, the range of `universal_regions` that use that SCC as
+    /// their value.
+    scc_regions: FxHashMap<ConstraintSccIndex, Range<usize>>,
+    /// All of the universal regions, in grouped so that `scc_regions` can
+    /// index into here.
+    universal_regions: Vec<RegionVid>,
+}
+
+impl ReverseSccGraph {
+    /// Find all universal regions that are required to outlive the given SCC.
+    pub(super) fn upper_bounds<'a>(
+        &'a self,
+        scc0: ConstraintSccIndex,
+    ) -> impl Iterator<Item = RegionVid> + 'a {
+        let mut duplicates = FxHashSet::default();
+        self.graph
+            .depth_first_search(scc0)
+            .flat_map(move |scc1| {
+                self.scc_regions
+                    .get(&scc1)
+                    .map_or(&[][..], |range| &self.universal_regions[range.clone()])
+            })
+            .copied()
+            .filter(move |r| duplicates.insert(*r))
+    }
+}
+
+impl RegionInferenceContext<'_> {
+    /// Compute and return the reverse SCC-based constraint graph (lazily).
+    pub(super) fn reverse_scc_graph(&mut self) -> Rc<ReverseSccGraph> {
+        if let Some(g) = &self.rev_scc_graph {
+            return g.clone();
+        }
+
+        let graph = self.constraint_sccs.reverse();
+        let mut paired_scc_regions = self
+            .universal_regions
+            .universal_regions()
+            .map(|region| (self.constraint_sccs.scc(region), region))
+            .collect_vec();
+        paired_scc_regions.sort();
+        let universal_regions = paired_scc_regions.iter().map(|&(_, region)| region).collect();
+
+        let mut scc_regions = FxHashMap::default();
+        let mut start = 0;
+        for (scc, group) in &paired_scc_regions.into_iter().group_by(|(scc, _)| *scc) {
+            let group_size = group.into_iter().count();
+            scc_regions.insert(scc, start..start + group_size);
+            start += group_size;
+        }
+
+        let rev_graph = Rc::new(ReverseSccGraph { graph, scc_regions, universal_regions });
+        self.rev_scc_graph = Some(rev_graph.clone());
+        rev_graph
+    }
+}