Skip to content

Commit

Permalink
Remove OpenSnapshot and CommittedSnapshot markers from `RegionCon…
Browse files Browse the repository at this point in the history
…straintCollector`.

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()`.
  • Loading branch information
nnethercote committed Nov 25, 2018
1 parent 2d68fa0 commit 94967ae
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 42 deletions.
6 changes: 1 addition & 5 deletions src/librustc/infer/higher_ranked/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
65 changes: 30 additions & 35 deletions src/librustc/infer/region_constraints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<UndoLog<'tcx>>,

/// 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
Expand Down Expand Up @@ -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),

Expand Down Expand Up @@ -387,6 +382,7 @@ impl<'tcx> RegionConstraintCollector<'tcx> {
glbs,
bound_count: _,
undo_log: _,
num_open_snapshots: _,
unification_table,
any_unifications,
} = self;
Expand Down Expand Up @@ -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(),
Expand All @@ -430,41 +426,45 @@ impl<'tcx> RegionConstraintCollector<'tcx> {
}

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) => {
Expand Down Expand Up @@ -524,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<ty::Region<'tcx>>,
snapshot: &RegionSnapshot,
) {
pub fn pop_placeholders(&mut self, placeholders: &FxHashSet<ty::Region<'tcx>>) {
debug!("pop_placeholders(placeholders={:?})", placeholders);

assert!(self.in_snapshot());
assert!(self.undo_log[snapshot.length] == OpenSnapshot);

let constraints_to_kill: Vec<usize> = self.undo_log
.iter()
Expand Down Expand Up @@ -565,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,
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/infer/region_constraints/taint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(..) => {}
}
}
}
Expand Down

0 comments on commit 94967ae

Please sign in to comment.