Skip to content

Commit ed36698

Browse files
committed
compute region values using SCCs not iterative flow
The strategy is this: - we compute SCCs once all outlives constraints are known - we allocate a set of values **per region** for storing liveness - we allocate a set of values **per SCC** for storing the final values - when we add a liveness constraint to the region R, we also add it to the final value of the SCC to which R belongs - then we can apply the constraints by just walking the DAG for the SCCs and union'ing the children (which have their liveness constraints within) There are a few intermediate refactorings that I really ought to have broken out into their own commits: - reverse the constraint graph so that `R1: R2` means `R1 -> R2` and not `R2 -> R1`. This fits better with the SCC computation and new style of inference (`->` now means "take value from" and not "push value into") - this does affect some of the UI tests, since they traverse the graph, but mostly the artificial ones and they don't necessarily seem worse - put some things (constraint set, etc) into `Rc`. This lets us root them to permit mutation and iteration. It also guarantees they don't change, which is critical to the correctness of the algorithm. - Generalize various helpers that previously operated only on points to work on any sort of region element.
1 parent 862c0dd commit ed36698

File tree

14 files changed

+340
-214
lines changed

14 files changed

+340
-214
lines changed

src/librustc_data_structures/graph/scc/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ impl<N: Idx, S: Idx> Sccs<N, S> {
5454
self.scc_data.len()
5555
}
5656

57+
/// Returns the number of SCCs in the graph.
58+
pub fn all_sccs(&self) -> impl Iterator<Item = S> {
59+
(0 .. self.scc_data.len()).map(S::new)
60+
}
61+
5762
/// Returns the SCC to which a node `r` belongs.
5863
pub fn scc(&self, r: N) -> S {
5964
self.scc_indices[r]

src/librustc_mir/borrow_check/nll/constraint_generation.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ impl<'cx, 'cg, 'gcx, 'tcx> ConstraintGeneration<'cx, 'cg, 'gcx, 'tcx> {
210210
for (region, location) in liveness_set {
211211
debug!("generate: {:#?} is live at {:#?}", region, location);
212212
let region_vid = regioncx.to_region_vid(region);
213-
regioncx.add_live_point(region_vid, *location);
213+
regioncx.add_live_element(region_vid, *location);
214214
}
215215

216216
if let Some(all_facts) = all_facts {
@@ -242,7 +242,7 @@ impl<'cx, 'cg, 'gcx, 'tcx> ConstraintGeneration<'cx, 'cg, 'gcx, 'tcx> {
242242
.tcx
243243
.for_each_free_region(&live_ty, |live_region| {
244244
let vid = live_region.to_region_vid();
245-
self.regioncx.add_live_point(vid, location);
245+
self.regioncx.add_live_element(vid, location);
246246
});
247247
}
248248
}
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use borrow_check::nll::constraints::{ConstraintIndex, ConstraintSet};
12+
use rustc::ty::RegionVid;
13+
use rustc_data_structures::graph;
14+
use rustc_data_structures::indexed_vec::IndexVec;
15+
16+
crate struct ConstraintGraph {
17+
first_constraints: IndexVec<RegionVid, Option<ConstraintIndex>>,
18+
next_constraints: IndexVec<ConstraintIndex, Option<ConstraintIndex>>,
19+
}
20+
21+
impl ConstraintGraph {
22+
/// Create a "dependency graph" where each region constraint `R1:
23+
/// R2` is treated as an edge `R1 -> R2`. We use this graph to
24+
/// construct SCCs for region inference but also for error
25+
/// reporting.
26+
crate fn new(set: &ConstraintSet, num_region_vars: usize) -> Self {
27+
let mut first_constraints = IndexVec::from_elem_n(None, num_region_vars);
28+
let mut next_constraints = IndexVec::from_elem(None, &set.constraints);
29+
30+
for (idx, constraint) in set.constraints.iter_enumerated().rev() {
31+
let mut head = &mut first_constraints[constraint.sup];
32+
let mut next = &mut next_constraints[idx];
33+
debug_assert!(next.is_none());
34+
*next = *head;
35+
*head = Some(idx);
36+
}
37+
38+
Self {
39+
first_constraints,
40+
next_constraints,
41+
}
42+
}
43+
44+
/// Given a region `R`, iterate over all constraints `R: R1`.
45+
crate fn outgoing_edges(&self, region_sup: RegionVid) -> Edges<'_> {
46+
let first = self.first_constraints[region_sup];
47+
Edges {
48+
graph: self,
49+
pointer: first,
50+
}
51+
}
52+
}
53+
54+
crate struct Edges<'s> {
55+
graph: &'s ConstraintGraph,
56+
pointer: Option<ConstraintIndex>,
57+
}
58+
59+
impl<'s> Iterator for Edges<'s> {
60+
type Item = ConstraintIndex;
61+
62+
fn next(&mut self) -> Option<Self::Item> {
63+
if let Some(p) = self.pointer {
64+
self.pointer = self.graph.next_constraints[p];
65+
Some(p)
66+
} else {
67+
None
68+
}
69+
}
70+
}
71+
72+
crate struct RegionGraph<'s> {
73+
set: &'s ConstraintSet,
74+
constraint_graph: &'s ConstraintGraph,
75+
}
76+
77+
impl<'s> RegionGraph<'s> {
78+
/// Create a "dependency graph" where each region constraint `R1:
79+
/// R2` is treated as an edge `R1 -> R2`. We use this graph to
80+
/// construct SCCs for region inference but also for error
81+
/// reporting.
82+
crate fn new(set: &'s ConstraintSet, constraint_graph: &'s ConstraintGraph) -> Self {
83+
Self {
84+
set,
85+
constraint_graph,
86+
}
87+
}
88+
89+
/// Given a region `R`, iterate over all regions `R1` such that
90+
/// there exists a constraint `R: R1`.
91+
crate fn sub_regions(&self, region_sup: RegionVid) -> Successors<'_> {
92+
Successors {
93+
set: self.set,
94+
edges: self.constraint_graph.outgoing_edges(region_sup),
95+
}
96+
}
97+
}
98+
99+
crate struct Successors<'s> {
100+
set: &'s ConstraintSet,
101+
edges: Edges<'s>,
102+
}
103+
104+
impl<'s> Iterator for Successors<'s> {
105+
type Item = RegionVid;
106+
107+
fn next(&mut self) -> Option<Self::Item> {
108+
self.edges.next().map(|c| self.set[c].sub)
109+
}
110+
}
111+
112+
impl<'s> graph::DirectedGraph for RegionGraph<'s> {
113+
type Node = RegionVid;
114+
}
115+
116+
impl<'s> graph::WithNumNodes for RegionGraph<'s> {
117+
fn num_nodes(&self) -> usize {
118+
self.constraint_graph.first_constraints.len()
119+
}
120+
}
121+
122+
impl<'s> graph::WithSuccessors for RegionGraph<'s> {
123+
fn successors<'graph>(
124+
&'graph self,
125+
node: Self::Node,
126+
) -> <Self as graph::GraphSuccessors<'graph>>::Iter {
127+
self.sub_regions(node)
128+
}
129+
}
130+
131+
impl<'s, 'graph> graph::GraphSuccessors<'graph> for RegionGraph<'s> {
132+
type Item = RegionVid;
133+
type Iter = Successors<'graph>;
134+
}

