From: Nicholas Nethercote Date: Mon, 5 Nov 2018 01:21:54 +0000 (+1100) Subject: Remove `OpenSnapshot` and `CommittedSnapshot` markers from `RegionConstraintCollector`. X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=94967ae8c1129d63df4446240df4fc45a57c8164;p=rust.git Remove `OpenSnapshot` and `CommittedSnapshot` markers from `RegionConstraintCollector`. They're not strictly necessary, and they result in the `Vec` being allocated even for the trivial (and common) case where a `start_snapshot` is immediately followed by a `commit` or `rollback_to`. The commit also removes a now-unnecessary argument of `pop_placeholders()`. --- diff --git a/src/librustc/infer/higher_ranked/mod.rs b/src/librustc/infer/higher_ranked/mod.rs index 8172f620c36..4e203b986df 100644 --- a/src/librustc/infer/higher_ranked/mod.rs +++ b/src/librustc/infer/higher_ranked/mod.rs @@ -543,11 +543,7 @@ pub fn pop_placeholders( ) { debug!("pop_placeholders({:?})", placeholder_map); let placeholder_regions: FxHashSet<_> = placeholder_map.values().cloned().collect(); - self.borrow_region_constraints() - .pop_placeholders( - &placeholder_regions, - &snapshot.region_constraints_snapshot, - ); + self.borrow_region_constraints().pop_placeholders(&placeholder_regions); self.universe.set(snapshot.universe); if !placeholder_map.is_empty() { self.projection_cache.borrow_mut().rollback_placeholder( diff --git a/src/librustc/infer/region_constraints/mod.rs b/src/librustc/infer/region_constraints/mod.rs index 231eff9ff9f..af1b6964b81 100644 --- a/src/librustc/infer/region_constraints/mod.rs +++ b/src/librustc/infer/region_constraints/mod.rs @@ -52,15 +52,18 @@ pub struct RegionConstraintCollector<'tcx> { /// The undo log records actions that might later be undone. /// - /// Note: when the undo_log is empty, we are not actively + /// Note: `num_open_snapshots` is used to track if we are actively /// snapshotting. When the `start_snapshot()` method is called, we - /// push an OpenSnapshot entry onto the list to indicate that we - /// are now actively snapshotting. The reason for this is that - /// otherwise we end up adding entries for things like the lower - /// bound on a variable and so forth, which can never be rolled - /// back. + /// increment `num_open_snapshots` to indicate that we are now actively + /// snapshotting. The reason for this is that otherwise we end up adding + /// entries for things like the lower bound on a variable and so forth, + /// which can never be rolled back. undo_log: Vec>, + /// The number of open snapshots, i.e. those that haven't been committed or + /// rolled back. + num_open_snapshots: usize, + /// When we add a R1 == R2 constriant, we currently add (a) edges /// R1 <= R2 and R2 <= R1 and (b) we unify the two regions in this /// table. You can then call `opportunistic_resolve_var` early @@ -255,14 +258,6 @@ struct TwoRegions<'tcx> { #[derive(Copy, Clone, PartialEq)] enum UndoLog<'tcx> { - /// Pushed when we start a snapshot. - OpenSnapshot, - - /// Replaces an `OpenSnapshot` when a snapshot is committed, but - /// that snapshot is not the root. If the root snapshot is - /// unrolled, all nested snapshots must be committed. - CommitedSnapshot, - /// We added `RegionVid` AddVar(RegionVid), @@ -387,6 +382,7 @@ pub fn take_and_reset_data(&mut self) -> RegionConstraintData<'tcx> { glbs, bound_count: _, undo_log: _, + num_open_snapshots: _, unification_table, any_unifications, } = self; @@ -415,13 +411,13 @@ pub fn data(&self) -> &RegionConstraintData<'tcx> { } fn in_snapshot(&self) -> bool { - !self.undo_log.is_empty() + self.num_open_snapshots > 0 } pub fn start_snapshot(&mut self) -> RegionSnapshot { let length = self.undo_log.len(); debug!("RegionConstraintCollector: start_snapshot({})", length); - self.undo_log.push(OpenSnapshot); + self.num_open_snapshots += 1; RegionSnapshot { length, region_snapshot: self.unification_table.snapshot(), @@ -430,41 +426,45 @@ pub fn start_snapshot(&mut self) -> RegionSnapshot { } fn assert_open_snapshot(&self, snapshot: &RegionSnapshot) { - assert!(self.undo_log.len() > snapshot.length); - assert!(self.undo_log[snapshot.length] == OpenSnapshot); + assert!(self.undo_log.len() >= snapshot.length); + assert!(self.num_open_snapshots > 0); } pub fn commit(&mut self, snapshot: RegionSnapshot) { debug!("RegionConstraintCollector: commit({})", snapshot.length); self.assert_open_snapshot(&snapshot); - if snapshot.length == 0 { + if self.num_open_snapshots == 1 { + // The root snapshot. It's safe to clear the undo log because + // there's no snapshot further out that we might need to roll back + // to. + assert!(snapshot.length == 0); self.undo_log.clear(); - } else { - (*self.undo_log)[snapshot.length] = CommitedSnapshot; } + + self.num_open_snapshots -= 1; + self.unification_table.commit(snapshot.region_snapshot); } pub fn rollback_to(&mut self, snapshot: RegionSnapshot) { debug!("RegionConstraintCollector: rollback_to({:?})", snapshot); self.assert_open_snapshot(&snapshot); - while self.undo_log.len() > snapshot.length + 1 { + + while self.undo_log.len() > snapshot.length { let undo_entry = self.undo_log.pop().unwrap(); self.rollback_undo_entry(undo_entry); } - let c = self.undo_log.pop().unwrap(); - assert!(c == OpenSnapshot); + + self.num_open_snapshots -= 1; + self.unification_table.rollback_to(snapshot.region_snapshot); self.any_unifications = snapshot.any_unifications; } fn rollback_undo_entry(&mut self, undo_entry: UndoLog<'tcx>) { match undo_entry { - OpenSnapshot => { - panic!("Failure to observe stack discipline"); - } - Purged | CommitedSnapshot => { + Purged => { // nothing to do here } AddVar(vid) => { @@ -524,15 +524,10 @@ pub fn var_origin(&self, vid: RegionVid) -> RegionVariableOrigin { /// in `skols`. This is used after a higher-ranked operation /// completes to remove all trace of the placeholder regions /// created in that time. - pub fn pop_placeholders( - &mut self, - placeholders: &FxHashSet>, - snapshot: &RegionSnapshot, - ) { + pub fn pop_placeholders(&mut self, placeholders: &FxHashSet>) { debug!("pop_placeholders(placeholders={:?})", placeholders); assert!(self.in_snapshot()); - assert!(self.undo_log[snapshot.length] == OpenSnapshot); let constraints_to_kill: Vec = self.undo_log .iter() @@ -565,7 +560,7 @@ fn kill_constraint<'tcx>( &AddCombination(_, ref two_regions) => { placeholders.contains(&two_regions.a) || placeholders.contains(&two_regions.b) } - &AddVar(..) | &OpenSnapshot | &Purged | &CommitedSnapshot => false, + &AddVar(..) | &Purged => false, } } } diff --git a/src/librustc/infer/region_constraints/taint.rs b/src/librustc/infer/region_constraints/taint.rs index 9f08fdcad7e..27ce7f10603 100644 --- a/src/librustc/infer/region_constraints/taint.rs +++ b/src/librustc/infer/region_constraints/taint.rs @@ -65,8 +65,7 @@ pub(super) fn fixed_point( "we never add verifications while doing higher-ranked things", ) } - &Purged | &AddCombination(..) | &AddVar(..) | &OpenSnapshot - | &CommitedSnapshot => {} + &Purged | &AddCombination(..) | &AddVar(..) => {} } } }