From 52dc18bae0cc7c58b99d35249642ad5c69fb1698 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 1 Jul 2018 06:11:08 -0400 Subject: [PATCH 01/14] introduce `ConstraintGraph`, stop mutating constraints in place Encapsulate the dependencies more cleanly. --- .../borrow_check/nll/constraint_set.rs | 86 +++++++++---------- .../borrow_check/nll/region_infer/dump_mir.rs | 1 - .../borrow_check/nll/region_infer/mod.rs | 45 +++------- .../nll/type_check/constraint_conversion.rs | 1 - 4 files changed, 57 insertions(+), 76 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/constraint_set.rs b/src/librustc_mir/borrow_check/nll/constraint_set.rs index 3bdf78ff3db54..76dc50dddc981 100644 --- a/src/librustc_mir/borrow_check/nll/constraint_set.rs +++ b/src/librustc_mir/borrow_check/nll/constraint_set.rs @@ -32,39 +32,6 @@ impl ConstraintSet { } self.constraints.push(constraint); } - - /// Once all constraints have been added, `link()` is used to thread together the constraints - /// based on which would be affected when a particular region changes. See the next field of - /// `OutlivesContraint` for more details. - /// link returns a map that is needed later by `each_affected_by_dirty`. - pub fn link(&mut self, len: usize) -> IndexVec> { - let mut map = IndexVec::from_elem_n(None, len); - - for (idx, constraint) in self.constraints.iter_enumerated_mut().rev() { - let mut head = &mut map[constraint.sub]; - debug_assert!(constraint.next.is_none()); - constraint.next = *head; - *head = Some(idx); - } - - map - } - - /// When a region R1 changes, we need to reprocess all constraints R2: R1 to take into account - /// any new elements that R1 now has. This method will quickly enumerate all such constraints - /// (that is, constraints where R1 is in the "subregion" position). - /// To use it, invoke with `map[R1]` where map is the map returned by `link`; - /// the callback op will be invoked for each affected constraint. - pub fn each_affected_by_dirty( - &self, - mut opt_dep_idx: Option, - mut op: impl FnMut(ConstraintIndex), - ) { - while let Some(dep_idx) = opt_dep_idx { - op(dep_idx); - opt_dep_idx = self.constraints[dep_idx].next; - } - } } impl Deref for ConstraintSet { @@ -85,16 +52,6 @@ pub struct OutlivesConstraint { /// Region that must be outlived. pub sub: RegionVid, - /// Later on, we thread the constraints onto a linked list - /// grouped by their `sub` field. So if you had: - /// - /// Index | Constraint | Next Field - /// ----- | ---------- | ---------- - /// 0 | `'a: 'b` | Some(2) - /// 1 | `'b: 'c` | None - /// 2 | `'c: 'b` | None - pub next: Option, - /// Where did this constraint arise? pub locations: Locations, } @@ -110,3 +67,46 @@ impl fmt::Debug for OutlivesConstraint { } newtype_index!(ConstraintIndex { DEBUG_FORMAT = "ConstraintIndex({})" }); + +crate struct ConstraintGraph { + first_constraints: IndexVec>, + next_constraints: IndexVec>, +} + +impl ConstraintGraph { + /// Constraint a graph where each region constraint `R1: R2` is + /// treated as an edge `R2 -> R1`. This is useful for cheaply + /// finding dirty constraints. + crate fn new(set: &ConstraintSet, num_region_vars: usize) -> Self { + let mut first_constraints = IndexVec::from_elem_n(None, num_region_vars); + let mut next_constraints = IndexVec::from_elem(None, &set.constraints); + + for (idx, constraint) in set.constraints.iter_enumerated().rev() { + let mut head = &mut first_constraints[constraint.sub]; + let mut next = &mut next_constraints[idx]; + debug_assert!(next.is_none()); + *next = *head; + *head = Some(idx); + } + + ConstraintGraph { first_constraints, next_constraints } + } + + /// Invokes `op` with the index of any constraints of the form + /// `region_sup: region_sub`. These are the constraints that must + /// be reprocessed when the value of `R1` changes. If you think of + /// each constraint `R1: R2` as an edge `R2 -> R1`, then this + /// gives the set of successors to R2. + crate fn for_each_dependent( + &self, + region_sub: RegionVid, + mut op: impl FnMut(ConstraintIndex), + ) { + let mut p = self.first_constraints[region_sub]; + while let Some(dep_idx) = p { + op(dep_idx); + p = self.next_constraints[dep_idx]; + } + } +} + diff --git a/src/librustc_mir/borrow_check/nll/region_infer/dump_mir.rs b/src/librustc_mir/borrow_check/nll/region_infer/dump_mir.rs index 88d9f46e340d3..3c73203706dcb 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/dump_mir.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/dump_mir.rs @@ -83,7 +83,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { sup, sub, locations, - next: _, } = constraint; with_msg(&format!( "{:?}: {:?} due to {:?}", diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 13cc0c0419eab..5b8b8513b6107 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -9,7 +9,9 @@ // except according to those terms. use super::universal_regions::UniversalRegions; -use borrow_check::nll::constraint_set::{ConstraintIndex, ConstraintSet, OutlivesConstraint}; +use borrow_check::nll::constraint_set::{ + ConstraintIndex, ConstraintGraph, ConstraintSet, OutlivesConstraint +}; use borrow_check::nll::type_check::Locations; use rustc::hir::def_id::DefId; use rustc::infer::canonical::QueryRegionConstraint; @@ -57,13 +59,6 @@ pub struct RegionInferenceContext<'tcx> { /// until `solve` is invoked. inferred_values: Option, - /// For each variable, stores the index of the first constraint - /// where that variable appears on the RHS. This is the start of a - /// 'linked list' threaded by the `next` field in `Constraint`. - /// - /// This map is build when values are inferred. - dependency_map: Option>>, - /// The constraints we have accumulated and used during solving. constraints: ConstraintSet, @@ -203,9 +198,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { outlives_constraints: ConstraintSet, type_tests: Vec>, ) -> Self { - // The `next` field should not yet have been initialized: - debug_assert!(outlives_constraints.iter().all(|c| c.next.is_none())); - let num_region_variables = var_infos.len(); let num_universal_regions = universal_regions.len(); @@ -222,7 +214,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { elements: elements.clone(), liveness_constraints: RegionValues::new(elements, num_region_variables), inferred_values: None, - dependency_map: None, constraints: outlives_constraints, type_tests, universal_regions, @@ -335,7 +326,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { locations, sup, sub, - next: None, }) } @@ -398,7 +388,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// 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>) { - self.dependency_map = Some(self.build_dependency_map()); + assert!(self.inferred_values.is_none()); let inferred_values = self.compute_region_values(mir); self.inferred_values = Some(inferred_values); } @@ -415,7 +405,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { // constraints we have accumulated. let mut inferred_values = self.liveness_constraints.clone(); - let dependency_map = self.dependency_map.as_ref().unwrap(); + let constraint_graph = ConstraintGraph::new(&self.constraints, self.definitions.len()); // Constraints that may need to be repropagated (initially all): let mut dirty_list: Vec<_> = self.constraints.indices().collect(); @@ -434,14 +424,15 @@ impl<'tcx> RegionInferenceContext<'tcx> { debug!("propagate_constraints: sub={:?}", constraint.sub); debug!("propagate_constraints: sup={:?}", constraint.sup); - self.constraints.each_affected_by_dirty( - dependency_map[constraint.sup], - |dep_idx| { - if clean_bit_vec.remove(dep_idx.index()) { - dirty_list.push(dep_idx); - } - }, - ); + // The region of `constraint.sup` changed, so find all + // constraints of the form `R: constriant.sup` and + // enqueue them as dirty. We will have to reprocess + // them. + constraint_graph.for_each_dependent(constraint.sup, |dep_idx| { + if clean_bit_vec.remove(dep_idx.index()) { + dirty_list.push(dep_idx); + } + }); } debug!("\n"); @@ -450,14 +441,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { inferred_values } - /// Builds up a map from each region variable X to a vector with the - /// indices of constraints that need to be re-evaluated when X changes. - /// These are constraints like Y: X @ P -- so if X changed, we may - /// need to grow Y. - fn build_dependency_map(&mut self) -> IndexVec> { - self.constraints.link(self.definitions.len()) - } - /// Once regions have been propagated, this method is used to see /// whether the "type tests" produced by typeck were satisfied; /// type tests encode type-outlives relationships like `T: diff --git a/src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs b/src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs index 27bd50427772d..a5a159fbb1c3d 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs @@ -186,7 +186,6 @@ impl<'a, 'gcx, 'tcx> ConstraintConversion<'a, 'gcx, 'tcx> { locations: self.locations, sub, sup, - next: None, }); } From 48b24280be7591dc1ca16f12e1c3395eb4a14b4a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 1 Jul 2018 06:12:04 -0400 Subject: [PATCH 02/14] fix `debug!` --- src/librustc_mir/borrow_check/nll/constraint_set.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/borrow_check/nll/constraint_set.rs b/src/librustc_mir/borrow_check/nll/constraint_set.rs index 76dc50dddc981..eab1de073116f 100644 --- a/src/librustc_mir/borrow_check/nll/constraint_set.rs +++ b/src/librustc_mir/borrow_check/nll/constraint_set.rs @@ -23,7 +23,7 @@ crate struct ConstraintSet { impl ConstraintSet { pub fn push(&mut self, constraint: OutlivesConstraint) { debug!( - "add_outlives({:?}: {:?} @ {:?})", + "ConstraintSet::push({:?}: {:?} @ {:?}", constraint.sup, constraint.sub, constraint.locations ); if constraint.sup == constraint.sub { From 01fc740b99ffc84d8aef865b9300e1a33543c19a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 1 Jul 2018 10:22:36 -0400 Subject: [PATCH 03/14] generate reborrow constraints at type check time --- .../borrow_check/nll/constraint_generation.rs | 138 +--------------- src/librustc_mir/borrow_check/nll/mod.rs | 1 + .../borrow_check/nll/region_infer/mod.rs | 10 -- .../borrow_check/nll/type_check/mod.rs | 155 +++++++++++++++++- 4 files changed, 153 insertions(+), 151 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/constraint_generation.rs b/src/librustc_mir/borrow_check/nll/constraint_generation.rs index 25a0123755f2c..9dcdf7de31455 100644 --- a/src/librustc_mir/borrow_check/nll/constraint_generation.rs +++ b/src/librustc_mir/borrow_check/nll/constraint_generation.rs @@ -13,14 +13,11 @@ use borrow_check::location::LocationTable; use borrow_check::nll::ToRegionVid; use borrow_check::nll::facts::AllFacts; use borrow_check::nll::region_infer::RegionInferenceContext; -use borrow_check::nll::type_check::AtLocation; -use rustc::hir; use rustc::infer::InferCtxt; use rustc::mir::visit::TyContext; use rustc::mir::visit::Visitor; -use rustc::mir::Place::Projection; use rustc::mir::{BasicBlock, BasicBlockData, Location, Mir, Place, Rvalue}; -use rustc::mir::{Local, PlaceProjection, ProjectionElem, Statement, Terminator}; +use rustc::mir::{Local, Statement, Terminator}; use rustc::ty::fold::TypeFoldable; use rustc::ty::subst::Substs; use rustc::ty::{self, CanonicalTy, ClosureSubsts, GeneratorSubsts}; @@ -41,7 +38,6 @@ pub(super) fn generate_constraints<'cx, 'gcx, 'tcx>( regioncx, location_table, all_facts, - mir, }; cg.add_region_liveness_constraints_from_type_check(liveness_set_from_typeck); @@ -57,7 +53,6 @@ struct ConstraintGeneration<'cg, 'cx: 'cg, 'gcx: 'tcx, 'tcx: 'cx> { all_facts: &'cg mut Option, location_table: &'cg LocationTable, regioncx: &'cg mut RegionInferenceContext<'tcx>, - mir: &'cg Mir<'tcx>, borrow_set: &'cg BorrowSet<'tcx>, } @@ -184,41 +179,6 @@ impl<'cg, 'cx, 'gcx, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cg, 'cx, 'gcx self.super_terminator(block, terminator, location); } - fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) { - debug!("visit_rvalue(rvalue={:?}, location={:?})", rvalue, location); - - match rvalue { - Rvalue::Ref(region, _borrow_kind, borrowed_place) => { - // In some cases, e.g. when borrowing from an unsafe - // place, we don't bother to create a loan, since - // there are no conditions to validate. - if let Some(all_facts) = self.all_facts { - if let Some(borrow_index) = self.borrow_set.location_map.get(&location) { - let region_vid = region.to_region_vid(); - all_facts.borrow_region.push(( - region_vid, - *borrow_index, - self.location_table.mid_index(location), - )); - } - } - - // Look for an rvalue like: - // - // & L - // - // where L is the path that is borrowed. In that case, we have - // to add the reborrow constraints (which don't fall out - // naturally from the type-checker). - self.add_reborrow_constraint(location, region, borrowed_place); - } - - _ => {} - } - - self.super_rvalue(rvalue, location); - } - fn visit_user_assert_ty( &mut self, _c_ty: &CanonicalTy<'tcx>, @@ -285,100 +245,4 @@ impl<'cx, 'cg, 'gcx, 'tcx> ConstraintGeneration<'cx, 'cg, 'gcx, 'tcx> { self.regioncx.add_live_point(vid, location); }); } - - // Add the reborrow constraint at `location` so that `borrowed_place` - // is valid for `borrow_region`. - fn add_reborrow_constraint( - &mut self, - location: Location, - borrow_region: ty::Region<'tcx>, - borrowed_place: &Place<'tcx>, - ) { - let mut borrowed_place = borrowed_place; - - debug!( - "add_reborrow_constraint({:?}, {:?}, {:?})", - location, borrow_region, borrowed_place - ); - while let Projection(box PlaceProjection { base, elem }) = borrowed_place { - debug!("add_reborrow_constraint - iteration {:?}", borrowed_place); - - match *elem { - ProjectionElem::Deref => { - let tcx = self.infcx.tcx; - let base_ty = base.ty(self.mir, tcx).to_ty(tcx); - - debug!("add_reborrow_constraint - base_ty = {:?}", base_ty); - match base_ty.sty { - ty::TyRef(ref_region, _, mutbl) => { - self.regioncx.add_outlives( - location.boring(), - ref_region.to_region_vid(), - borrow_region.to_region_vid(), - ); - - if let Some(all_facts) = self.all_facts { - all_facts.outlives.push(( - ref_region.to_region_vid(), - borrow_region.to_region_vid(), - self.location_table.mid_index(location), - )); - } - - match mutbl { - hir::Mutability::MutImmutable => { - // Immutable reference. We don't need the base - // to be valid for the entire lifetime of - // the borrow. - break; - } - hir::Mutability::MutMutable => { - // Mutable reference. We *do* need the base - // to be valid, because after the base becomes - // invalid, someone else can use our mutable deref. - - // This is in order to make the following function - // illegal: - // ``` - // fn unsafe_deref<'a, 'b>(x: &'a &'b mut T) -> &'b mut T { - // &mut *x - // } - // ``` - // - // As otherwise you could clone `&mut T` using the - // following function: - // ``` - // fn bad(x: &mut T) -> (&mut T, &mut T) { - // let my_clone = unsafe_deref(&'a x); - // ENDREGION 'a; - // (my_clone, x) - // } - // ``` - } - } - } - ty::TyRawPtr(..) => { - // deref of raw pointer, guaranteed to be valid - break; - } - ty::TyAdt(def, _) if def.is_box() => { - // deref of `Box`, need the base to be valid - propagate - } - _ => bug!("unexpected deref ty {:?} in {:?}", base_ty, borrowed_place), - } - } - ProjectionElem::Field(..) - | ProjectionElem::Downcast(..) - | ProjectionElem::Index(..) - | ProjectionElem::ConstantIndex { .. } - | ProjectionElem::Subslice { .. } => { - // other field access - } - } - - // The "propagate" case. We need to check that our base is valid - // for the borrow's lifetime. - borrowed_place = base; - } - } } diff --git a/src/librustc_mir/borrow_check/nll/mod.rs b/src/librustc_mir/borrow_check/nll/mod.rs index 16506800c9e16..1891cac268c52 100644 --- a/src/librustc_mir/borrow_check/nll/mod.rs +++ b/src/librustc_mir/borrow_check/nll/mod.rs @@ -108,6 +108,7 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>( def_id, &universal_regions, location_table, + borrow_set, &liveness, &mut all_facts, flow_inits, diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 5b8b8513b6107..13c5d6c05893a 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -319,16 +319,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.liveness_constraints.add_element(v, element) } - /// Indicates that the region variable `sup` must outlive `sub` is live at the point `point`. - pub(super) fn add_outlives(&mut self, locations: Locations, sup: RegionVid, sub: RegionVid) { - assert!(self.inferred_values.is_none(), "values already inferred"); - self.constraints.push(OutlivesConstraint { - locations, - sup, - sub, - }) - } - /// Perform region inference and report errors if we see any /// unsatisfiable constraints. If this is a closure, returns the /// region requirements to propagate to our creator, if any. diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 2b47d50b4c2c7..97a74e0d3365b 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -11,14 +11,17 @@ //! This pass type-checks the MIR to ensure it is not broken. #![allow(unreachable_code)] +use borrow_check::borrow_set::BorrowSet; use borrow_check::location::LocationTable; -use borrow_check::nll::constraint_set::ConstraintSet; +use borrow_check::nll::constraint_set::{ConstraintSet, OutlivesConstraint}; use borrow_check::nll::facts::AllFacts; use borrow_check::nll::region_infer::{ClosureRegionRequirementsExt, TypeTest}; use borrow_check::nll::universal_regions::UniversalRegions; +use borrow_check::nll::ToRegionVid; use dataflow::move_paths::MoveData; use dataflow::FlowAtLocation; use dataflow::MaybeInitializedPlaces; +use rustc::hir; use rustc::hir::def_id::DefId; use rustc::infer::canonical::QueryRegionConstraint; use rustc::infer::region_constraints::GenericKind; @@ -103,6 +106,7 @@ pub(crate) fn type_check<'gcx, 'tcx>( mir_def_id: DefId, universal_regions: &UniversalRegions<'tcx>, location_table: &LocationTable, + borrow_set: &BorrowSet<'tcx>, liveness: &LivenessResults, all_facts: &mut Option, flow_inits: &mut FlowAtLocation>, @@ -119,6 +123,7 @@ pub(crate) fn type_check<'gcx, 'tcx>( Some(BorrowCheckContext { universal_regions, location_table, + borrow_set, all_facts, }), &mut |cx| { @@ -141,6 +146,7 @@ fn type_check_internal<'gcx, 'tcx>( ) -> MirTypeckRegionConstraints<'tcx> { let mut checker = TypeChecker::new( infcx, + mir, mir_def_id, param_env, region_bound_pairs, @@ -592,6 +598,7 @@ struct TypeChecker<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, param_env: ty::ParamEnv<'gcx>, last_span: Span, + mir: &'a Mir<'tcx>, mir_def_id: DefId, region_bound_pairs: &'a [(ty::Region<'tcx>, GenericKind<'tcx>)], implicit_region_bound: Option>, @@ -604,6 +611,7 @@ struct BorrowCheckContext<'a, 'tcx: 'a> { universal_regions: &'a UniversalRegions<'tcx>, location_table: &'a LocationTable, all_facts: &'a mut Option, + borrow_set: &'a BorrowSet<'tcx>, } /// A collection of region constraints that must be satisfied for the @@ -704,6 +712,7 @@ impl Locations { impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { fn new( infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, + mir: &'a Mir<'tcx>, mir_def_id: DefId, param_env: ty::ParamEnv<'gcx>, region_bound_pairs: &'a [(ty::Region<'tcx>, GenericKind<'tcx>)], @@ -713,6 +722,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { TypeChecker { infcx, last_span: DUMMY_SP, + mir, mir_def_id, param_env, region_bound_pairs, @@ -857,8 +867,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } StatementKind::UserAssertTy(ref c_ty, ref local) => { let local_ty = mir.local_decls()[*local].ty; - let (ty, _) = self - .infcx + let (ty, _) = self.infcx .instantiate_canonical_with_fresh_inference_vars(stmt.source_info.span, c_ty); debug!( "check_stmt: user_assert_ty ty={:?} local_ty={:?}", @@ -1400,9 +1409,12 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { CastKind::Misc => {} }, + Rvalue::Ref(region, _borrow_kind, borrowed_place) => { + self.add_reborrow_constraint(location, region, borrowed_place); + } + // FIXME: These other cases have to be implemented in future PRs Rvalue::Use(..) - | Rvalue::Ref(..) | Rvalue::Len(..) | Rvalue::BinaryOp(..) | Rvalue::CheckedBinaryOp(..) @@ -1457,6 +1469,141 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } + /// Add the constraints that arise from a borrow expression `&'a P` at the location `L`. + /// + /// # Parameters + /// + /// - `location`: the location `L` where the borrow expression occurs + /// - `borrow_region`: the region `'a` associated with the borrow + /// - `borrowed_place`: the place `P` being borrowed + fn add_reborrow_constraint( + &mut self, + location: Location, + borrow_region: ty::Region<'tcx>, + borrowed_place: &Place<'tcx>, + ) { + // These constraints are only meaningful during borrowck: + let BorrowCheckContext { + borrow_set, + location_table, + all_facts, + .. + } = match &mut self.borrowck_context { + Some(borrowck_context) => borrowck_context, + None => return, + }; + + // In Polonius mode, we also push a `borrow_region` fact + // linking the loan to the region (in some cases, though, + // there is no loan associated with this borrow expression -- + // that occurs when we are borrowing an unsafe place, for + // example). + if let Some(all_facts) = all_facts { + if let Some(borrow_index) = borrow_set.location_map.get(&location) { + let region_vid = borrow_region.to_region_vid(); + all_facts.borrow_region.push(( + region_vid, + *borrow_index, + location_table.mid_index(location), + )); + } + } + + // If we are reborrowing the referent of another reference, we + // need to add outlives relationships. In a case like `&mut + // *p`, where the `p` has type `&'b mut Foo`, for example, we + // need to ensure that `'b: 'a`. + + let mut borrowed_place = borrowed_place; + + debug!( + "add_reborrow_constraint({:?}, {:?}, {:?})", + location, borrow_region, borrowed_place + ); + while let Place::Projection(box PlaceProjection { base, elem }) = borrowed_place { + debug!("add_reborrow_constraint - iteration {:?}", borrowed_place); + + match *elem { + ProjectionElem::Deref => { + let tcx = self.infcx.tcx; + let base_ty = base.ty(self.mir, tcx).to_ty(tcx); + + debug!("add_reborrow_constraint - base_ty = {:?}", base_ty); + match base_ty.sty { + ty::TyRef(ref_region, _, mutbl) => { + self.constraints + .outlives_constraints + .push(OutlivesConstraint { + sup: ref_region.to_region_vid(), + sub: borrow_region.to_region_vid(), + locations: location.boring(), + }); + + if let Some(all_facts) = all_facts { + all_facts.outlives.push(( + ref_region.to_region_vid(), + borrow_region.to_region_vid(), + location_table.mid_index(location), + )); + } + + match mutbl { + hir::Mutability::MutImmutable => { + // Immutable reference. We don't need the base + // to be valid for the entire lifetime of + // the borrow. + break; + } + hir::Mutability::MutMutable => { + // Mutable reference. We *do* need the base + // to be valid, because after the base becomes + // invalid, someone else can use our mutable deref. + + // This is in order to make the following function + // illegal: + // ``` + // fn unsafe_deref<'a, 'b>(x: &'a &'b mut T) -> &'b mut T { + // &mut *x + // } + // ``` + // + // As otherwise you could clone `&mut T` using the + // following function: + // ``` + // fn bad(x: &mut T) -> (&mut T, &mut T) { + // let my_clone = unsafe_deref(&'a x); + // ENDREGION 'a; + // (my_clone, x) + // } + // ``` + } + } + } + ty::TyRawPtr(..) => { + // deref of raw pointer, guaranteed to be valid + break; + } + ty::TyAdt(def, _) if def.is_box() => { + // deref of `Box`, need the base to be valid - propagate + } + _ => bug!("unexpected deref ty {:?} in {:?}", base_ty, borrowed_place), + } + } + ProjectionElem::Field(..) + | ProjectionElem::Downcast(..) + | ProjectionElem::Index(..) + | ProjectionElem::ConstantIndex { .. } + | ProjectionElem::Subslice { .. } => { + // other field access + } + } + + // The "propagate" case. We need to check that our base is valid + // for the borrow's lifetime. + borrowed_place = base; + } + } + fn prove_aggregate_predicates( &mut self, aggregate_kind: &AggregateKind<'tcx>, From 537c6678d2c30a5f6a74eb8ac6ba64ca88154f95 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 1 Jul 2018 10:29:18 -0400 Subject: [PATCH 04/14] rename `constraint_set` to `constraints` also promote to its own directory, make local to nll --- .../borrow_check/nll/{constraint_set.rs => constraints/mod.rs} | 0 src/librustc_mir/borrow_check/nll/mod.rs | 2 +- src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs | 2 +- src/librustc_mir/borrow_check/nll/region_infer/mod.rs | 2 +- .../borrow_check/nll/type_check/constraint_conversion.rs | 3 +-- src/librustc_mir/borrow_check/nll/type_check/mod.rs | 2 +- 6 files changed, 5 insertions(+), 6 deletions(-) rename src/librustc_mir/borrow_check/nll/{constraint_set.rs => constraints/mod.rs} (100%) diff --git a/src/librustc_mir/borrow_check/nll/constraint_set.rs b/src/librustc_mir/borrow_check/nll/constraints/mod.rs similarity index 100% rename from src/librustc_mir/borrow_check/nll/constraint_set.rs rename to src/librustc_mir/borrow_check/nll/constraints/mod.rs diff --git a/src/librustc_mir/borrow_check/nll/mod.rs b/src/librustc_mir/borrow_check/nll/mod.rs index 1891cac268c52..26298a289fc61 100644 --- a/src/librustc_mir/borrow_check/nll/mod.rs +++ b/src/librustc_mir/borrow_check/nll/mod.rs @@ -45,7 +45,7 @@ mod renumber; crate mod type_check; mod universal_regions; -crate mod constraint_set; +mod constraints; use self::facts::AllFacts; use self::region_infer::RegionInferenceContext; diff --git a/src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs b/src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs index 0116fbcfc8860..8be51257cd6cb 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs @@ -17,7 +17,7 @@ use rustc_data_structures::indexed_vec::Idx; use std::borrow::Cow; use std::io::{self, Write}; use super::*; -use borrow_check::nll::constraint_set::OutlivesConstraint; +use borrow_check::nll::constraints::OutlivesConstraint; impl<'tcx> RegionInferenceContext<'tcx> { diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 13c5d6c05893a..3db525573b00b 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -9,7 +9,7 @@ // except according to those terms. use super::universal_regions::UniversalRegions; -use borrow_check::nll::constraint_set::{ +use borrow_check::nll::constraints::{ ConstraintIndex, ConstraintGraph, ConstraintSet, OutlivesConstraint }; use borrow_check::nll::type_check::Locations; diff --git a/src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs b/src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs index a5a159fbb1c3d..64a61972a2206 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs @@ -9,12 +9,11 @@ // except according to those terms. use borrow_check::location::LocationTable; -use borrow_check::nll::constraint_set::OutlivesConstraint; +use borrow_check::nll::constraints::{ConstraintSet, OutlivesConstraint}; use borrow_check::nll::facts::AllFacts; use borrow_check::nll::region_infer::{RegionTest, TypeTest}; use borrow_check::nll::type_check::Locations; use borrow_check::nll::universal_regions::UniversalRegions; -use borrow_check::nll::constraint_set::ConstraintSet; use rustc::infer::canonical::QueryRegionConstraint; use rustc::infer::outlives::obligations::{TypeOutlives, TypeOutlivesDelegate}; use rustc::infer::region_constraints::{GenericKind, VerifyBound}; diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 97a74e0d3365b..25f2be231772d 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -13,7 +13,7 @@ use borrow_check::borrow_set::BorrowSet; use borrow_check::location::LocationTable; -use borrow_check::nll::constraint_set::{ConstraintSet, OutlivesConstraint}; +use borrow_check::nll::constraints::{ConstraintSet, OutlivesConstraint}; use borrow_check::nll::facts::AllFacts; use borrow_check::nll::region_infer::{ClosureRegionRequirementsExt, TypeTest}; use borrow_check::nll::universal_regions::UniversalRegions; From 6be4691b6f20af1be99d98d86ed150436ecb7901 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 1 Jul 2018 16:54:01 -0400 Subject: [PATCH 05/14] deconstruct the `ControlFlowGraph` trait into more granular traits --- src/librustc/mir/mod.rs | 17 +++-- .../control_flow_graph/dominators/mod.rs | 67 ++++++++++--------- .../control_flow_graph/iterate/mod.rs | 31 ++++++--- .../control_flow_graph/mod.rs | 57 +++++++++++++--- .../control_flow_graph/reference.rs | 22 ++++-- 5 files changed, 130 insertions(+), 64 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index dca0d4f442a4c..3831ea0e6f94d 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -22,7 +22,7 @@ use mir::visit::MirVisitable; use rustc_apfloat::ieee::{Double, Single}; use rustc_apfloat::Float; use rustc_data_structures::control_flow_graph::dominators::{dominators, Dominators}; -use rustc_data_structures::control_flow_graph::ControlFlowGraph; +use rustc_data_structures::control_flow_graph; use rustc_data_structures::control_flow_graph::{GraphPredecessors, GraphSuccessors}; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; use rustc_data_structures::small_vec::SmallVec; @@ -2277,23 +2277,32 @@ fn item_path_str(def_id: DefId) -> String { ty::tls::with(|tcx| tcx.item_path_str(def_id)) } -impl<'tcx> ControlFlowGraph for Mir<'tcx> { +impl<'tcx> control_flow_graph::DirectedGraph for Mir<'tcx> { type Node = BasicBlock; +} +impl<'tcx> control_flow_graph::WithNumNodes for Mir<'tcx> { fn num_nodes(&self) -> usize { self.basic_blocks.len() } +} +impl<'tcx> control_flow_graph::WithStartNode for Mir<'tcx> { fn start_node(&self) -> Self::Node { START_BLOCK } +} +impl<'tcx> control_flow_graph::WithPredecessors for Mir<'tcx> { fn predecessors<'graph>( &'graph self, node: Self::Node, ) -> >::Iter { self.predecessors_for(node).clone().into_iter() } +} + +impl<'tcx> control_flow_graph::WithSuccessors for Mir<'tcx> { fn successors<'graph>( &'graph self, node: Self::Node, @@ -2302,12 +2311,12 @@ impl<'tcx> ControlFlowGraph for Mir<'tcx> { } } -impl<'a, 'b> GraphPredecessors<'b> for Mir<'a> { +impl<'a, 'b> control_flow_graph::GraphPredecessors<'b> for Mir<'a> { type Item = BasicBlock; type Iter = IntoIter; } -impl<'a, 'b> GraphSuccessors<'b> for Mir<'a> { +impl<'a, 'b> control_flow_graph::GraphSuccessors<'b> for Mir<'a> { type Item = BasicBlock; type Iter = iter::Cloned>; } diff --git a/src/librustc_data_structures/control_flow_graph/dominators/mod.rs b/src/librustc_data_structures/control_flow_graph/dominators/mod.rs index 54407658e6ccc..d134fad2855bb 100644 --- a/src/librustc_data_structures/control_flow_graph/dominators/mod.rs +++ b/src/librustc_data_structures/control_flow_graph/dominators/mod.rs @@ -14,9 +14,9 @@ //! Rice Computer Science TS-06-33870 //! -use super::ControlFlowGraph; +use super::super::indexed_vec::{Idx, IndexVec}; use super::iterate::reverse_post_order; -use super::super::indexed_vec::{IndexVec, Idx}; +use super::ControlFlowGraph; use std::fmt; @@ -29,15 +29,16 @@ pub fn dominators(graph: &G) -> Dominators { dominators_given_rpo(graph, &rpo) } -pub fn dominators_given_rpo(graph: &G, - rpo: &[G::Node]) - -> Dominators { +pub fn dominators_given_rpo( + graph: &G, + rpo: &[G::Node], +) -> Dominators { let start_node = graph.start_node(); assert_eq!(rpo[0], start_node); // compute the post order index (rank) for each node - let mut post_order_rank: IndexVec = IndexVec::from_elem_n(usize::default(), - graph.num_nodes()); + let mut post_order_rank: IndexVec = + IndexVec::from_elem_n(usize::default(), graph.num_nodes()); for (index, node) in rpo.iter().rev().cloned().enumerate() { post_order_rank[node] = index; } @@ -56,10 +57,12 @@ pub fn dominators_given_rpo(graph: &G, if immediate_dominators[pred].is_some() { // (*) // (*) dominators for `pred` have been calculated - new_idom = intersect_opt(&post_order_rank, - &immediate_dominators, - new_idom, - Some(pred)); + new_idom = intersect_opt( + &post_order_rank, + &immediate_dominators, + new_idom, + Some(pred), + ); } } @@ -76,11 +79,12 @@ pub fn dominators_given_rpo(graph: &G, } } -fn intersect_opt(post_order_rank: &IndexVec, - immediate_dominators: &IndexVec>, - node1: Option, - node2: Option) - -> Option { +fn intersect_opt( + post_order_rank: &IndexVec, + immediate_dominators: &IndexVec>, + node1: Option, + node2: Option, +) -> Option { match (node1, node2) { (None, None) => None, (Some(n), None) | (None, Some(n)) => Some(n), @@ -88,11 +92,12 @@ fn intersect_opt(post_order_rank: &IndexVec, } } -fn intersect(post_order_rank: &IndexVec, - immediate_dominators: &IndexVec>, - mut node1: Node, - mut node2: Node) - -> Node { +fn intersect( + post_order_rank: &IndexVec, + immediate_dominators: &IndexVec>, + mut node1: Node, + mut node2: Node, +) -> Node { while node1 != node2 { while post_order_rank[node1] < post_order_rank[node2] { node1 = immediate_dominators[node1].unwrap(); @@ -176,11 +181,13 @@ impl DominatorTree { impl fmt::Debug for DominatorTree { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - fmt::Debug::fmt(&DominatorTreeNode { - tree: self, - node: self.root, - }, - fmt) + fmt::Debug::fmt( + &DominatorTreeNode { + tree: self, + node: self.root, + }, + fmt, + ) } } @@ -194,11 +201,9 @@ impl<'tree, Node: Idx> fmt::Debug for DominatorTreeNode<'tree, Node> { let subtrees: Vec<_> = self.tree .children(self.node) .iter() - .map(|&child| { - DominatorTreeNode { - tree: self.tree, - node: child, - } + .map(|&child| DominatorTreeNode { + tree: self.tree, + node: child, }) .collect(); fmt.debug_tuple("") diff --git a/src/librustc_data_structures/control_flow_graph/iterate/mod.rs b/src/librustc_data_structures/control_flow_graph/iterate/mod.rs index 2d70b4063426d..3afdc88d60279 100644 --- a/src/librustc_data_structures/control_flow_graph/iterate/mod.rs +++ b/src/librustc_data_structures/control_flow_graph/iterate/mod.rs @@ -8,20 +8,24 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use super::ControlFlowGraph; use super::super::indexed_vec::IndexVec; +use super::{DirectedGraph, WithSuccessors, WithNumNodes}; #[cfg(test)] mod test; -pub fn post_order_from(graph: &G, start_node: G::Node) -> Vec { +pub fn post_order_from( + graph: &G, + start_node: G::Node, +) -> Vec { post_order_from_to(graph, start_node, None) } -pub fn post_order_from_to(graph: &G, - start_node: G::Node, - end_node: Option) - -> Vec { +pub fn post_order_from_to( + graph: &G, + start_node: G::Node, + end_node: Option, +) -> Vec { let mut visited: IndexVec = IndexVec::from_elem_n(false, graph.num_nodes()); let mut result: Vec = Vec::with_capacity(graph.num_nodes()); if let Some(end_node) = end_node { @@ -31,10 +35,12 @@ pub fn post_order_from_to(graph: &G, result } -fn post_order_walk(graph: &G, - node: G::Node, - result: &mut Vec, - visited: &mut IndexVec) { +fn post_order_walk( + graph: &G, + node: G::Node, + result: &mut Vec, + visited: &mut IndexVec, +) { if visited[node] { return; } @@ -47,7 +53,10 @@ fn post_order_walk(graph: &G, result.push(node); } -pub fn reverse_post_order(graph: &G, start_node: G::Node) -> Vec { +pub fn reverse_post_order( + graph: &G, + start_node: G::Node, +) -> Vec { let mut vec = post_order_from(graph, start_node); vec.reverse(); vec diff --git a/src/librustc_data_structures/control_flow_graph/mod.rs b/src/librustc_data_structures/control_flow_graph/mod.rs index 7bf776675c6a0..cfb4b07b50582 100644 --- a/src/librustc_data_structures/control_flow_graph/mod.rs +++ b/src/librustc_data_structures/control_flow_graph/mod.rs @@ -17,26 +17,61 @@ mod reference; #[cfg(test)] mod test; -pub trait ControlFlowGraph - where Self: for<'graph> GraphPredecessors<'graph, Item=::Node>, - Self: for<'graph> GraphSuccessors<'graph, Item=::Node> -{ +pub trait DirectedGraph { type Node: Idx; +} +pub trait WithNumNodes: DirectedGraph { fn num_nodes(&self) -> usize; - fn start_node(&self) -> Self::Node; - fn predecessors<'graph>(&'graph self, node: Self::Node) - -> >::Iter; - fn successors<'graph>(&'graph self, node: Self::Node) - -> >::Iter; } -pub trait GraphPredecessors<'graph> { +pub trait WithSuccessors: DirectedGraph +where + Self: for<'graph> GraphSuccessors<'graph, Item = ::Node>, +{ + fn successors<'graph>( + &'graph self, + node: Self::Node, + ) -> >::Iter; +} + +pub trait GraphSuccessors<'graph> { type Item; type Iter: Iterator; } -pub trait GraphSuccessors<'graph> { +pub trait WithPredecessors: DirectedGraph +where + Self: for<'graph> GraphPredecessors<'graph, Item = ::Node>, +{ + fn predecessors<'graph>( + &'graph self, + node: Self::Node, + ) -> >::Iter; +} + +pub trait GraphPredecessors<'graph> { type Item; type Iter: Iterator; } + +pub trait WithStartNode: DirectedGraph { + fn start_node(&self) -> Self::Node; +} + +pub trait ControlFlowGraph: + DirectedGraph + WithStartNode + WithPredecessors + WithStartNode + WithSuccessors + WithNumNodes +{ + // convenient trait +} + +impl ControlFlowGraph for T +where + T: DirectedGraph + + WithStartNode + + WithPredecessors + + WithStartNode + + WithSuccessors + + WithNumNodes, +{ +} diff --git a/src/librustc_data_structures/control_flow_graph/reference.rs b/src/librustc_data_structures/control_flow_graph/reference.rs index 3b8b01f2ff43b..a7b763db8da29 100644 --- a/src/librustc_data_structures/control_flow_graph/reference.rs +++ b/src/librustc_data_structures/control_flow_graph/reference.rs @@ -10,34 +10,42 @@ use super::*; -impl<'graph, G: ControlFlowGraph> ControlFlowGraph for &'graph G { +impl<'graph, G: DirectedGraph> DirectedGraph for &'graph G { type Node = G::Node; +} +impl<'graph, G: WithNumNodes> WithNumNodes for &'graph G { fn num_nodes(&self) -> usize { (**self).num_nodes() } +} +impl<'graph, G: WithStartNode> WithStartNode for &'graph G { fn start_node(&self) -> Self::Node { (**self).start_node() } +} + +impl<'graph, G: WithSuccessors> WithSuccessors for &'graph G { + fn successors<'iter>(&'iter self, node: Self::Node) -> >::Iter { + (**self).successors(node) + } +} +impl<'graph, G: WithPredecessors> WithPredecessors for &'graph G { fn predecessors<'iter>(&'iter self, node: Self::Node) -> >::Iter { (**self).predecessors(node) } - - fn successors<'iter>(&'iter self, node: Self::Node) -> >::Iter { - (**self).successors(node) - } } -impl<'iter, 'graph, G: ControlFlowGraph> GraphPredecessors<'iter> for &'graph G { +impl<'iter, 'graph, G: WithPredecessors> GraphPredecessors<'iter> for &'graph G { type Item = G::Node; type Iter = >::Iter; } -impl<'iter, 'graph, G: ControlFlowGraph> GraphSuccessors<'iter> for &'graph G { +impl<'iter, 'graph, G: WithSuccessors> GraphSuccessors<'iter> for &'graph G { type Item = G::Node; type Iter = >::Iter; } From fbf06a1a41c89c431e8633e3263a8f445e830b75 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 1 Jul 2018 17:06:00 -0400 Subject: [PATCH 06/14] rename `graph` to `control_flow_graph::implementation` --- src/librustc/cfg/construct.rs | 4 ++-- src/librustc/cfg/mod.rs | 2 +- src/librustc/dep_graph/query.rs | 4 +++- src/librustc/infer/lexical_region_resolve/mod.rs | 10 +++++----- src/librustc/middle/dataflow.rs | 2 +- .../implementation}/mod.rs | 0 .../implementation}/tests.rs | 0 src/librustc_data_structures/control_flow_graph/mod.rs | 1 + src/librustc_data_structures/lib.rs | 1 - src/librustc_incremental/assert_dep_graph.rs | 4 +++- 10 files changed, 16 insertions(+), 12 deletions(-) rename src/librustc_data_structures/{graph => control_flow_graph/implementation}/mod.rs (100%) rename src/librustc_data_structures/{graph => control_flow_graph/implementation}/tests.rs (100%) diff --git a/src/librustc/cfg/construct.rs b/src/librustc/cfg/construct.rs index f52d201abecb9..9712a4aa0f8a9 100644 --- a/src/librustc/cfg/construct.rs +++ b/src/librustc/cfg/construct.rs @@ -8,11 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use rustc_data_structures::graph; use cfg::*; use middle::region; -use ty::{self, TyCtxt}; +use rustc_data_structures::control_flow_graph::implementation as graph; use syntax::ptr::P; +use ty::{self, TyCtxt}; use hir::{self, PatKind}; use hir::def_id::DefId; diff --git a/src/librustc/cfg/mod.rs b/src/librustc/cfg/mod.rs index b379d3956e944..70e86c454f783 100644 --- a/src/librustc/cfg/mod.rs +++ b/src/librustc/cfg/mod.rs @@ -11,7 +11,7 @@ //! Module that constructs a control-flow graph representing an item. //! Uses `Graph` as the underlying representation. -use rustc_data_structures::graph; +use rustc_data_structures::control_flow_graph::implementation as graph; use ty::TyCtxt; use hir; use hir::def_id::DefId; diff --git a/src/librustc/dep_graph/query.rs b/src/librustc/dep_graph/query.rs index ea83a4f8b3104..c877a8b9bf827 100644 --- a/src/librustc/dep_graph/query.rs +++ b/src/librustc/dep_graph/query.rs @@ -9,7 +9,9 @@ // except according to those terms. use rustc_data_structures::fx::FxHashMap; -use rustc_data_structures::graph::{Direction, INCOMING, Graph, NodeIndex, OUTGOING}; +use rustc_data_structures::control_flow_graph::implementation::{ + Direction, INCOMING, Graph, NodeIndex, OUTGOING +}; use super::DepNode; diff --git a/src/librustc/infer/lexical_region_resolve/mod.rs b/src/librustc/infer/lexical_region_resolve/mod.rs index 5984a831e6fa0..fff1550fdbf22 100644 --- a/src/librustc/infer/lexical_region_resolve/mod.rs +++ b/src/librustc/infer/lexical_region_resolve/mod.rs @@ -20,7 +20,7 @@ use infer::region_constraints::VerifyBound; use middle::free_region::RegionRelations; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; use rustc_data_structures::fx::FxHashSet; -use rustc_data_structures::graph::{self, Direction, NodeIndex, OUTGOING}; +use rustc_data_structures::control_flow_graph::implementation::{Graph, Direction, NodeIndex, INCOMING, OUTGOING}; use std::fmt; use std::u32; use ty::{self, TyCtxt}; @@ -99,7 +99,7 @@ struct RegionAndOrigin<'tcx> { origin: SubregionOrigin<'tcx>, } -type RegionGraph<'tcx> = graph::Graph<(), Constraint<'tcx>>; +type RegionGraph<'tcx> = Graph<(), Constraint<'tcx>>; struct LexicalResolver<'cx, 'gcx: 'tcx, 'tcx: 'cx> { region_rels: &'cx RegionRelations<'cx, 'gcx, 'tcx>, @@ -501,7 +501,7 @@ impl<'cx, 'gcx, 'tcx> LexicalResolver<'cx, 'gcx, 'tcx> { fn construct_graph(&self) -> RegionGraph<'tcx> { let num_vars = self.num_vars(); - let mut graph = graph::Graph::new(); + let mut graph = Graph::new(); for _ in 0..num_vars { graph.add_node(()); @@ -550,9 +550,9 @@ impl<'cx, 'gcx, 'tcx> LexicalResolver<'cx, 'gcx, 'tcx> { // Errors in expanding nodes result from a lower-bound that is // not contained by an upper-bound. let (mut lower_bounds, lower_dup) = - self.collect_concrete_regions(graph, node_idx, graph::INCOMING, dup_vec); + self.collect_concrete_regions(graph, node_idx, INCOMING, dup_vec); let (mut upper_bounds, upper_dup) = - self.collect_concrete_regions(graph, node_idx, graph::OUTGOING, dup_vec); + self.collect_concrete_regions(graph, node_idx, OUTGOING, dup_vec); if lower_dup || upper_dup { return; diff --git a/src/librustc/middle/dataflow.rs b/src/librustc/middle/dataflow.rs index 5c86554f90790..78e8f5f9ef3ec 100644 --- a/src/librustc/middle/dataflow.rs +++ b/src/librustc/middle/dataflow.rs @@ -22,7 +22,7 @@ use std::mem; use std::usize; use syntax::print::pprust::PrintState; -use rustc_data_structures::graph::OUTGOING; +use rustc_data_structures::control_flow_graph::implementation::OUTGOING; use util::nodemap::FxHashMap; use hir; diff --git a/src/librustc_data_structures/graph/mod.rs b/src/librustc_data_structures/control_flow_graph/implementation/mod.rs similarity index 100% rename from src/librustc_data_structures/graph/mod.rs rename to src/librustc_data_structures/control_flow_graph/implementation/mod.rs diff --git a/src/librustc_data_structures/graph/tests.rs b/src/librustc_data_structures/control_flow_graph/implementation/tests.rs similarity index 100% rename from src/librustc_data_structures/graph/tests.rs rename to src/librustc_data_structures/control_flow_graph/implementation/tests.rs diff --git a/src/librustc_data_structures/control_flow_graph/mod.rs b/src/librustc_data_structures/control_flow_graph/mod.rs index cfb4b07b50582..bd933e57b392f 100644 --- a/src/librustc_data_structures/control_flow_graph/mod.rs +++ b/src/librustc_data_structures/control_flow_graph/mod.rs @@ -11,6 +11,7 @@ use super::indexed_vec::Idx; pub mod dominators; +pub mod implementation; pub mod iterate; mod reference; diff --git a/src/librustc_data_structures/lib.rs b/src/librustc_data_structures/lib.rs index e4d0bc596cba6..59b7b4300835e 100644 --- a/src/librustc_data_structures/lib.rs +++ b/src/librustc_data_structures/lib.rs @@ -59,7 +59,6 @@ pub mod small_vec; pub mod base_n; pub mod bitslice; pub mod bitvec; -pub mod graph; pub mod indexed_set; pub mod indexed_vec; pub mod obligation_forest; diff --git a/src/librustc_incremental/assert_dep_graph.rs b/src/librustc_incremental/assert_dep_graph.rs index c317d31b95ab9..39cb952a3850b 100644 --- a/src/librustc_incremental/assert_dep_graph.rs +++ b/src/librustc_incremental/assert_dep_graph.rs @@ -49,7 +49,9 @@ use rustc::dep_graph::debug::{DepNodeFilter, EdgeFilter}; use rustc::hir::def_id::DefId; use rustc::ty::TyCtxt; use rustc_data_structures::fx::FxHashSet; -use rustc_data_structures::graph::{Direction, INCOMING, OUTGOING, NodeIndex}; +use rustc_data_structures::control_flow_graph::implementation::{ + Direction, INCOMING, OUTGOING, NodeIndex +}; use rustc::hir; use rustc::hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc::ich::{ATTR_IF_THIS_CHANGED, ATTR_THEN_THIS_WOULD_NEED}; From 87f4b8d97156131befb09d030625052db3c2dc22 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 2 Jul 2018 06:14:49 -0400 Subject: [PATCH 07/14] rename `control_flow_graph` to `graph` --- src/librustc/cfg/construct.rs | 2 +- src/librustc/cfg/mod.rs | 2 +- src/librustc/dep_graph/query.rs | 2 +- .../infer/lexical_region_resolve/mod.rs | 2 +- src/librustc/middle/dataflow.rs | 2 +- src/librustc/mir/mod.rs | 20 +++++++++---------- src/librustc_codegen_llvm/mir/analyze.rs | 2 +- .../dominators/mod.rs | 0 .../dominators/test.rs | 0 .../implementation/mod.rs | 0 .../implementation/tests.rs | 0 .../iterate/mod.rs | 0 .../iterate/test.rs | 0 .../{control_flow_graph => graph}/mod.rs | 0 .../reference.rs | 0 .../{control_flow_graph => graph}/test.rs | 0 src/librustc_data_structures/lib.rs | 2 +- src/librustc_incremental/assert_dep_graph.rs | 2 +- src/librustc_mir/borrow_check/mod.rs | 2 +- .../borrow_check/nll/invalidation.rs | 2 +- src/librustc_mir/borrow_check/path_utils.rs | 2 +- 21 files changed, 21 insertions(+), 21 deletions(-) rename src/librustc_data_structures/{control_flow_graph => graph}/dominators/mod.rs (100%) rename src/librustc_data_structures/{control_flow_graph => graph}/dominators/test.rs (100%) rename src/librustc_data_structures/{control_flow_graph => graph}/implementation/mod.rs (100%) rename src/librustc_data_structures/{control_flow_graph => graph}/implementation/tests.rs (100%) rename src/librustc_data_structures/{control_flow_graph => graph}/iterate/mod.rs (100%) rename src/librustc_data_structures/{control_flow_graph => graph}/iterate/test.rs (100%) rename src/librustc_data_structures/{control_flow_graph => graph}/mod.rs (100%) rename src/librustc_data_structures/{control_flow_graph => graph}/reference.rs (100%) rename src/librustc_data_structures/{control_flow_graph => graph}/test.rs (100%) diff --git a/src/librustc/cfg/construct.rs b/src/librustc/cfg/construct.rs index 9712a4aa0f8a9..aab70456dc18d 100644 --- a/src/librustc/cfg/construct.rs +++ b/src/librustc/cfg/construct.rs @@ -10,7 +10,7 @@ use cfg::*; use middle::region; -use rustc_data_structures::control_flow_graph::implementation as graph; +use rustc_data_structures::graph::implementation as graph; use syntax::ptr::P; use ty::{self, TyCtxt}; diff --git a/src/librustc/cfg/mod.rs b/src/librustc/cfg/mod.rs index 70e86c454f783..cf9c24cc58a62 100644 --- a/src/librustc/cfg/mod.rs +++ b/src/librustc/cfg/mod.rs @@ -11,7 +11,7 @@ //! Module that constructs a control-flow graph representing an item. //! Uses `Graph` as the underlying representation. -use rustc_data_structures::control_flow_graph::implementation as graph; +use rustc_data_structures::graph::implementation as graph; use ty::TyCtxt; use hir; use hir::def_id::DefId; diff --git a/src/librustc/dep_graph/query.rs b/src/librustc/dep_graph/query.rs index c877a8b9bf827..ce0b5557a34bf 100644 --- a/src/librustc/dep_graph/query.rs +++ b/src/librustc/dep_graph/query.rs @@ -9,7 +9,7 @@ // except according to those terms. use rustc_data_structures::fx::FxHashMap; -use rustc_data_structures::control_flow_graph::implementation::{ +use rustc_data_structures::graph::implementation::{ Direction, INCOMING, Graph, NodeIndex, OUTGOING }; diff --git a/src/librustc/infer/lexical_region_resolve/mod.rs b/src/librustc/infer/lexical_region_resolve/mod.rs index fff1550fdbf22..120b45ec01e5e 100644 --- a/src/librustc/infer/lexical_region_resolve/mod.rs +++ b/src/librustc/infer/lexical_region_resolve/mod.rs @@ -20,7 +20,7 @@ use infer::region_constraints::VerifyBound; use middle::free_region::RegionRelations; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; use rustc_data_structures::fx::FxHashSet; -use rustc_data_structures::control_flow_graph::implementation::{Graph, Direction, NodeIndex, INCOMING, OUTGOING}; +use rustc_data_structures::graph::implementation::{Graph, Direction, NodeIndex, INCOMING, OUTGOING}; use std::fmt; use std::u32; use ty::{self, TyCtxt}; diff --git a/src/librustc/middle/dataflow.rs b/src/librustc/middle/dataflow.rs index 78e8f5f9ef3ec..b949fd02126ba 100644 --- a/src/librustc/middle/dataflow.rs +++ b/src/librustc/middle/dataflow.rs @@ -22,7 +22,7 @@ use std::mem; use std::usize; use syntax::print::pprust::PrintState; -use rustc_data_structures::control_flow_graph::implementation::OUTGOING; +use rustc_data_structures::graph::implementation::OUTGOING; use util::nodemap::FxHashMap; use hir; diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 3831ea0e6f94d..9cf12dbfbb2da 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -21,9 +21,9 @@ use mir::interpret::{EvalErrorKind, Scalar, Value}; use mir::visit::MirVisitable; use rustc_apfloat::ieee::{Double, Single}; use rustc_apfloat::Float; -use rustc_data_structures::control_flow_graph::dominators::{dominators, Dominators}; -use rustc_data_structures::control_flow_graph; -use rustc_data_structures::control_flow_graph::{GraphPredecessors, GraphSuccessors}; +use rustc_data_structures::graph::dominators::{dominators, Dominators}; +use rustc_data_structures::graph; +use rustc_data_structures::graph::{GraphPredecessors, GraphSuccessors}; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; use rustc_data_structures::small_vec::SmallVec; use rustc_data_structures::sync::Lrc; @@ -2277,23 +2277,23 @@ fn item_path_str(def_id: DefId) -> String { ty::tls::with(|tcx| tcx.item_path_str(def_id)) } -impl<'tcx> control_flow_graph::DirectedGraph for Mir<'tcx> { +impl<'tcx> graph::DirectedGraph for Mir<'tcx> { type Node = BasicBlock; } -impl<'tcx> control_flow_graph::WithNumNodes for Mir<'tcx> { +impl<'tcx> graph::WithNumNodes for Mir<'tcx> { fn num_nodes(&self) -> usize { self.basic_blocks.len() } } -impl<'tcx> control_flow_graph::WithStartNode for Mir<'tcx> { +impl<'tcx> graph::WithStartNode for Mir<'tcx> { fn start_node(&self) -> Self::Node { START_BLOCK } } -impl<'tcx> control_flow_graph::WithPredecessors for Mir<'tcx> { +impl<'tcx> graph::WithPredecessors for Mir<'tcx> { fn predecessors<'graph>( &'graph self, node: Self::Node, @@ -2302,7 +2302,7 @@ impl<'tcx> control_flow_graph::WithPredecessors for Mir<'tcx> { } } -impl<'tcx> control_flow_graph::WithSuccessors for Mir<'tcx> { +impl<'tcx> graph::WithSuccessors for Mir<'tcx> { fn successors<'graph>( &'graph self, node: Self::Node, @@ -2311,12 +2311,12 @@ impl<'tcx> control_flow_graph::WithSuccessors for Mir<'tcx> { } } -impl<'a, 'b> control_flow_graph::GraphPredecessors<'b> for Mir<'a> { +impl<'a, 'b> graph::GraphPredecessors<'b> for Mir<'a> { type Item = BasicBlock; type Iter = IntoIter; } -impl<'a, 'b> control_flow_graph::GraphSuccessors<'b> for Mir<'a> { +impl<'a, 'b> graph::GraphSuccessors<'b> for Mir<'a> { type Item = BasicBlock; type Iter = iter::Cloned>; } diff --git a/src/librustc_codegen_llvm/mir/analyze.rs b/src/librustc_codegen_llvm/mir/analyze.rs index 9e5298eb736a3..efd829c283f06 100644 --- a/src/librustc_codegen_llvm/mir/analyze.rs +++ b/src/librustc_codegen_llvm/mir/analyze.rs @@ -12,7 +12,7 @@ //! which do not. use rustc_data_structures::bitvec::BitVector; -use rustc_data_structures::control_flow_graph::dominators::Dominators; +use rustc_data_structures::graph::dominators::Dominators; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; use rustc::mir::{self, Location, TerminatorKind}; use rustc::mir::visit::{Visitor, PlaceContext}; diff --git a/src/librustc_data_structures/control_flow_graph/dominators/mod.rs b/src/librustc_data_structures/graph/dominators/mod.rs similarity index 100% rename from src/librustc_data_structures/control_flow_graph/dominators/mod.rs rename to src/librustc_data_structures/graph/dominators/mod.rs diff --git a/src/librustc_data_structures/control_flow_graph/dominators/test.rs b/src/librustc_data_structures/graph/dominators/test.rs similarity index 100% rename from src/librustc_data_structures/control_flow_graph/dominators/test.rs rename to src/librustc_data_structures/graph/dominators/test.rs diff --git a/src/librustc_data_structures/control_flow_graph/implementation/mod.rs b/src/librustc_data_structures/graph/implementation/mod.rs similarity index 100% rename from src/librustc_data_structures/control_flow_graph/implementation/mod.rs rename to src/librustc_data_structures/graph/implementation/mod.rs diff --git a/src/librustc_data_structures/control_flow_graph/implementation/tests.rs b/src/librustc_data_structures/graph/implementation/tests.rs similarity index 100% rename from src/librustc_data_structures/control_flow_graph/implementation/tests.rs rename to src/librustc_data_structures/graph/implementation/tests.rs diff --git a/src/librustc_data_structures/control_flow_graph/iterate/mod.rs b/src/librustc_data_structures/graph/iterate/mod.rs similarity index 100% rename from src/librustc_data_structures/control_flow_graph/iterate/mod.rs rename to src/librustc_data_structures/graph/iterate/mod.rs diff --git a/src/librustc_data_structures/control_flow_graph/iterate/test.rs b/src/librustc_data_structures/graph/iterate/test.rs similarity index 100% rename from src/librustc_data_structures/control_flow_graph/iterate/test.rs rename to src/librustc_data_structures/graph/iterate/test.rs diff --git a/src/librustc_data_structures/control_flow_graph/mod.rs b/src/librustc_data_structures/graph/mod.rs similarity index 100% rename from src/librustc_data_structures/control_flow_graph/mod.rs rename to src/librustc_data_structures/graph/mod.rs diff --git a/src/librustc_data_structures/control_flow_graph/reference.rs b/src/librustc_data_structures/graph/reference.rs similarity index 100% rename from src/librustc_data_structures/control_flow_graph/reference.rs rename to src/librustc_data_structures/graph/reference.rs diff --git a/src/librustc_data_structures/control_flow_graph/test.rs b/src/librustc_data_structures/graph/test.rs similarity index 100% rename from src/librustc_data_structures/control_flow_graph/test.rs rename to src/librustc_data_structures/graph/test.rs diff --git a/src/librustc_data_structures/lib.rs b/src/librustc_data_structures/lib.rs index 59b7b4300835e..c4e04c758311d 100644 --- a/src/librustc_data_structures/lib.rs +++ b/src/librustc_data_structures/lib.rs @@ -70,7 +70,7 @@ pub mod transitive_relation; pub use ena::unify; pub mod fx; pub mod tuple_slice; -pub mod control_flow_graph; +pub mod graph; pub mod flock; pub mod sync; pub mod owning_ref; diff --git a/src/librustc_incremental/assert_dep_graph.rs b/src/librustc_incremental/assert_dep_graph.rs index 39cb952a3850b..a78a2008eecc5 100644 --- a/src/librustc_incremental/assert_dep_graph.rs +++ b/src/librustc_incremental/assert_dep_graph.rs @@ -49,7 +49,7 @@ use rustc::dep_graph::debug::{DepNodeFilter, EdgeFilter}; use rustc::hir::def_id::DefId; use rustc::ty::TyCtxt; use rustc_data_structures::fx::FxHashSet; -use rustc_data_structures::control_flow_graph::implementation::{ +use rustc_data_structures::graph::implementation::{ Direction, INCOMING, OUTGOING, NodeIndex }; use rustc::hir; diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 3aaa3378bb005..9fec3894cb590 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -23,7 +23,7 @@ use rustc::mir::{Terminator, TerminatorKind}; use rustc::ty::query::Providers; use rustc::ty::{self, ParamEnv, TyCtxt}; -use rustc_data_structures::control_flow_graph::dominators::Dominators; +use rustc_data_structures::graph::dominators::Dominators; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::indexed_set::IdxSetBuf; use rustc_data_structures::indexed_vec::Idx; diff --git a/src/librustc_mir/borrow_check/nll/invalidation.rs b/src/librustc_mir/borrow_check/nll/invalidation.rs index 46026cdc94121..301999cc4a51e 100644 --- a/src/librustc_mir/borrow_check/nll/invalidation.rs +++ b/src/librustc_mir/borrow_check/nll/invalidation.rs @@ -29,7 +29,7 @@ use rustc::mir::{Terminator, TerminatorKind}; use rustc::mir::{Field, Operand, BorrowKind}; use rustc::ty::{self, ParamEnv}; use rustc_data_structures::indexed_vec::Idx; -use rustc_data_structures::control_flow_graph::dominators::Dominators; +use rustc_data_structures::graph::dominators::Dominators; pub(super) fn generate_invalidates<'cx, 'gcx, 'tcx>( infcx: &InferCtxt<'cx, 'gcx, 'tcx>, diff --git a/src/librustc_mir/borrow_check/path_utils.rs b/src/librustc_mir/borrow_check/path_utils.rs index 8ae98bde00344..3f5d1a2250619 100644 --- a/src/librustc_mir/borrow_check/path_utils.rs +++ b/src/librustc_mir/borrow_check/path_utils.rs @@ -16,7 +16,7 @@ use dataflow::indexes::BorrowIndex; use rustc::mir::{BasicBlock, Location, Mir, Place}; use rustc::mir::{ProjectionElem, BorrowKind}; use rustc::ty::TyCtxt; -use rustc_data_structures::control_flow_graph::dominators::Dominators; +use rustc_data_structures::graph::dominators::Dominators; /// Returns true if the borrow represented by `kind` is /// allowed to be split into separate Reservation and From 3895ff3a06a99a39da3d23ce6217d72f1287b0c0 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 2 Jul 2018 10:40:03 -0400 Subject: [PATCH 08/14] introduce a generic SCC computation --- .../graph/implementation/tests.rs | 2 +- src/librustc_data_structures/graph/mod.rs | 1 + src/librustc_data_structures/graph/scc/mod.rs | 324 ++++++++++++++++++ .../graph/scc/test.rs | 178 ++++++++++ src/librustc_data_structures/graph/test.rs | 12 +- 5 files changed, 514 insertions(+), 3 deletions(-) create mode 100644 src/librustc_data_structures/graph/scc/mod.rs create mode 100644 src/librustc_data_structures/graph/scc/test.rs diff --git a/src/librustc_data_structures/graph/implementation/tests.rs b/src/librustc_data_structures/graph/implementation/tests.rs index 007704357af4f..3814827b5df6e 100644 --- a/src/librustc_data_structures/graph/implementation/tests.rs +++ b/src/librustc_data_structures/graph/implementation/tests.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use graph::*; +use graph::implementation::*; use std::fmt::Debug; type TestGraph = Graph<&'static str, &'static str>; diff --git a/src/librustc_data_structures/graph/mod.rs b/src/librustc_data_structures/graph/mod.rs index bd933e57b392f..7265e4e8c7c66 100644 --- a/src/librustc_data_structures/graph/mod.rs +++ b/src/librustc_data_structures/graph/mod.rs @@ -14,6 +14,7 @@ pub mod dominators; pub mod implementation; pub mod iterate; mod reference; +pub mod scc; #[cfg(test)] mod test; diff --git a/src/librustc_data_structures/graph/scc/mod.rs b/src/librustc_data_structures/graph/scc/mod.rs new file mode 100644 index 0000000000000..eeec8280fc878 --- /dev/null +++ b/src/librustc_data_structures/graph/scc/mod.rs @@ -0,0 +1,324 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! Routine to compute the strongly connected components (SCCs) of a +//! graph, as well as the resulting DAG if each SCC is replaced with a +//! node in the graph. This uses Tarjan's algorithm that completes in +//! O(n) time. + +use graph::{DirectedGraph, WithNumNodes, WithSuccessors}; +use indexed_vec::{Idx, IndexVec}; +use std::ops::Range; + +mod test; + +/// Strongly connected components (SCC) of a graph. The type `N` is +/// the index type for the graph nodes and `S` is the index type for +/// the SCCs. We can map from each node to the SCC that it +/// participates in, and we also have the successors of each SCC. +pub struct Sccs { + /// For each node, what is the SCC index of the SCC to which it + /// belongs. + scc_indices: IndexVec, + + /// Data about each SCC. + scc_data: SccData, +} + +struct SccData { + /// For each SCC, the range of `all_successors` where its + /// successors can be found. + ranges: IndexVec>, + + /// Contains the succcessors for all the Sccs, concatenated. The + /// range of indices corresponding to a given SCC is found in its + /// SccData. + all_successors: Vec, +} + +impl Sccs { + pub fn new(graph: &(impl DirectedGraph + WithNumNodes + WithSuccessors)) -> Self { + SccsConstruction::construct(graph) + } + + /// Returns the number of SCCs in the graph. + pub fn num_sccs(&self) -> usize { + self.scc_data.len() + } + + /// Returns the SCC to which a node `r` belongs. + pub fn scc(&self, r: N) -> S { + self.scc_indices[r] + } + + /// Returns the successor of the given SCC. + pub fn successors(&self, scc: S) -> &[S] { + self.scc_data.successors(scc) + } +} + +impl SccData { + /// Number of SCCs, + fn len(&self) -> usize { + self.ranges.len() + } + + /// Returns the successor of the given SCC. + fn successors(&self, scc: S) -> &[S] { + // Annoyingly, `range` does not implement `Copy`, so we have + // to do `range.start..range.end`: + let range = &self.ranges[scc]; + &self.all_successors[range.start..range.end] + } + + /// Creates a new SCC with `successors` as its successors and + /// returns the resulting index. + fn create_scc(&mut self, successors: impl IntoIterator) -> S { + // Store the successors on `scc_successors_vec`, remembering + // the range of indices. + let all_successors_start = self.all_successors.len(); + self.all_successors.extend(successors); + let all_successors_end = self.all_successors.len(); + + debug!( + "create_scc({:?}) successors={:?}", + self.ranges.len(), + &self.all_successors[all_successors_start..all_successors_end], + ); + + self.ranges.push(all_successors_start..all_successors_end) + } +} + +struct SccsConstruction<'c, G: DirectedGraph + WithNumNodes + WithSuccessors + 'c, S: Idx> { + graph: &'c G, + + /// The state of each node; used during walk to record the stack + /// and after walk to record what cycle each node ended up being + /// in. + node_states: IndexVec>, + + /// The stack of nodes that we are visiting as part of the DFS. + node_stack: Vec, + + /// The stack of successors: as we visit a node, we mark our + /// position in this stack, and when we encounter a successor SCC, + /// we push it on the stack. When we complete an SCC, we can pop + /// everything off the stack that was found along the way. + successors_stack: Vec, + scc_data: SccData, +} + +#[derive(Copy, Clone, Debug)] +enum NodeState { + /// This node has not yet been visited as part of the DFS. + /// + /// After SCC construction is complete, this state ought to be + /// impossible. + NotVisited, + + /// This node is currently being walk as part of our DFS. It is on + /// the stack at the depth `depth`. + /// + /// After SCC construction is complete, this state ought to be + /// impossible. + BeingVisited { depth: usize }, + + /// Indicates that this node is a member of the given cycle. + InCycle { scc_index: S }, + + /// Indicates that this node is a member of whatever cycle + /// `parent` is a member of. This state is transient: whenever we + /// see it, we try to overwrite it with the current state of + /// `parent` (this is the "path compression" step of a union-find + /// algorithm). + InCycleWith { parent: N }, +} + +#[derive(Copy, Clone, Debug)] +enum WalkReturn { + Cycle { min_depth: usize }, + Complete { scc_index: S }, +} + +impl<'c, G, S> SccsConstruction<'c, G, S> +where + G: DirectedGraph + WithNumNodes + WithSuccessors, + S: Idx, +{ + /// Identifies SCCs in the graph `G` and computes the resulting + /// DAG. This uses a variant of [Tarjan's + /// algorithm][wikipedia]. The high-level summary of the algorithm + /// is that we do a depth-first search. Along the way, we keep a + /// stack of each node whose successors are being visited. We + /// track the depth of each node on this stack (there is no depth + /// if the node is not on the stack). When we find that some node + /// N with depth D can reach some other node N' with lower depth + /// D' (i.e., D' < D), we know that N, N', and all nodes in + /// between them on the stack are part of an SCC. + /// + /// For each node, we track the lowest depth of any successor we + /// have found, along with that + /// + /// [wikipedia]: https://bit.ly/2EZIx84 + fn construct(graph: &'c G) -> Sccs { + let num_nodes = graph.num_nodes(); + + let mut this = Self { + graph, + node_states: IndexVec::from_elem_n(NodeState::NotVisited, num_nodes), + node_stack: Vec::with_capacity(num_nodes), + successors_stack: Vec::new(), + scc_data: SccData { + ranges: IndexVec::new(), + all_successors: Vec::new(), + }, + }; + + let scc_indices = (0..num_nodes) + .map(G::Node::new) + .map(|node| match this.walk_node(0, node) { + WalkReturn::Complete { scc_index } => scc_index, + WalkReturn::Cycle { min_depth } => panic!( + "`walk_node(0, {:?})` returned cycle with depth {:?}", + node, min_depth + ), + }) + .collect(); + + Sccs { + scc_indices, + scc_data: this.scc_data, + } + } + + fn walk_node(&mut self, depth: usize, node: G::Node) -> WalkReturn { + debug!("walk_node(depth = {:?}, node = {:?})", depth, node); + match self.find_state(node) { + NodeState::InCycle { scc_index } => WalkReturn::Complete { scc_index }, + + NodeState::BeingVisited { depth: min_depth } => WalkReturn::Cycle { min_depth }, + + NodeState::NotVisited => self.walk_unvisited_node(depth, node), + + NodeState::InCycleWith { parent } => panic!( + "`find_state` returned `InCycleWith({:?})`, which ought to be impossible", + parent + ), + } + } + + /// Fetches the state of the node `r`. If `r` is recorded as being + /// in a cycle with some other node `r2`, then fetches the state + /// of `r2` (and updates `r` to reflect current result). This is + /// basically the "find" part of a standard union-find algorithm + /// (with path compression). + fn find_state(&mut self, r: G::Node) -> NodeState { + debug!("find_state(r = {:?} in state {:?})", r, self.node_states[r]); + match self.node_states[r] { + NodeState::InCycle { scc_index } => NodeState::InCycle { scc_index }, + NodeState::BeingVisited { depth } => NodeState::BeingVisited { depth }, + NodeState::NotVisited => NodeState::NotVisited, + NodeState::InCycleWith { parent } => { + let parent_state = self.find_state(parent); + debug!("find_state: parent_state = {:?}", parent_state); + match parent_state { + NodeState::InCycle { .. } => { + self.node_states[r] = parent_state; + parent_state + } + + NodeState::BeingVisited { depth } => { + self.node_states[r] = NodeState::InCycleWith { + parent: self.node_stack[depth], + }; + parent_state + } + + NodeState::NotVisited | NodeState::InCycleWith { .. } => { + panic!("invalid parent state: {:?}", parent_state) + } + } + } + } + } + + /// Walks a node that has never been visited before. + fn walk_unvisited_node(&mut self, depth: usize, node: G::Node) -> WalkReturn { + debug!( + "walk_unvisited_node(depth = {:?}, node = {:?})", + depth, node + ); + + debug_assert!(match self.node_states[node] { + NodeState::NotVisited => true, + _ => false, + }); + + self.node_states[node] = NodeState::BeingVisited { depth }; + self.node_stack.push(node); + + // Walk each successor of the node, looking to see if any of + // them can reach a node that is presently on the stack. If + // so, that means they can also reach us. + let mut min_depth = depth; + let mut min_cycle_root = node; + let successors_len = self.successors_stack.len(); + for successor_node in self.graph.successors(node) { + debug!( + "walk_unvisited_node: node = {:?} successor_ode = {:?}", + node, successor_node + ); + match self.walk_node(depth + 1, successor_node) { + WalkReturn::Cycle { + min_depth: successor_min_depth, + } => { + assert!(successor_min_depth <= depth); + if successor_min_depth < min_depth { + debug!( + "walk_unvisited_node: node = {:?} successor_min_depth = {:?}", + node, successor_min_depth + ); + min_depth = successor_min_depth; + min_cycle_root = successor_node; + } + } + + WalkReturn::Complete { + scc_index: successor_scc_index, + } => { + debug!( + "walk_unvisited_node: node = {:?} successor_scc_index = {:?}", + node, successor_scc_index + ); + self.successors_stack.push(successor_scc_index); + } + } + } + + let r = self.node_stack.pop(); + debug_assert_eq!(r, Some(node)); + + if min_depth == depth { + let scc_index = self.scc_data + .create_scc(self.successors_stack.drain(successors_len..)); + self.node_states[node] = NodeState::InCycle { scc_index }; + WalkReturn::Complete { scc_index } + } else { + // We are not the head of the cycle. Return back to our + // caller. They will take ownership of the + // `self.successors` data that we pushed. + self.node_states[node] = NodeState::InCycleWith { + parent: min_cycle_root, + }; + WalkReturn::Cycle { min_depth } + } + } +} diff --git a/src/librustc_data_structures/graph/scc/test.rs b/src/librustc_data_structures/graph/scc/test.rs new file mode 100644 index 0000000000000..a273d64a40c41 --- /dev/null +++ b/src/librustc_data_structures/graph/scc/test.rs @@ -0,0 +1,178 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![cfg(test)] + +use graph::test::TestGraph; +use super::*; + +#[test] +fn diamond() { + let graph = TestGraph::new(0, &[(0, 1), (0, 2), (1, 3), (2, 3)]); + let sccs: Sccs<_, usize> = Sccs::new(&graph); + assert_eq!(sccs.num_sccs(), 4); + assert_eq!(sccs.num_sccs(), 4); +} + +#[test] +fn test_big_scc() { + // The order in which things will be visited is important to this + // test. + // + // We will visit: + // + // 0 -> 1 -> 2 -> 0 + // + // and at this point detect a cycle. 2 will return back to 1 which + // will visit 3. 3 will visit 2 before the cycle is complete, and + // hence it too will return a cycle. + + /* ++-> 0 +| | +| v +| 1 -> 3 +| | | +| v | ++-- 2 <--+ + */ + let graph = TestGraph::new(0, &[ + (0, 1), + (1, 2), + (1, 3), + (2, 0), + (3, 2), + ]); + let sccs: Sccs<_, usize> = Sccs::new(&graph); + assert_eq!(sccs.num_sccs(), 1); +} + +#[test] +fn test_three_sccs() { + /* + 0 + | + v ++-> 1 3 +| | | +| v | ++-- 2 <--+ + */ + let graph = TestGraph::new(0, &[ + (0, 1), + (1, 2), + (2, 1), + (3, 2), + ]); + let sccs: Sccs<_, usize> = Sccs::new(&graph); + assert_eq!(sccs.num_sccs(), 3); + assert_eq!(sccs.scc(0), 1); + assert_eq!(sccs.scc(1), 0); + assert_eq!(sccs.scc(2), 0); + assert_eq!(sccs.scc(3), 2); + assert_eq!(sccs.successors(0), &[]); + assert_eq!(sccs.successors(1), &[0]); + assert_eq!(sccs.successors(2), &[0]); +} + +#[test] +fn test_find_state_2() { + // The order in which things will be visited is important to this + // test. It tests part of the `find_state` behavior. + // + // We will start in our DFS by visiting: + // + // 0 -> 1 -> 2 -> 1 + // + // and at this point detect a cycle. The state of 2 will thus be + // `InCycleWith { 1 }`. We will then visit the 1 -> 3 edge, which + // will attempt to visit 0 as well, thus going to the state + // `InCycleWith { 0 }`. Finally, node 1 will complete; the lowest + // depth of any successor was 3 which had depth 0, and thus it + // will be in the state `InCycleWith { 3 }`. + // + // When we finally traverse the `0 -> 4` edge and then visit node 2, + // the states of the nodes are: + // + // 0 BeingVisited { 0 } + // 1 InCycleWith { 3 } + // 2 InCycleWith { 1 } + // 3 InCycleWith { 0 } + // + // and hence 4 will traverse the links, finding an ultimate depth of 0. + // If will also collapse the states to the following: + // + // 0 BeingVisited { 0 } + // 1 InCycleWith { 3 } + // 2 InCycleWith { 1 } + // 3 InCycleWith { 0 } + + /* + /----+ + 0 <--+ | + | | | + v | | ++-> 1 -> 3 4 +| | | +| v | ++-- 2 <----+ + */ + let graph = TestGraph::new(0, &[ + (0, 1), + (0, 4), + (1, 2), + (1, 3), + (2, 1), + (3, 0), + (4, 2), + ]); + let sccs: Sccs<_, usize> = Sccs::new(&graph); + assert_eq!(sccs.num_sccs(), 1); + assert_eq!(sccs.scc(0), 0); + assert_eq!(sccs.scc(1), 0); + assert_eq!(sccs.scc(2), 0); + assert_eq!(sccs.scc(3), 0); + assert_eq!(sccs.scc(4), 0); + assert_eq!(sccs.successors(0), &[]); +} + +#[test] +fn test_find_state_3() { + /* + /----+ + 0 <--+ | + | | | + v | | ++-> 1 -> 3 4 5 +| | | | +| v | | ++-- 2 <----+-+ + */ + let graph = TestGraph::new(0, &[ + (0, 1), + (0, 4), + (1, 2), + (1, 3), + (2, 1), + (3, 0), + (4, 2), + (5, 2), + ]); + let sccs: Sccs<_, usize> = Sccs::new(&graph); + assert_eq!(sccs.num_sccs(), 2); + assert_eq!(sccs.scc(0), 0); + assert_eq!(sccs.scc(1), 0); + assert_eq!(sccs.scc(2), 0); + assert_eq!(sccs.scc(3), 0); + assert_eq!(sccs.scc(4), 0); + assert_eq!(sccs.scc(5), 1); + assert_eq!(sccs.successors(0), &[]); + assert_eq!(sccs.successors(1), &[0]); +} diff --git a/src/librustc_data_structures/graph/test.rs b/src/librustc_data_structures/graph/test.rs index f04b536bc185f..48b654726b8f2 100644 --- a/src/librustc_data_structures/graph/test.rs +++ b/src/librustc_data_structures/graph/test.rs @@ -13,7 +13,7 @@ use std::cmp::max; use std::slice; use std::iter; -use super::{ControlFlowGraph, GraphPredecessors, GraphSuccessors}; +use super::*; pub struct TestGraph { num_nodes: usize, @@ -44,23 +44,31 @@ impl TestGraph { } } -impl ControlFlowGraph for TestGraph { +impl DirectedGraph for TestGraph { type Node = usize; +} +impl WithStartNode for TestGraph { fn start_node(&self) -> usize { self.start_node } +} +impl WithNumNodes for TestGraph { fn num_nodes(&self) -> usize { self.num_nodes } +} +impl WithPredecessors for TestGraph { fn predecessors<'graph>(&'graph self, node: usize) -> >::Iter { self.predecessors[&node].iter().cloned() } +} +impl WithSuccessors for TestGraph { fn successors<'graph>(&'graph self, node: usize) -> >::Iter { self.successors[&node].iter().cloned() } From 818d6c9693d2eb24dcc9b2db4cb04ba3049d58d2 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 2 Jul 2018 10:55:44 -0400 Subject: [PATCH 09/14] strengthen `Idx` to require `Ord + Hash` You should always be able to know that any `T` where `T: Idx` can be used in a `BTreeMap` and a `FxHashMap`. --- src/librustc_data_structures/indexed_vec.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_data_structures/indexed_vec.rs b/src/librustc_data_structures/indexed_vec.rs index ad3710e9536f1..26de2191090e3 100644 --- a/src/librustc_data_structures/indexed_vec.rs +++ b/src/librustc_data_structures/indexed_vec.rs @@ -14,6 +14,7 @@ use std::slice; use std::marker::PhantomData; use std::ops::{Index, IndexMut, Range, RangeBounds}; use std::fmt; +use std::hash::Hash; use std::vec; use std::u32; @@ -22,7 +23,7 @@ use rustc_serialize as serialize; /// Represents some newtyped `usize` wrapper. /// /// (purpose: avoid mixing indexes for different bitvector domains.) -pub trait Idx: Copy + 'static + Eq + Debug { +pub trait Idx: Copy + 'static + Ord + Debug + Hash { fn new(idx: usize) -> Self; fn index(self) -> usize; } From af10f6e982de982782b884b21c6d438cb4adf932 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 2 Jul 2018 10:57:52 -0400 Subject: [PATCH 10/14] make `RegionValues` generic over its domain We used to store one value per RegionVid; we will soon be storing one value per SCC. --- .../borrow_check/nll/region_infer/mod.rs | 6 ++--- .../borrow_check/nll/region_infer/values.rs | 24 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 3db525573b00b..fcbba73411562 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -53,11 +53,11 @@ pub struct RegionInferenceContext<'tcx> { /// regions, these start out empty and steadily grow, though for /// each universally quantified region R they start out containing /// the entire CFG and `end(R)`. - liveness_constraints: RegionValues, + liveness_constraints: RegionValues, /// The final inferred values of the inference variables; `None` /// until `solve` is invoked. - inferred_values: Option, + inferred_values: Option>, /// The constraints we have accumulated and used during solving. constraints: ConstraintSet, @@ -383,7 +383,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.inferred_values = Some(inferred_values); } - fn compute_region_values(&self, _mir: &Mir<'tcx>) -> RegionValues { + fn compute_region_values(&self, _mir: &Mir<'tcx>) -> RegionValues { debug!("compute_region_values()"); debug!("compute_region_values: constraints={:#?}", { let mut constraints: Vec<_> = self.constraints.iter().collect(); diff --git a/src/librustc_mir/borrow_check/nll/region_infer/values.rs b/src/librustc_mir/borrow_check/nll/region_infer/values.rs index 1039e6d7b972c..bb4fa73ebb0d1 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/values.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/values.rs @@ -179,12 +179,12 @@ impl ToElementIndex for RegionElementIndex { /// variable. The columns consist of either universal regions or /// points in the CFG. #[derive(Clone)] -pub(super) struct RegionValues { +pub(super) struct RegionValues { elements: Rc, - matrix: SparseBitMatrix, + matrix: SparseBitMatrix, } -impl RegionValues { +impl RegionValues { /// Creates a new set of "region values" that tracks causal information. /// Each of the regions in num_region_variables will be initialized with an /// empty set of points and no causal information. @@ -197,7 +197,7 @@ impl RegionValues { Self { elements: elements.clone(), matrix: SparseBitMatrix::new( - RegionVid::new(num_region_variables), + N::new(num_region_variables), RegionElementIndex::new(elements.num_elements()), ), } @@ -205,7 +205,7 @@ impl RegionValues { /// Adds the given element to the value for the given region. Returns true if /// the element is newly added (i.e., was not already present). - pub(super) fn add_element(&mut self, r: RegionVid, elem: E) -> bool { + pub(super) fn add_element(&mut self, r: N, elem: E) -> bool { let i = self.elements.index(elem); debug!("add(r={:?}, elem={:?})", r, elem); self.matrix.add(r, i) @@ -213,19 +213,19 @@ impl RegionValues { /// Add all elements in `r_from` to `r_to` (because e.g. `r_to: /// r_from`). - pub(super) fn add_region(&mut self, r_to: RegionVid, r_from: RegionVid) -> bool { + pub(super) fn add_region(&mut self, r_to: N, r_from: N) -> bool { self.matrix.merge(r_from, r_to) } /// True if the region `r` contains the given element. - pub(super) fn contains(&self, r: RegionVid, elem: E) -> bool { + pub(super) fn contains(&self, r: N, elem: E) -> bool { let i = self.elements.index(elem); self.matrix.contains(r, i) } /// True if `sup_region` contains all the CFG points that /// `sub_region` contains. Ignores universal regions. - pub(super) fn contains_points(&self, sup_region: RegionVid, sub_region: RegionVid) -> bool { + pub(super) fn contains_points(&self, sup_region: N, sub_region: N) -> bool { // This could be done faster by comparing the bitsets. But I // am lazy. self.element_indices_contained_in(sub_region) @@ -238,7 +238,7 @@ impl RegionValues { /// `elements_contained_in`. pub(super) fn element_indices_contained_in<'a>( &'a self, - r: RegionVid, + r: N, ) -> impl Iterator + 'a { self.matrix.iter(r).map(move |i| i) } @@ -246,7 +246,7 @@ impl RegionValues { /// Returns just the universal regions that are contained in a given region's value. pub(super) fn universal_regions_outlived_by<'a>( &'a self, - r: RegionVid, + r: N, ) -> impl Iterator + 'a { self.element_indices_contained_in(r) .map(move |i| self.elements.to_universal_region(i)) @@ -257,14 +257,14 @@ impl RegionValues { /// Returns all the elements contained in a given region's value. pub(super) fn elements_contained_in<'a>( &'a self, - r: RegionVid, + r: N, ) -> impl Iterator + 'a { self.element_indices_contained_in(r) .map(move |r| self.elements.to_element(r)) } /// Returns a "pretty" string value of the region. Meant for debugging. - pub(super) fn region_value_str(&self, r: RegionVid) -> String { + pub(super) fn region_value_str(&self, r: N) -> String { let mut result = String::new(); result.push_str("{"); From 1f749b3040df049cf8ebc062d75c04f16adce316 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 2 Jul 2018 11:29:39 -0400 Subject: [PATCH 11/14] [WIP] compute region values using SCCs not iterative flow This is marked as [WIP] because it contains a number of minor refactorings that ought to be broken out. --- .../borrow_check/nll/constraint_generation.rs | 4 +- .../nll/constraints/full_graph.rs | 96 ++++++++ .../borrow_check/nll/constraints/mod.rs | 57 ++--- .../nll/explain_borrow/find_use.rs | 2 +- .../nll/region_infer/error_reporting.rs | 2 +- .../borrow_check/nll/region_infer/mod.rs | 219 ++++++++---------- .../borrow_check/nll/region_infer/values.rs | 42 ++-- src/librustc_mir/dataflow/impls/borrows.rs | 2 +- 8 files changed, 240 insertions(+), 184 deletions(-) create mode 100644 src/librustc_mir/borrow_check/nll/constraints/full_graph.rs diff --git a/src/librustc_mir/borrow_check/nll/constraint_generation.rs b/src/librustc_mir/borrow_check/nll/constraint_generation.rs index 9dcdf7de31455..68484888477c8 100644 --- a/src/librustc_mir/borrow_check/nll/constraint_generation.rs +++ b/src/librustc_mir/borrow_check/nll/constraint_generation.rs @@ -210,7 +210,7 @@ impl<'cx, 'cg, 'gcx, 'tcx> ConstraintGeneration<'cx, 'cg, 'gcx, 'tcx> { for (region, location) in liveness_set { debug!("generate: {:#?} is live at {:#?}", region, location); let region_vid = regioncx.to_region_vid(region); - regioncx.add_live_point(region_vid, *location); + regioncx.add_live_element(region_vid, *location); } if let Some(all_facts) = all_facts { @@ -242,7 +242,7 @@ impl<'cx, 'cg, 'gcx, 'tcx> ConstraintGeneration<'cx, 'cg, 'gcx, 'tcx> { .tcx .for_each_free_region(&live_ty, |live_region| { let vid = live_region.to_region_vid(); - self.regioncx.add_live_point(vid, location); + self.regioncx.add_live_element(vid, location); }); } } diff --git a/src/librustc_mir/borrow_check/nll/constraints/full_graph.rs b/src/librustc_mir/borrow_check/nll/constraints/full_graph.rs new file mode 100644 index 0000000000000..7427913888e71 --- /dev/null +++ b/src/librustc_mir/borrow_check/nll/constraints/full_graph.rs @@ -0,0 +1,96 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use borrow_check::nll::constraints::{ConstraintIndex, ConstraintSet}; +use rustc::ty::RegionVid; +use rustc_data_structures::graph; +use rustc_data_structures::indexed_vec::IndexVec; + +/// An initial graph which contains a node for every constraint index. +/// This is basically just represented as a set of linked lists; for +/// each region R, there is a linked list of constraints that lead to +/// other regions. (The graph is always defined relative to some set.) +crate struct FullConstraintGraph<'s> { + set: &'s ConstraintSet, + first_constraints: IndexVec>, + next_constraints: IndexVec>, +} + +impl<'s> FullConstraintGraph<'s> { + /// Constraint a graph where each region constraint `R1: R2` is + /// treated as an edge `R2 -> R1`. This is useful for cheaply + /// finding dirty constraints. + crate fn new(set: &'s ConstraintSet, num_region_vars: usize) -> Self { + let mut first_constraints = IndexVec::from_elem_n(None, num_region_vars); + let mut next_constraints = IndexVec::from_elem(None, &set.constraints); + + for (idx, constraint) in set.constraints.iter_enumerated().rev() { + let mut head = &mut first_constraints[constraint.sup]; + let mut next = &mut next_constraints[idx]; + debug_assert!(next.is_none()); + *next = *head; + *head = Some(idx); + } + + Self { set, first_constraints, next_constraints } + } + + crate fn sub_regions( + &self, + region_sup: RegionVid, + ) -> Successors<'_> { + let first = self.first_constraints[region_sup]; + Successors { graph: self, pointer: first } + } +} + +crate struct Successors<'s> { + graph: &'s FullConstraintGraph<'s>, + pointer: Option, +} + +impl<'s> Iterator for Successors<'s> { + type Item = RegionVid; + + fn next(&mut self) -> Option { + if let Some(p) = self.pointer { + let r = self.graph.set[p].sub; + self.pointer = self.graph.next_constraints[p]; + Some(r) + } else { + None + } + } +} + +impl<'s> graph::DirectedGraph for FullConstraintGraph<'s> { + type Node = RegionVid; +} + +impl<'s> graph::WithNumNodes for FullConstraintGraph<'s> { + fn num_nodes(&self) -> usize { + self.first_constraints.len() + } +} + +impl<'s> graph::WithSuccessors for FullConstraintGraph<'s> { + fn successors<'graph>( + &'graph self, + node: Self::Node, + ) -> >::Iter { + self.sub_regions(node) + } +} + +impl<'s, 'graph> graph::GraphSuccessors<'graph> for FullConstraintGraph<'s> { + type Item = RegionVid; + type Iter = Successors<'graph>; +} + diff --git a/src/librustc_mir/borrow_check/nll/constraints/mod.rs b/src/librustc_mir/borrow_check/nll/constraints/mod.rs index eab1de073116f..3c84701447919 100644 --- a/src/librustc_mir/borrow_check/nll/constraints/mod.rs +++ b/src/librustc_mir/borrow_check/nll/constraints/mod.rs @@ -9,19 +9,22 @@ // except according to those terms. use rustc::ty::RegionVid; +use rustc_data_structures::graph::scc::Sccs; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; use borrow_check::nll::type_check::Locations; use std::fmt; use std::ops::Deref; +mod full_graph; + #[derive(Clone, Default)] crate struct ConstraintSet { constraints: IndexVec, } impl ConstraintSet { - pub fn push(&mut self, constraint: OutlivesConstraint) { + crate fn push(&mut self, constraint: OutlivesConstraint) { debug!( "ConstraintSet::push({:?}: {:?} @ {:?}", constraint.sup, constraint.sub, constraint.locations @@ -32,12 +35,19 @@ impl ConstraintSet { } self.constraints.push(constraint); } + + crate fn compute_sccs(&self, num_region_vars: usize) -> Sccs { + let graph = &full_graph::FullConstraintGraph::new(self, num_region_vars); + Sccs::new(graph) + } } impl Deref for ConstraintSet { type Target = IndexVec; - fn deref(&self) -> &Self::Target { &self.constraints } + fn deref(&self) -> &Self::Target { + &self.constraints + } } #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -68,45 +78,4 @@ impl fmt::Debug for OutlivesConstraint { newtype_index!(ConstraintIndex { DEBUG_FORMAT = "ConstraintIndex({})" }); -crate struct ConstraintGraph { - first_constraints: IndexVec>, - next_constraints: IndexVec>, -} - -impl ConstraintGraph { - /// Constraint a graph where each region constraint `R1: R2` is - /// treated as an edge `R2 -> R1`. This is useful for cheaply - /// finding dirty constraints. - crate fn new(set: &ConstraintSet, num_region_vars: usize) -> Self { - let mut first_constraints = IndexVec::from_elem_n(None, num_region_vars); - let mut next_constraints = IndexVec::from_elem(None, &set.constraints); - - for (idx, constraint) in set.constraints.iter_enumerated().rev() { - let mut head = &mut first_constraints[constraint.sub]; - let mut next = &mut next_constraints[idx]; - debug_assert!(next.is_none()); - *next = *head; - *head = Some(idx); - } - - ConstraintGraph { first_constraints, next_constraints } - } - - /// Invokes `op` with the index of any constraints of the form - /// `region_sup: region_sub`. These are the constraints that must - /// be reprocessed when the value of `R1` changes. If you think of - /// each constraint `R1: R2` as an edge `R2 -> R1`, then this - /// gives the set of successors to R2. - crate fn for_each_dependent( - &self, - region_sub: RegionVid, - mut op: impl FnMut(ConstraintIndex), - ) { - let mut p = self.first_constraints[region_sub]; - while let Some(dep_idx) = p { - op(dep_idx); - p = self.next_constraints[dep_idx]; - } - } -} - +newtype_index!(ConstraintSccIndex { DEBUG_FORMAT = "ConstraintSccIndex({})" }); diff --git a/src/librustc_mir/borrow_check/nll/explain_borrow/find_use.rs b/src/librustc_mir/borrow_check/nll/explain_borrow/find_use.rs index a65019690e307..9fd9d6cd97c59 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/find_use.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/find_use.rs @@ -57,7 +57,7 @@ impl<'cx, 'gcx, 'tcx> UseFinder<'cx, 'gcx, 'tcx> { queue.push_back(self.start_point); while let Some(p) = queue.pop_front() { - if !self.regioncx.region_contains_point(self.region_vid, p) { + if !self.regioncx.region_contains(self.region_vid, p) { continue; } diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting.rs index 6543516b9c2c6..518bab5568dec 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting.rs @@ -318,7 +318,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { while changed { changed = false; - for constraint in &*self.constraints { + for constraint in self.constraints.iter() { if let Some(n) = result_set[constraint.sup] { let m = n + 1; if result_set[constraint.sub] diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index fcbba73411562..16c5e91938b50 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -9,9 +9,9 @@ // except according to those terms. use super::universal_regions::UniversalRegions; -use borrow_check::nll::constraints::{ - ConstraintIndex, ConstraintGraph, ConstraintSet, OutlivesConstraint -}; +use borrow_check::nll::constraints::{ConstraintIndex, ConstraintSccIndex, ConstraintSet, + OutlivesConstraint}; +use borrow_check::nll::region_infer::values::ToElementIndex; use borrow_check::nll::type_check::Locations; use rustc::hir::def_id::DefId; use rustc::infer::canonical::QueryRegionConstraint; @@ -25,7 +25,7 @@ use rustc::mir::{ }; use rustc::ty::{self, RegionVid, Ty, TyCtxt, TypeFoldable}; use rustc::util::common; -use rustc_data_structures::bitvec::BitVector; +use rustc_data_structures::graph::scc::Sccs; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; use std::rc::Rc; @@ -55,19 +55,23 @@ pub struct RegionInferenceContext<'tcx> { /// the entire CFG and `end(R)`. liveness_constraints: RegionValues, - /// The final inferred values of the inference variables; `None` - /// until `solve` is invoked. - inferred_values: Option>, + /// The outlives constraints computed by the type-check. + constraints: Rc, + + /// The SCC computed from `constraints`. + constraints_scc: Rc>, - /// The constraints we have accumulated and used during solving. - constraints: ConstraintSet, + /// The final inferred values of the region variables; we compute + /// one value per SCC. To get the value for any given *region*, + /// you first find which scc it is a part of. + scc_values: RegionValues, /// Type constraints that we check after solving. type_tests: Vec>, /// Information about the universally quantified regions in scope /// on this function and their (known) relations to one another. - universal_regions: UniversalRegions<'tcx>, + universal_regions: Rc>, } struct RegionDefinition<'tcx> { @@ -198,6 +202,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { outlives_constraints: ConstraintSet, type_tests: Vec>, ) -> Self { + let universal_regions = Rc::new(universal_regions); let num_region_variables = var_infos.len(); let num_universal_regions = universal_regions.len(); @@ -209,12 +214,18 @@ impl<'tcx> RegionInferenceContext<'tcx> { .map(|info| RegionDefinition::new(info.origin)) .collect(); + let constraints = Rc::new(outlives_constraints); // freeze constraints + let constraints_scc = Rc::new(constraints.compute_sccs(num_region_variables)); + + let scc_values = RegionValues::new(elements, constraints_scc.num_sccs()); + let mut result = Self { definitions, elements: elements.clone(), liveness_constraints: RegionValues::new(elements, num_region_variables), - inferred_values: None, - constraints: outlives_constraints, + constraints, + constraints_scc, + scc_values, type_tests, universal_regions, }; @@ -251,7 +262,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { } // For each universally quantified region X: - for variable in self.universal_regions.universal_regions() { + let elements = self.elements.clone(); + let universal_regions = self.universal_regions.clone(); + for variable in universal_regions.universal_regions() { // These should be free-region variables. assert!(match self.definitions[variable].origin { RegionVariableOrigin::NLL(NLLRegionVariableOrigin::FreeRegion) => true, @@ -261,12 +274,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.definitions[variable].is_universal = true; // Add all nodes in the CFG to liveness constraints - for point_index in self.elements.all_point_indices() { - self.liveness_constraints.add_element(variable, point_index); + for point_index in elements.all_point_indices() { + self.add_live_element(variable, point_index); } // Add `end(X)` into the set for X. - self.liveness_constraints.add_element(variable, variable); + self.add_live_element(variable, variable); } } @@ -286,37 +299,38 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// Returns true if the region `r` contains the point `p`. /// /// Panics if called before `solve()` executes, - pub fn region_contains_point(&self, r: R, p: Location) -> bool - where - R: ToRegionVid, - { - let inferred_values = self - .inferred_values - .as_ref() - .expect("region values not yet inferred"); - inferred_values.contains(r.to_region_vid(), p) + crate fn region_contains(&self, r: impl ToRegionVid, p: impl ToElementIndex) -> bool { + let scc = self.constraints_scc.scc(r.to_region_vid()); + self.scc_values.contains(scc, p) } /// Returns access to the value of `r` for debugging purposes. crate fn region_value_str(&self, r: RegionVid) -> String { - let inferred_values = self - .inferred_values - .as_ref() - .expect("region values not yet inferred"); - - inferred_values.region_value_str(r) + let scc = self.constraints_scc.scc(r.to_region_vid()); + self.scc_values.region_value_str(scc) } /// Indicates that the region variable `v` is live at the point `point`. /// /// Returns `true` if this constraint is new and `false` is the /// constraint was already present. - pub(super) fn add_live_point(&mut self, v: RegionVid, point: Location) -> bool { - debug!("add_live_point({:?}, {:?})", v, point); - assert!(self.inferred_values.is_none(), "values already inferred"); + pub(super) fn add_live_element( + &mut self, + v: RegionVid, + elem: impl ToElementIndex, + ) -> bool { + debug!("add_live_element({:?}, {:?})", v, elem); + + // Add to the liveness values for `v`... + if self.liveness_constraints.add_element(v, elem) { + // ...but also add to the SCC in which `v` appears. + let scc = self.constraints_scc.scc(v); + self.scc_values.add_element(scc, elem); - let element = self.elements.index(point); - self.liveness_constraints.add_element(v, element) + true + } else { + false + } } /// Perform region inference and report errors if we see any @@ -341,8 +355,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { mir: &Mir<'tcx>, mir_def_id: DefId, ) -> Option> { - assert!(self.inferred_values.is_none(), "values already inferred"); - self.propagate_constraints(mir); // If this is a closure, we can propagate unsatisfied @@ -377,58 +389,48 @@ impl<'tcx> RegionInferenceContext<'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>) { - assert!(self.inferred_values.is_none()); - let inferred_values = self.compute_region_values(mir); - self.inferred_values = Some(inferred_values); - } + fn propagate_constraints(&mut self, _mir: &Mir<'tcx>) { + debug!("propagate_constraints()"); - fn compute_region_values(&self, _mir: &Mir<'tcx>) -> RegionValues { - debug!("compute_region_values()"); - debug!("compute_region_values: constraints={:#?}", { + debug!("propagate_constraints: constraints={:#?}", { let mut constraints: Vec<_> = self.constraints.iter().collect(); constraints.sort(); constraints }); - // The initial values for each region are derived from the liveness - // constraints we have accumulated. - let mut inferred_values = self.liveness_constraints.clone(); - - let constraint_graph = ConstraintGraph::new(&self.constraints, self.definitions.len()); - - // Constraints that may need to be repropagated (initially all): - let mut dirty_list: Vec<_> = self.constraints.indices().collect(); - - // Set to 0 for each constraint that is on the dirty list: - let mut clean_bit_vec = BitVector::new(dirty_list.len()); + // To propagate constriants, we walk the DAG induced by the + // SCC. For each SCC, we visit its successors and compute + // their values, then we union all those values to get our + // own. + for scc_index in (0..self.constraints_scc.num_sccs()).map(ConstraintSccIndex::new) { + self.propagate_constraints_scc(scc_index); + } + } - debug!("propagate_constraints: --------------------"); - while let Some(constraint_idx) = dirty_list.pop() { - clean_bit_vec.insert(constraint_idx.index()); + fn propagate_constraints_scc(&mut self, scc_a: ConstraintSccIndex) { + debug!("propagate_constraints_scc(scc_a = {:?})", scc_a); - let constraint = &self.constraints[constraint_idx]; - debug!("propagate_constraints: constraint={:?}", constraint); + let constraints_scc = self.constraints_scc.clone(); - if inferred_values.add_region(constraint.sup, constraint.sub) { - debug!("propagate_constraints: sub={:?}", constraint.sub); - debug!("propagate_constraints: sup={:?}", constraint.sup); + // Walk each SCC `B` such that `A: B`... + for &scc_b in constraints_scc.successors(scc_a) { + debug!( + "propagate_constraints_scc: scc_a = {:?} scc_b = {:?}", + scc_a, scc_b + ); - // The region of `constraint.sup` changed, so find all - // constraints of the form `R: constriant.sup` and - // enqueue them as dirty. We will have to reprocess - // them. - constraint_graph.for_each_dependent(constraint.sup, |dep_idx| { - if clean_bit_vec.remove(dep_idx.index()) { - dirty_list.push(dep_idx); - } - }); - } + // ...compute the value of `B`... + self.propagate_constraints_scc(scc_b); - debug!("\n"); + // ...and add elements from `B` into `A`. + self.scc_values.add_region(scc_a, scc_b); } - inferred_values + debug!( + "propagate_constraints_scc: scc_a = {:?} has value {:?}", + scc_a, + self.scc_values.region_value_str(scc_a), + ); } /// Once regions have been propagated, this method is used to see @@ -503,12 +505,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { if self.universal_regions.is_universal_region(r) { return self.definitions[r].external_name; } else { - let inferred_values = self - .inferred_values - .as_ref() - .expect("region values not yet inferred"); + let r_scc = self.constraints_scc.scc(r); let upper_bound = self.universal_upper_bound(r); - if inferred_values.contains(r, upper_bound) { + if self.scc_values.contains(r_scc, upper_bound) { self.to_error_region(upper_bound) } else { None @@ -543,11 +542,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { // region, which ensures it can be encoded in a `ClosureOutlivesRequirement`. let lower_bound_plus = self.non_local_universal_upper_bound(*lower_bound); assert!(self.universal_regions.is_universal_region(lower_bound_plus)); - assert!( - !self - .universal_regions - .is_local_free_region(lower_bound_plus) - ); + assert!(!self.universal_regions + .is_local_free_region(lower_bound_plus)); propagated_outlives_requirements.push(ClosureOutlivesRequirement { subject, @@ -575,10 +571,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { ) -> Option> { let tcx = infcx.tcx; let gcx = tcx.global_tcx(); - let inferred_values = self - .inferred_values - .as_ref() - .expect("region values not yet inferred"); debug!("try_promote_type_test_subject(ty = {:?})", ty); @@ -621,7 +613,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { // `'static` is not contained in `r`, we would fail to // find an equivalent. let upper_bound = self.non_local_universal_upper_bound(region_vid); - if inferred_values.contains(region_vid, upper_bound) { + if self.region_contains(region_vid, upper_bound) { tcx.mk_region(ty::ReClosureBound(upper_bound)) } else { // In the case of a failure, use a `ReVar` @@ -654,12 +646,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// except that it converts further takes the non-local upper /// bound of `'y`, so that the final result is non-local. fn non_local_universal_upper_bound(&self, r: RegionVid) -> RegionVid { - let inferred_values = self.inferred_values.as_ref().unwrap(); - debug!( "non_local_universal_upper_bound(r={:?}={})", r, - inferred_values.region_value_str(r) + self.region_value_str(r) ); let lub = self.universal_upper_bound(r); @@ -691,18 +681,17 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// - For each `end('x)` element in `'r`, compute the mutual LUB, yielding /// a result `'y`. fn universal_upper_bound(&self, r: RegionVid) -> RegionVid { - let inferred_values = self.inferred_values.as_ref().unwrap(); - debug!( "universal_upper_bound(r={:?}={})", r, - inferred_values.region_value_str(r) + self.region_value_str(r) ); // Find the smallest universal region that contains all other // universal regions within `region`. let mut lub = self.universal_regions.fr_fn_body; - for ur in inferred_values.universal_regions_outlived_by(r) { + let r_scc = self.constraints_scc.scc(r); + for ur in self.scc_values.universal_regions_outlived_by(r_scc) { lub = self.universal_regions.postdom_upper_bound(lub, ur); } @@ -747,31 +736,29 @@ impl<'tcx> RegionInferenceContext<'tcx> { ) -> bool { debug!("eval_outlives({:?}: {:?})", sup_region, sub_region); - let inferred_values = self - .inferred_values - .as_ref() - .expect("values for regions not yet inferred"); - debug!( "eval_outlives: sup_region's value = {:?}", - inferred_values.region_value_str(sup_region), + self.region_value_str(sup_region), ); debug!( "eval_outlives: sub_region's value = {:?}", - inferred_values.region_value_str(sub_region), + self.region_value_str(sub_region), ); + let sub_region_scc = self.constraints_scc.scc(sub_region); + let sup_region_scc = self.constraints_scc.scc(sup_region); + // Both the `sub_region` and `sup_region` consist of the union // of some number of universal regions (along with the union // of various points in the CFG; ignore those points for // now). Therefore, the sup-region outlives the sub-region if, // for each universal region R1 in the sub-region, there // exists some region R2 in the sup-region that outlives R1. - let universal_outlives = inferred_values - .universal_regions_outlived_by(sub_region) + let universal_outlives = self.scc_values + .universal_regions_outlived_by(sub_region_scc) .all(|r1| { - inferred_values - .universal_regions_outlived_by(sup_region) + self.scc_values + .universal_regions_outlived_by(sup_region_scc) .any(|r2| self.universal_regions.outlives(r2, r1)) }); @@ -787,7 +774,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { return true; } - inferred_values.contains_points(sup_region, sub_region) + self.scc_values + .contains_points(sup_region_scc, sub_region_scc) } /// Once regions have been propagated, this method is used to see @@ -816,8 +804,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { ) { // The universal regions are always found in a prefix of the // full list. - let universal_definitions = self - .definitions + let universal_definitions = self.definitions .iter_enumerated() .take_while(|(_, fr_definition)| fr_definition.is_universal); @@ -851,13 +838,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { longer_fr: RegionVid, propagated_outlives_requirements: &mut Option<&mut Vec>>, ) { - let inferred_values = self.inferred_values.as_ref().unwrap(); - debug!("check_universal_region(fr={:?})", longer_fr); + let longer_fr_scc = self.constraints_scc.scc(longer_fr); + // Find every region `o` such that `fr: o` // (because `fr` includes `end(o)`). - for shorter_fr in inferred_values.universal_regions_outlived_by(longer_fr) { + for shorter_fr in self.scc_values.universal_regions_outlived_by(longer_fr_scc) { // If it is known that `fr: o`, carry on. if self.universal_regions.outlives(longer_fr, shorter_fr) { continue; diff --git a/src/librustc_mir/borrow_check/nll/region_infer/values.rs b/src/librustc_mir/borrow_check/nll/region_infer/values.rs index bb4fa73ebb0d1..c5bfb1fc6a588 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/values.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/values.rs @@ -18,7 +18,7 @@ use std::rc::Rc; /// Maps between the various kinds of elements of a region value to /// the internal indices that w use. -pub(super) struct RegionValueElements { +crate struct RegionValueElements { /// For each basic block, how many points are contained within? statements_before_block: IndexVec, num_points: usize, @@ -26,7 +26,7 @@ pub(super) struct RegionValueElements { } impl RegionValueElements { - pub(super) fn new(mir: &Mir<'_>, num_universal_regions: usize) -> Self { + crate fn new(mir: &Mir<'_>, num_universal_regions: usize) -> Self { let mut num_points = 0; let statements_before_block = mir .basic_blocks() @@ -56,22 +56,22 @@ impl RegionValueElements { } /// Total number of element indices that exist. - pub(super) fn num_elements(&self) -> usize { + crate fn num_elements(&self) -> usize { self.num_points + self.num_universal_regions } /// Converts an element of a region value into a `RegionElementIndex`. - pub(super) fn index(&self, elem: T) -> RegionElementIndex { + crate fn index(&self, elem: T) -> RegionElementIndex { elem.to_element_index(self) } /// Iterates over the `RegionElementIndex` for all points in the CFG. - pub(super) fn all_point_indices<'a>(&'a self) -> impl Iterator + 'a { + crate fn all_point_indices<'a>(&'a self) -> impl Iterator + 'a { (0..self.num_points).map(move |i| RegionElementIndex::new(i + self.num_universal_regions)) } /// Converts a particular `RegionElementIndex` to the `RegionElement` it represents. - pub(super) fn to_element(&self, i: RegionElementIndex) -> RegionElement { + crate fn to_element(&self, i: RegionElementIndex) -> RegionElement { debug!("to_element(i={:?})", i); if let Some(r) = self.to_universal_region(i) { @@ -114,7 +114,7 @@ impl RegionValueElements { /// Converts a particular `RegionElementIndex` to a universal /// region, if that is what it represents. Returns `None` /// otherwise. - pub(super) fn to_universal_region(&self, i: RegionElementIndex) -> Option { + crate fn to_universal_region(&self, i: RegionElementIndex) -> Option { if i.index() < self.num_universal_regions { Some(RegionVid::new(i.index())) } else { @@ -138,7 +138,7 @@ newtype_index!(RegionElementIndex { DEBUG_FORMAT = "RegionElementIndex({})" }); /// An individual element in a region value -- the value of a /// particular region variable consists of a set of these elements. #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub(super) enum RegionElement { +crate enum RegionElement { /// A point in the control-flow graph. Location(Location), @@ -146,7 +146,7 @@ pub(super) enum RegionElement { UniversalRegion(RegionVid), } -pub(super) trait ToElementIndex: Debug + Copy { +crate trait ToElementIndex: Debug + Copy { fn to_element_index(self, elements: &RegionValueElements) -> RegionElementIndex; } @@ -179,7 +179,7 @@ impl ToElementIndex for RegionElementIndex { /// variable. The columns consist of either universal regions or /// points in the CFG. #[derive(Clone)] -pub(super) struct RegionValues { +crate struct RegionValues { elements: Rc, matrix: SparseBitMatrix, } @@ -188,7 +188,7 @@ impl RegionValues { /// Creates a new set of "region values" that tracks causal information. /// Each of the regions in num_region_variables will be initialized with an /// empty set of points and no causal information. - pub(super) fn new(elements: &Rc, num_region_variables: usize) -> Self { + crate fn new(elements: &Rc, num_region_variables: usize) -> Self { assert!( elements.num_universal_regions <= num_region_variables, "universal regions are a subset of the region variables" @@ -205,7 +205,11 @@ impl RegionValues { /// Adds the given element to the value for the given region. Returns true if /// the element is newly added (i.e., was not already present). - pub(super) fn add_element(&mut self, r: N, elem: E) -> bool { + crate fn add_element( + &mut self, + r: N, + elem: impl ToElementIndex, + ) -> bool { let i = self.elements.index(elem); debug!("add(r={:?}, elem={:?})", r, elem); self.matrix.add(r, i) @@ -213,19 +217,19 @@ impl RegionValues { /// Add all elements in `r_from` to `r_to` (because e.g. `r_to: /// r_from`). - pub(super) fn add_region(&mut self, r_to: N, r_from: N) -> bool { + crate fn add_region(&mut self, r_to: N, r_from: N) -> bool { self.matrix.merge(r_from, r_to) } /// True if the region `r` contains the given element. - pub(super) fn contains(&self, r: N, elem: E) -> bool { + crate fn contains(&self, r: N, elem: impl ToElementIndex) -> bool { let i = self.elements.index(elem); self.matrix.contains(r, i) } /// True if `sup_region` contains all the CFG points that /// `sub_region` contains. Ignores universal regions. - pub(super) fn contains_points(&self, sup_region: N, sub_region: N) -> bool { + crate fn contains_points(&self, sup_region: N, sub_region: N) -> bool { // This could be done faster by comparing the bitsets. But I // am lazy. self.element_indices_contained_in(sub_region) @@ -236,7 +240,7 @@ impl RegionValues { /// Iterate over the value of the region `r`, yielding up element /// indices. You may prefer `universal_regions_outlived_by` or /// `elements_contained_in`. - pub(super) fn element_indices_contained_in<'a>( + crate fn element_indices_contained_in<'a>( &'a self, r: N, ) -> impl Iterator + 'a { @@ -244,7 +248,7 @@ impl RegionValues { } /// Returns just the universal regions that are contained in a given region's value. - pub(super) fn universal_regions_outlived_by<'a>( + crate fn universal_regions_outlived_by<'a>( &'a self, r: N, ) -> impl Iterator + 'a { @@ -255,7 +259,7 @@ impl RegionValues { } /// Returns all the elements contained in a given region's value. - pub(super) fn elements_contained_in<'a>( + crate fn elements_contained_in<'a>( &'a self, r: N, ) -> impl Iterator + 'a { @@ -264,7 +268,7 @@ impl RegionValues { } /// Returns a "pretty" string value of the region. Meant for debugging. - pub(super) fn region_value_str(&self, r: N) -> String { + crate fn region_value_str(&self, r: N) -> String { let mut result = String::new(); result.push_str("{"); diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index a109389aa312c..9736ab797b2c1 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -76,7 +76,7 @@ fn precompute_borrows_out_of_scope<'a, 'tcx>( while let Some(location) = stack.pop() { // If region does not contain a point at the location, then add to list and skip // successor locations. - if !regioncx.region_contains_point(borrow_region, location) { + if !regioncx.region_contains(borrow_region, location) { debug!("borrow {:?} gets killed at {:?}", borrow_index, location); borrows_out_of_scope_at_location .entry(location) From 4f4350d9b0c5da472801a0aec8737b0ee10c5a83 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 4 Jul 2018 06:07:04 -0400 Subject: [PATCH 12/14] [WIP] deduplicate the successors of each SCC --- src/librustc_data_structures/graph/scc/mod.rs | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/librustc_data_structures/graph/scc/mod.rs b/src/librustc_data_structures/graph/scc/mod.rs index eeec8280fc878..b0f098b3d204f 100644 --- a/src/librustc_data_structures/graph/scc/mod.rs +++ b/src/librustc_data_structures/graph/scc/mod.rs @@ -13,6 +13,7 @@ //! node in the graph. This uses Tarjan's algorithm that completes in //! O(n) time. +use fx::FxHashSet; use graph::{DirectedGraph, WithNumNodes, WithSuccessors}; use indexed_vec::{Idx, IndexVec}; use std::ops::Range; @@ -113,6 +114,13 @@ struct SccsConstruction<'c, G: DirectedGraph + WithNumNodes + WithSuccessors + ' /// we push it on the stack. When we complete an SCC, we can pop /// everything off the stack that was found along the way. successors_stack: Vec, + + /// A set used to strip duplicates. As we accumulate successors + /// into the successors_stack, we sometimes get duplicate entries. + /// We use this set to remove those -- we keep it around between + /// successors to amortize memory allocation costs. + duplicate_set: FxHashSet, + scc_data: SccData, } @@ -180,6 +188,7 @@ where ranges: IndexVec::new(), all_successors: Vec::new(), }, + duplicate_set: FxHashSet::default(), }; let scc_indices = (0..num_nodes) @@ -307,8 +316,16 @@ where debug_assert_eq!(r, Some(node)); if min_depth == depth { - let scc_index = self.scc_data - .create_scc(self.successors_stack.drain(successors_len..)); + // Note that successor stack may have duplicates, so we + // want to remove those: + let deduplicated_successors = { + let duplicate_set = &mut self.duplicate_set; + duplicate_set.clear(); + self.successors_stack + .drain(successors_len..) + .filter(move |&i| duplicate_set.insert(i)) + }; + let scc_index = self.scc_data.create_scc(deduplicated_successors); self.node_states[node] = NodeState::InCycle { scc_index }; WalkReturn::Complete { scc_index } } else { From 3472813caf8ed0d5b6358e2ddf6bc29ea9e9c707 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 4 Jul 2018 06:07:17 -0400 Subject: [PATCH 13/14] impl graphviz trait for a newtype of regioncx --- src/librustc_mir/borrow_check/nll/mod.rs | 2 +- .../borrow_check/nll/region_infer/graphviz.rs | 17 ++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/mod.rs b/src/librustc_mir/borrow_check/nll/mod.rs index 26298a289fc61..641d7ed342c24 100644 --- a/src/librustc_mir/borrow_check/nll/mod.rs +++ b/src/librustc_mir/borrow_check/nll/mod.rs @@ -296,7 +296,7 @@ fn dump_mir_results<'a, 'gcx, 'tcx>( let _: io::Result<()> = do catch { let mut file = pretty::create_dump_file(infcx.tcx, "regioncx.dot", None, "nll", &0, source)?; - regioncx.dump_graphviz(&mut file)?; + regioncx.dump_graphviz_raw_constraints(&mut file)?; }; } diff --git a/src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs b/src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs index 8be51257cd6cb..2e41c8f8dd6a2 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs @@ -19,15 +19,18 @@ use std::io::{self, Write}; use super::*; use borrow_check::nll::constraints::OutlivesConstraint; - impl<'tcx> RegionInferenceContext<'tcx> { /// Write out the region constraint graph. - pub(crate) fn dump_graphviz(&self, mut w: &mut dyn Write) -> io::Result<()> { - dot::render(self, &mut w) + pub(crate) fn dump_graphviz_raw_constraints(&self, mut w: &mut dyn Write) -> io::Result<()> { + dot::render(&RawConstraints { regioncx: self }, &mut w) } } -impl<'this, 'tcx> dot::Labeller<'this> for RegionInferenceContext<'tcx> { +struct RawConstraints<'a, 'tcx: 'a> { + regioncx: &'a RegionInferenceContext<'tcx> +} + +impl<'a, 'this, 'tcx> dot::Labeller<'this> for RawConstraints<'a, 'tcx> { type Node = RegionVid; type Edge = OutlivesConstraint; @@ -48,16 +51,16 @@ impl<'this, 'tcx> dot::Labeller<'this> for RegionInferenceContext<'tcx> { } } -impl<'this, 'tcx> dot::GraphWalk<'this> for RegionInferenceContext<'tcx> { +impl<'a, 'this, 'tcx> dot::GraphWalk<'this> for RawConstraints<'a, 'tcx> { type Node = RegionVid; type Edge = OutlivesConstraint; fn nodes(&'this self) -> dot::Nodes<'this, RegionVid> { - let vids: Vec = self.definitions.indices().collect(); + let vids: Vec = self.regioncx.definitions.indices().collect(); vids.into_cow() } fn edges(&'this self) -> dot::Edges<'this, OutlivesConstraint> { - (&self.constraints.raw[..]).into_cow() + (&self.regioncx.constraints.raw[..]).into_cow() } // Render `a: b` as `a <- b`, indicating the flow From 05d4880b39f9efd46f8df62e89013c0c9cc88bcf Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 4 Jul 2018 06:29:06 -0400 Subject: [PATCH 14/14] dump scc graphviz too --- src/librustc_data_structures/graph/scc/mod.rs | 5 ++ src/librustc_mir/borrow_check/nll/mod.rs | 9 +- .../borrow_check/nll/region_infer/graphviz.rs | 89 +++++++++++++++++-- .../borrow_check/nll/region_infer/mod.rs | 32 +++++-- 4 files changed, 119 insertions(+), 16 deletions(-) diff --git a/src/librustc_data_structures/graph/scc/mod.rs b/src/librustc_data_structures/graph/scc/mod.rs index b0f098b3d204f..e7a84e4779754 100644 --- a/src/librustc_data_structures/graph/scc/mod.rs +++ b/src/librustc_data_structures/graph/scc/mod.rs @@ -54,6 +54,11 @@ impl Sccs { self.scc_data.len() } + /// Returns the number of SCCs in the graph. + pub fn all_sccs(&self) -> impl Iterator { + (0 .. self.scc_data.len()).map(S::new) + } + /// Returns the SCC to which a node `r` belongs. pub fn scc(&self, r: N) -> S { self.scc_indices[r] diff --git a/src/librustc_mir/borrow_check/nll/mod.rs b/src/librustc_mir/borrow_check/nll/mod.rs index 641d7ed342c24..acd9223e42545 100644 --- a/src/librustc_mir/borrow_check/nll/mod.rs +++ b/src/librustc_mir/borrow_check/nll/mod.rs @@ -295,9 +295,16 @@ fn dump_mir_results<'a, 'gcx, 'tcx>( // Also dump the inference graph constraints as a graphviz file. let _: io::Result<()> = do catch { let mut file = - pretty::create_dump_file(infcx.tcx, "regioncx.dot", None, "nll", &0, source)?; + pretty::create_dump_file(infcx.tcx, "regioncx.all.dot", None, "nll", &0, source)?; regioncx.dump_graphviz_raw_constraints(&mut file)?; }; + + // Also dump the inference graph constraints as a graphviz file. + let _: io::Result<()> = do catch { + let mut file = + pretty::create_dump_file(infcx.tcx, "regioncx.scc.dot", None, "nll", &0, source)?; + regioncx.dump_graphviz_scc_constraints(&mut file)?; + }; } fn dump_annotation<'a, 'gcx, 'tcx>( diff --git a/src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs b/src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs index 2e41c8f8dd6a2..8e18be15737f0 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs @@ -12,22 +12,37 @@ //! libgraphviz traits, specialized to attaching borrowck analysis //! data to rendered labels. +use super::*; +use borrow_check::nll::constraints::OutlivesConstraint; use dot::{self, IntoCow}; use rustc_data_structures::indexed_vec::Idx; use std::borrow::Cow; use std::io::{self, Write}; -use super::*; -use borrow_check::nll::constraints::OutlivesConstraint; impl<'tcx> RegionInferenceContext<'tcx> { /// Write out the region constraint graph. - pub(crate) fn dump_graphviz_raw_constraints(&self, mut w: &mut dyn Write) -> io::Result<()> { + crate fn dump_graphviz_raw_constraints(&self, mut w: &mut dyn Write) -> io::Result<()> { dot::render(&RawConstraints { regioncx: self }, &mut w) } + + /// Write out the region constraint graph. + crate fn dump_graphviz_scc_constraints(&self, mut w: &mut dyn Write) -> io::Result<()> { + let mut nodes_per_scc: IndexVec = self.constraints_scc + .all_sccs() + .map(|_| Vec::new()) + .collect(); + + for region in self.definitions.indices() { + let scc = self.constraints_scc.scc(region); + nodes_per_scc[scc].push(region); + } + + dot::render(&SccConstraints { regioncx: self, nodes_per_scc }, &mut w) + } } struct RawConstraints<'a, 'tcx: 'a> { - regioncx: &'a RegionInferenceContext<'tcx> + regioncx: &'a RegionInferenceContext<'tcx>, } impl<'a, 'this, 'tcx> dot::Labeller<'this> for RawConstraints<'a, 'tcx> { @@ -63,14 +78,74 @@ impl<'a, 'this, 'tcx> dot::GraphWalk<'this> for RawConstraints<'a, 'tcx> { (&self.regioncx.constraints.raw[..]).into_cow() } - // Render `a: b` as `a <- b`, indicating the flow + // Render `a: b` as `a -> b`, indicating the flow // of data during inference. fn source(&'this self, edge: &OutlivesConstraint) -> RegionVid { - edge.sub + edge.sup } fn target(&'this self, edge: &OutlivesConstraint) -> RegionVid { - edge.sup + edge.sub + } +} + +struct SccConstraints<'a, 'tcx: 'a> { + regioncx: &'a RegionInferenceContext<'tcx>, + nodes_per_scc: IndexVec>, +} + +impl<'a, 'this, 'tcx> dot::Labeller<'this> for SccConstraints<'a, 'tcx> { + type Node = ConstraintSccIndex; + type Edge = (ConstraintSccIndex, ConstraintSccIndex); + + fn graph_id(&'this self) -> dot::Id<'this> { + dot::Id::new(format!("RegionInferenceContext")).unwrap() + } + fn node_id(&'this self, n: &ConstraintSccIndex) -> dot::Id<'this> { + dot::Id::new(format!("r{}", n.index())).unwrap() + } + fn node_shape(&'this self, _node: &ConstraintSccIndex) -> Option> { + Some(dot::LabelText::LabelStr(Cow::Borrowed("box"))) + } + fn node_label(&'this self, n: &ConstraintSccIndex) -> dot::LabelText<'this> { + let nodes = &self.nodes_per_scc[*n]; + dot::LabelText::LabelStr(format!("{:?} = {:?}", n, nodes).into_cow()) + } +} + +impl<'a, 'this, 'tcx> dot::GraphWalk<'this> for SccConstraints<'a, 'tcx> { + type Node = ConstraintSccIndex; + type Edge = (ConstraintSccIndex, ConstraintSccIndex); + + fn nodes(&'this self) -> dot::Nodes<'this, ConstraintSccIndex> { + let vids: Vec = self.regioncx.constraints_scc.all_sccs().collect(); + vids.into_cow() + } + fn edges(&'this self) -> dot::Edges<'this, (ConstraintSccIndex, ConstraintSccIndex)> { + let edges: Vec<_> = self.regioncx + .constraints_scc + .all_sccs() + .flat_map(|scc_a| { + self.regioncx + .constraints_scc + .successors(scc_a) + .iter() + .map(move |&scc_b| (scc_a, scc_b)) + }) + .collect(); + + edges.into_cow() + } + + // Render `a: b` as `a -> b`, indicating the flow + // of data during inference. + + fn source(&'this self, edge: &(ConstraintSccIndex, ConstraintSccIndex)) -> ConstraintSccIndex { + edge.0 + } + + fn target(&'this self, edge: &(ConstraintSccIndex, ConstraintSccIndex)) -> ConstraintSccIndex { + edge.1 } } diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 16c5e91938b50..1f1e8ed728675 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -9,8 +9,9 @@ // except according to those terms. use super::universal_regions::UniversalRegions; -use borrow_check::nll::constraints::{ConstraintIndex, ConstraintSccIndex, ConstraintSet, - OutlivesConstraint}; +use borrow_check::nll::constraints::{ + ConstraintIndex, ConstraintSccIndex, ConstraintSet, OutlivesConstraint, +}; use borrow_check::nll::region_infer::values::ToElementIndex; use borrow_check::nll::type_check::Locations; use rustc::hir::def_id::DefId; @@ -26,7 +27,8 @@ use rustc::mir::{ use rustc::ty::{self, RegionVid, Ty, TyCtxt, TypeFoldable}; use rustc::util::common; use rustc_data_structures::graph::scc::Sccs; -use rustc_data_structures::indexed_vec::{Idx, IndexVec}; +use rustc_data_structures::indexed_set::{IdxSet, IdxSetBuf}; +use rustc_data_structures::indexed_vec::IndexVec; use std::rc::Rc; @@ -402,14 +404,28 @@ impl<'tcx> RegionInferenceContext<'tcx> { // SCC. For each SCC, we visit its successors and compute // their values, then we union all those values to get our // own. - for scc_index in (0..self.constraints_scc.num_sccs()).map(ConstraintSccIndex::new) { - self.propagate_constraints_scc(scc_index); + let visited = &mut IdxSetBuf::new_empty(self.constraints_scc.num_sccs()); + for scc_index in self.constraints_scc.all_sccs() { + self.propagate_constraints_scc_if_new(scc_index, visited); } } - fn propagate_constraints_scc(&mut self, scc_a: ConstraintSccIndex) { - debug!("propagate_constraints_scc(scc_a = {:?})", scc_a); + #[inline] + fn propagate_constraints_scc_if_new( + &mut self, + scc_a: ConstraintSccIndex, + visited: &mut IdxSet, + ) { + if visited.add(&scc_a) { + self.propagate_constraints_scc_new(scc_a, visited); + } + } + fn propagate_constraints_scc_new( + &mut self, + scc_a: ConstraintSccIndex, + visited: &mut IdxSet, + ) { let constraints_scc = self.constraints_scc.clone(); // Walk each SCC `B` such that `A: B`... @@ -420,7 +436,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { ); // ...compute the value of `B`... - self.propagate_constraints_scc(scc_b); + self.propagate_constraints_scc_if_new(scc_b, visited); // ...and add elements from `B` into `A`. self.scc_values.add_region(scc_a, scc_b);