diff --git a/Cargo.lock b/Cargo.lock index dc9296b81e223..a7b83f87b191b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -661,7 +661,7 @@ dependencies = [ [[package]] name = "ena" -version = "0.10.1" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2230,7 +2230,7 @@ name = "rustc_data_structures" version = "0.0.0" dependencies = [ "cfg-if 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)", - "ena 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)", + "ena 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", "graphviz 0.0.0", "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3295,7 +3295,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum difference 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "524cbf6897b527295dff137cec09ecf3a05f4fddffd7dfcd1585403449e74198" "checksum either 1.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "3be565ca5c557d7f59e7cfcf1844f9e3033650c929c6566f511e8005f205c1d0" "checksum elasticlunr-rs 2.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "4837d77a1e157489a3933b743fd774ae75074e0e390b2b7f071530048a0d87ee" -"checksum ena 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)" = "25b4e5febb25f08c49f1b07dc33a182729a6b21edfb562b5aef95f78e0dbe5bb" +"checksum ena 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f56c93cc076508c549d9bb747f79aa9b4eb098be7b8cad8830c3137ef52d1e00" "checksum ena 0.9.3 (registry+https://github.com/rust-lang/crates.io-index)" = "88dc8393b3c7352f94092497f6b52019643e493b6b890eb417cdb7c46117e621" "checksum env_logger 0.5.12 (registry+https://github.com/rust-lang/crates.io-index)" = "f4d7e69c283751083d53d01eac767407343b8b69c4bd70058e08adc2637cb257" "checksum env_logger 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "afb070faf94c85d17d50ca44f6ad076bce18ae92f0037d350947240a36e9d42e" diff --git a/src/librustc/infer/higher_ranked/mod.rs b/src/librustc/infer/higher_ranked/mod.rs index 8172f620c3646..4e203b986dfb0 100644 --- a/src/librustc/infer/higher_ranked/mod.rs +++ b/src/librustc/infer/higher_ranked/mod.rs @@ -543,11 +543,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { ) { 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/mod.rs b/src/librustc/infer/mod.rs index 87e32be1a1759..9e51a4c16e5cb 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -790,7 +790,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { self.projection_cache .borrow_mut() - .commit(&projection_cache_snapshot); + .commit(projection_cache_snapshot); self.type_variables.borrow_mut().commit(type_snapshot); self.int_unification_table.borrow_mut().commit(int_snapshot); self.float_unification_table diff --git a/src/librustc/infer/region_constraints/mod.rs b/src/librustc/infer/region_constraints/mod.rs index 391bfc428c3bb..af1b6964b8189 100644 --- a/src/librustc/infer/region_constraints/mod.rs +++ b/src/librustc/infer/region_constraints/mod.rs @@ -11,7 +11,7 @@ //! See README.md use self::CombineMapType::*; -use self::UndoLogEntry::*; +use self::UndoLog::*; use super::unify_key; use super::{MiscVariable, RegionVariableOrigin, SubregionOrigin}; @@ -52,14 +52,17 @@ 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. - undo_log: Vec>, + /// 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 @@ -254,15 +257,7 @@ struct TwoRegions<'tcx> { } #[derive(Copy, Clone, PartialEq)] -enum UndoLogEntry<'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, - +enum UndoLog<'tcx> { /// We added `RegionVid` AddVar(RegionVid), @@ -387,6 +382,7 @@ impl<'tcx> RegionConstraintCollector<'tcx> { glbs, bound_count: _, undo_log: _, + num_open_snapshots: _, unification_table, any_unifications, } = self; @@ -415,13 +411,13 @@ impl<'tcx> RegionConstraintCollector<'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(), @@ -429,39 +425,46 @@ impl<'tcx> RegionConstraintCollector<'tcx> { } } + fn assert_open_snapshot(&self, snapshot: &RegionSnapshot) { + 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); - assert!(self.undo_log.len() > snapshot.length); - assert!(self.undo_log[snapshot.length] == OpenSnapshot); + 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); - assert!(self.undo_log.len() > snapshot.length); - assert!(self.undo_log[snapshot.length] == OpenSnapshot); - while self.undo_log.len() > snapshot.length + 1 { + self.assert_open_snapshot(&snapshot); + + 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: UndoLogEntry<'tcx>) { + 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) => { @@ -521,15 +524,10 @@ impl<'tcx> RegionConstraintCollector<'tcx> { /// 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() @@ -548,7 +546,7 @@ impl<'tcx> RegionConstraintCollector<'tcx> { fn kill_constraint<'tcx>( placeholders: &FxHashSet>, - undo_entry: &UndoLogEntry<'tcx>, + undo_entry: &UndoLog<'tcx>, ) -> bool { match undo_entry { &AddConstraint(Constraint::VarSubVar(..)) => false, @@ -562,7 +560,7 @@ impl<'tcx> RegionConstraintCollector<'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 ef7365276f6d2..27ce7f106030a 100644 --- a/src/librustc/infer/region_constraints/taint.rs +++ b/src/librustc/infer/region_constraints/taint.rs @@ -29,7 +29,7 @@ impl<'tcx> TaintSet<'tcx> { pub(super) fn fixed_point( &mut self, tcx: TyCtxt<'_, '_, 'tcx>, - undo_log: &[UndoLogEntry<'tcx>], + undo_log: &[UndoLog<'tcx>], verifys: &[Verify<'tcx>], ) { let mut prev_len = 0; @@ -65,8 +65,7 @@ impl<'tcx> TaintSet<'tcx> { "we never add verifications while doing higher-ranked things", ) } - &Purged | &AddCombination(..) | &AddVar(..) | &OpenSnapshot - | &CommitedSnapshot => {} + &Purged | &AddCombination(..) | &AddVar(..) => {} } } } diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs index b59bd0e238873..8f165863b3bab 100644 --- a/src/librustc/traits/project.rs +++ b/src/librustc/traits/project.rs @@ -1652,15 +1652,15 @@ impl<'tcx> ProjectionCache<'tcx> { } pub fn rollback_to(&mut self, snapshot: ProjectionCacheSnapshot) { - self.map.rollback_to(&snapshot.snapshot); + self.map.rollback_to(snapshot.snapshot); } pub fn rollback_placeholder(&mut self, snapshot: &ProjectionCacheSnapshot) { self.map.partial_rollback(&snapshot.snapshot, &|k| k.ty.has_re_skol()); } - pub fn commit(&mut self, snapshot: &ProjectionCacheSnapshot) { - self.map.commit(&snapshot.snapshot); + pub fn commit(&mut self, snapshot: ProjectionCacheSnapshot) { + self.map.commit(snapshot.snapshot); } /// Try to start normalize `key`; returns an error if @@ -1714,12 +1714,8 @@ impl<'tcx> ProjectionCache<'tcx> { /// to be a NormalizedTy. pub fn complete_normalized(&mut self, key: ProjectionCacheKey<'tcx>, ty: &NormalizedTy<'tcx>) { // We want to insert `ty` with no obligations. If the existing value - // already has no obligations (as is common) we can use `insert_noop` - // to do a minimal amount of work -- the HashMap insertion is skipped, - // and minimal changes are made to the undo log. - if ty.obligations.is_empty() { - self.map.insert_noop(); - } else { + // already has no obligations (as is common) we don't insert anything. + if !ty.obligations.is_empty() { self.map.insert(key, ProjectionCacheEntry::NormalizedTy(Normalized { value: ty.value, obligations: vec![] diff --git a/src/librustc_data_structures/Cargo.toml b/src/librustc_data_structures/Cargo.toml index 5b3dd38adf23f..188919d063351 100644 --- a/src/librustc_data_structures/Cargo.toml +++ b/src/librustc_data_structures/Cargo.toml @@ -9,7 +9,7 @@ path = "lib.rs" crate-type = ["dylib"] [dependencies] -ena = "0.10.1" +ena = "0.11" log = "0.4" rustc_cratesio_shim = { path = "../librustc_cratesio_shim" } serialize = { path = "../libserialize" } diff --git a/src/librustc_data_structures/snapshot_map/mod.rs b/src/librustc_data_structures/snapshot_map/mod.rs index 0b42cb1edddec..c256506a19d42 100644 --- a/src/librustc_data_structures/snapshot_map/mod.rs +++ b/src/librustc_data_structures/snapshot_map/mod.rs @@ -21,6 +21,7 @@ pub struct SnapshotMap { map: FxHashMap, undo_log: Vec>, + num_open_snapshots: usize, } // HACK(eddyb) manual impl avoids `Default` bounds on `K` and `V`. @@ -31,6 +32,7 @@ impl Default for SnapshotMap SnapshotMap { map: Default::default(), undo_log: Default::default(), + num_open_snapshots: 0, } } } @@ -40,11 +42,9 @@ pub struct Snapshot { } enum UndoLog { - OpenSnapshot, - CommittedSnapshot, Inserted(K), Overwrite(K, V), - Noop, + Purged, } impl SnapshotMap @@ -53,18 +53,23 @@ impl SnapshotMap pub fn clear(&mut self) { self.map.clear(); self.undo_log.clear(); + self.num_open_snapshots = 0; + } + + fn in_snapshot(&self) -> bool { + self.num_open_snapshots > 0 } pub fn insert(&mut self, key: K, value: V) -> bool { match self.map.insert(key.clone(), value) { None => { - if !self.undo_log.is_empty() { + if self.in_snapshot() { self.undo_log.push(UndoLog::Inserted(key)); } true } Some(old_value) => { - if !self.undo_log.is_empty() { + if self.in_snapshot() { self.undo_log.push(UndoLog::Overwrite(key, old_value)); } false @@ -72,16 +77,10 @@ impl SnapshotMap } } - pub fn insert_noop(&mut self) { - if !self.undo_log.is_empty() { - self.undo_log.push(UndoLog::Noop); - } - } - pub fn remove(&mut self, key: K) -> bool { match self.map.remove(&key) { Some(old_value) => { - if !self.undo_log.is_empty() { + if self.in_snapshot() { self.undo_log.push(UndoLog::Overwrite(key, old_value)); } true @@ -95,27 +94,27 @@ impl SnapshotMap } pub fn snapshot(&mut self) -> Snapshot { - self.undo_log.push(UndoLog::OpenSnapshot); - let len = self.undo_log.len() - 1; + let len = self.undo_log.len(); + self.num_open_snapshots += 1; Snapshot { len } } fn assert_open_snapshot(&self, snapshot: &Snapshot) { - assert!(snapshot.len < self.undo_log.len()); - assert!(match self.undo_log[snapshot.len] { - UndoLog::OpenSnapshot => true, - _ => false, - }); + assert!(self.undo_log.len() >= snapshot.len); + assert!(self.num_open_snapshots > 0); } - pub fn commit(&mut self, snapshot: &Snapshot) { - self.assert_open_snapshot(snapshot); - if snapshot.len == 0 { - // The root snapshot. - self.undo_log.truncate(0); - } else { - self.undo_log[snapshot.len] = UndoLog::CommittedSnapshot; + pub fn commit(&mut self, snapshot: Snapshot) { + self.assert_open_snapshot(&snapshot); + 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.len == 0); + self.undo_log.clear(); } + + self.num_open_snapshots -= 1; } pub fn partial_rollback(&mut self, @@ -124,45 +123,32 @@ impl SnapshotMap where F: Fn(&K) -> bool { self.assert_open_snapshot(snapshot); - for i in (snapshot.len + 1..self.undo_log.len()).rev() { + for i in (snapshot.len .. self.undo_log.len()).rev() { let reverse = match self.undo_log[i] { - UndoLog::OpenSnapshot => false, - UndoLog::CommittedSnapshot => false, - UndoLog::Noop => false, + UndoLog::Purged => false, UndoLog::Inserted(ref k) => should_revert_key(k), UndoLog::Overwrite(ref k, _) => should_revert_key(k), }; if reverse { - let entry = mem::replace(&mut self.undo_log[i], UndoLog::Noop); + let entry = mem::replace(&mut self.undo_log[i], UndoLog::Purged); self.reverse(entry); } } } - pub fn rollback_to(&mut self, snapshot: &Snapshot) { - self.assert_open_snapshot(snapshot); - while self.undo_log.len() > snapshot.len + 1 { + pub fn rollback_to(&mut self, snapshot: Snapshot) { + self.assert_open_snapshot(&snapshot); + while self.undo_log.len() > snapshot.len { let entry = self.undo_log.pop().unwrap(); self.reverse(entry); } - let v = self.undo_log.pop().unwrap(); - assert!(match v { - UndoLog::OpenSnapshot => true, - _ => false, - }); - assert!(self.undo_log.len() == snapshot.len); + self.num_open_snapshots -= 1; } fn reverse(&mut self, entry: UndoLog) { match entry { - UndoLog::OpenSnapshot => { - panic!("cannot rollback an uncommitted snapshot"); - } - - UndoLog::CommittedSnapshot => {} - UndoLog::Inserted(key) => { self.map.remove(&key); } @@ -171,7 +157,7 @@ impl SnapshotMap self.map.insert(key, old_value); } - UndoLog::Noop => {} + UndoLog::Purged => {} } } } diff --git a/src/librustc_data_structures/snapshot_map/test.rs b/src/librustc_data_structures/snapshot_map/test.rs index 700f9c95e3b57..b4ecb85fc4302 100644 --- a/src/librustc_data_structures/snapshot_map/test.rs +++ b/src/librustc_data_structures/snapshot_map/test.rs @@ -17,10 +17,10 @@ fn basic() { let snapshot = map.snapshot(); map.insert(22, "thirty-three"); assert_eq!(map[&22], "thirty-three"); - map.insert(44, "fourty-four"); - assert_eq!(map[&44], "fourty-four"); + map.insert(44, "forty-four"); + assert_eq!(map[&44], "forty-four"); assert_eq!(map.get(&33), None); - map.rollback_to(&snapshot); + map.rollback_to(snapshot); assert_eq!(map[&22], "twenty-two"); assert_eq!(map.get(&33), None); assert_eq!(map.get(&44), None); @@ -32,8 +32,11 @@ fn out_of_order() { let mut map = SnapshotMap::default(); map.insert(22, "twenty-two"); let snapshot1 = map.snapshot(); - let _snapshot2 = map.snapshot(); - map.rollback_to(&snapshot1); + map.insert(33, "thirty-three"); + let snapshot2 = map.snapshot(); + map.insert(44, "forty-four"); + map.rollback_to(snapshot1); // bogus, but accepted + map.rollback_to(snapshot2); // asserts } #[test] @@ -43,8 +46,8 @@ fn nested_commit_then_rollback() { let snapshot1 = map.snapshot(); let snapshot2 = map.snapshot(); map.insert(22, "thirty-three"); - map.commit(&snapshot2); + map.commit(snapshot2); assert_eq!(map[&22], "thirty-three"); - map.rollback_to(&snapshot1); + map.rollback_to(snapshot1); assert_eq!(map[&22], "twenty-two"); }