src/librustc_mir/borrow_check/nll/constraints/mod.rs

Lines changed: 27 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,22 @@
99
// except according to those terms.
1010

1111
use rustc::ty::RegionVid;
12+
use rustc_data_structures::graph::scc::Sccs;
1213
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
1314
use borrow_check::nll::type_check::Locations;
1415

1516
use std::fmt;
1617
use std::ops::Deref;
1718

19+
crate mod graph;
20+
1821
#[derive(Clone, Default)]
1922
crate struct ConstraintSet {
2023
constraints: IndexVec<ConstraintIndex, OutlivesConstraint>,
2124
}
2225

2326
impl ConstraintSet {
24-
pub fn push(&mut self, constraint: OutlivesConstraint) {
27+
crate fn push(&mut self, constraint: OutlivesConstraint) {
2528
debug!(
2629
"ConstraintSet::push({:?}: {:?} @ {:?}",
2730
constraint.sup, constraint.sub, constraint.locations
@@ -32,12 +35,33 @@ impl ConstraintSet {
3235
}
3336
self.constraints.push(constraint);
3437
}
38+
39+
/// Constructs a graph from the constraint set; the graph makes it
40+
/// easy to find the constriants affecting a particular region
41+
/// (you should not mutate the set once this graph is
42+
/// constructed).
43+
crate fn graph(&self, num_region_vars: usize) -> graph::ConstraintGraph {
44+
graph::ConstraintGraph::new(self, num_region_vars)
45+
}
46+
47+
/// Compute cycles (SCCs) in the graph of regions. In particular,
48+
/// find all regions R1, R2 such that R1: R2 and R2: R1 and group
49+
/// them into an SCC, and find the relationships between SCCs.
50+
crate fn compute_sccs(
51+
&self,
52+
constraint_graph: &graph::ConstraintGraph,
53+
) -> Sccs<RegionVid, ConstraintSccIndex> {
54+
let region_graph = &graph::RegionGraph::new(self, constraint_graph);
55+
Sccs::new(region_graph)
56+
}
3557
}
3658

3759
impl Deref for ConstraintSet {
3860
type Target = IndexVec<ConstraintIndex, OutlivesConstraint>;
3961

40-
fn deref(&self) -> &Self::Target { &self.constraints }
62+
fn deref(&self) -> &Self::Target {
63+
&self.constraints
64+
}
4165
}
4266

4367
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -68,45 +92,4 @@ impl fmt::Debug for OutlivesConstraint {
6892

6993
newtype_index!(ConstraintIndex { DEBUG_FORMAT = "ConstraintIndex({})" });
7094

71-
crate struct ConstraintGraph {
72-
first_constraints: IndexVec<RegionVid, Option<ConstraintIndex>>,
73-
next_constraints: IndexVec<ConstraintIndex, Option<ConstraintIndex>>,
74-
}
75-
76-
impl ConstraintGraph {
77-
/// Constraint a graph where each region constraint `R1: R2` is
78-
/// treated as an edge `R2 -> R1`. This is useful for cheaply
79-
/// finding dirty constraints.
80-
crate fn new(set: &ConstraintSet, num_region_vars: usize) -> Self {
81-
let mut first_constraints = IndexVec::from_elem_n(None, num_region_vars);
82-
let mut next_constraints = IndexVec::from_elem(None, &set.constraints);
83-
84-
for (idx, constraint) in set.constraints.iter_enumerated().rev() {
85-
let mut head = &mut first_constraints[constraint.sub];
86-
let mut next = &mut next_constraints[idx];
87-
debug_assert!(next.is_none());
88-
*next = *head;
89-
*head = Some(idx);
90-
}
91-
92-
ConstraintGraph { first_constraints, next_constraints }
93-
}
94-
95-
/// Invokes `op` with the index of any constraints of the form
96-
/// `region_sup: region_sub`. These are the constraints that must
97-
/// be reprocessed when the value of `R1` changes. If you think of
98-
/// each constraint `R1: R2` as an edge `R2 -> R1`, then this
99-
/// gives the set of successors to R2.
100-
crate fn for_each_dependent(
101-
&self,
102-
region_sub: RegionVid,
103-
mut op: impl FnMut(ConstraintIndex),
104-
) {
105-
let mut p = self.first_constraints[region_sub];
106-
while let Some(dep_idx) = p {
107-
op(dep_idx);
108-
p = self.next_constraints[dep_idx];
109-
}
110-
}
111-
}
112-
95+
newtype_index!(ConstraintSccIndex { DEBUG_FORMAT = "ConstraintSccIndex({})" });

src/librustc_mir/borrow_check/nll/explain_borrow/find_use.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ impl<'cx, 'gcx, 'tcx> UseFinder<'cx, 'gcx, 'tcx> {
5757

5858
queue.push_back(self.start_point);
5959
while let Some(p) = queue.pop_front() {
60-
if !self.regioncx.region_contains_point(self.region_vid, p) {
60+
if !self.regioncx.region_contains(self.region_vid, p) {
6161
continue;
6262
}
6363

src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,18 +50,10 @@ impl fmt::Display for ConstraintCategory {
5050

5151
impl<'tcx> RegionInferenceContext<'tcx> {
5252
/// Walks the graph of constraints (where `'a: 'b` is considered
53-
/// an edge `'b -> 'a`) to find all paths from `from_region` to
53+
/// an edge `'a -> 'b`) to find all paths from `from_region` to
5454
/// `to_region`. The paths are accumulated into the vector
5555
/// `results`. The paths are stored as a series of
5656
/// `ConstraintIndex` values -- in other words, a list of *edges*.
57-
///
58-
/// # Parameters
59-
///
60-
/// - `from_region`
61-
/// When reporting an error, it is useful to be able to determine
62-
/// which constraints influenced the region being reported as an
63-
/// error. This function finds all of the paths from the
64-
/// constraint.
6557
fn find_constraint_paths_between_regions(
6658
&self,
6759
from_region: RegionVid,
@@ -97,25 +89,25 @@ impl<'tcx> RegionInferenceContext<'tcx> {
9789
// Check if we reached the region we were looking for.
9890
if target_test(current_region) {
9991
if !stack.is_empty() {
100-
assert_eq!(self.constraints[stack[0]].sub, from_region);
92+
assert_eq!(self.constraints[stack[0]].sup, from_region);
10193
results.push(stack.clone());
10294
}
10395
return;
10496
}
10597

106-
self.constraint_graph.for_each_dependent(current_region, |constraint| {
107-
assert_eq!(self.constraints[constraint].sub, current_region);
98+
for constraint in self.constraint_graph.outgoing_edges(current_region) {
99+
assert_eq!(self.constraints[constraint].sup, current_region);
108100
stack.push(constraint);
109101
self.find_constraint_paths_between_regions_helper(
110102
from_region,
111-
self.constraints[constraint].sup,
103+
self.constraints[constraint].sub,
112104
target_test,
113105
visited,
114106
stack,
115107
results,
116108
);
117109
stack.pop();
118-
});
110+
}
119111
}
120112

121113
/// This function will return true if a constraint is interesting and false if a constraint
@@ -207,7 +199,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
207199
}
208200

209201
// Find all paths
210-
let constraint_paths = self.find_constraint_paths_between_regions(outlived_fr, |r| r == fr);
202+
let constraint_paths = self.find_constraint_paths_between_regions(fr, |r| r == outlived_fr);
211203
debug!("report_error: constraint_paths={:#?}", constraint_paths);
212204

213205
// Find the shortest such path.
@@ -316,7 +308,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
316308

317309
while changed {
318310
changed = false;
319-
for constraint in &*self.constraints {
311+
for constraint in self.constraints.iter() {
320312
if let Some(n) = result_set[constraint.sup] {
321313
let m = n + 1;
322314
if result_set[constraint.sub]

0 commit comments

Comments
 (0)