From 5388037f8a16d3bf443f348e28873d255db044d0 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 23 Oct 2018 15:59:50 +0200
Subject: [PATCH] remove code duplication by letting reactivatable() compute
what reactivate() has to do
---
src/stacked_borrows.rs | 89 ++++++++-----------
.../stacked_borrows/alias_through_mutation.rs | 2 +-
.../stacked_borrows/buggy_as_mut_slice.rs | 2 +-
.../stacked_borrows/buggy_split_at_mut.rs | 2 +-
.../stacked_borrows/illegal_write1.rs | 2 +-
.../stacked_borrows/load_invalid_mut.rs | 2 +-
.../stacked_borrows/pass_invalid_mut.rs | 2 +-
.../stacked_borrows/return_invalid_mut.rs | 2 +-
.../stacked_borrows/shared_confusion.rs | 2 +-
9 files changed, 47 insertions(+), 58 deletions(-)
diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs
index 31631635186..f634f17109c 100644
--- a/src/stacked_borrows.rs
+++ b/src/stacked_borrows.rs
@@ -137,75 +137,64 @@ impl<'tcx> Stack {
}
/// Check if `bor` could be activated by unfreezing and popping.
- /// This should be in sync with `reactivate`!
- fn reactivatable(&self, bor: Borrow) -> bool {
- if self.check(bor) {
- return true;
- }
-
- let acc_m = match bor {
- Borrow::Frz(_) => return false,
- Borrow::Mut(acc_m) => acc_m
- };
- // This is where we would unfreeze.
- for &itm in self.borrows.iter().rev() {
- match itm {
- BorStackItem::FnBarrier(_) => return false,
- BorStackItem::Mut(loc_m) => {
- if loc_m == acc_m { return true; }
- // Go on looking.
- }
- }
- }
- // Nothing to be found.
- false
- }
-
- /// Reactive `bor` for this stack. If `force_mut` is set, we want to aggressively
- /// unfreeze this location (because we are about to mutate, so a frozen `Raw` is not okay).
- fn reactivate(&mut self, bor: Borrow, force_mut: bool) -> EvalResult<'tcx> {
+ /// `force_mut` indicates whether being frozen is potentially acceptable.
+ /// Returns `Err` if the answer is "no"; otherwise the data says
+ /// what needs to happen to activate this: `None` = nothing,
+ /// `Some(n)` = unfreeze and make item `n` the top item of the stack.
+ fn reactivatable(&self, bor: Borrow, force_mut: bool) -> Result<Option<usize>, String> {
// Unless mutation is bound to happen, do NOT change anything if `bor` is already active.
// In particular, if it is a `Mut(Raw)` and we are frozen, this should be a NOP.
if !force_mut && self.check(bor) {
- return Ok(());
+ return Ok(None);
}
let acc_m = match bor {
Borrow::Frz(since) =>
- if force_mut {
- return err!(MachineError(format!("Using a shared borrow for mutation")))
+ return Err(if force_mut {
+ format!("Using a shared borrow for mutation")
} else {
- return err!(MachineError(format!(
+ format!(
"Location should be frozen since {} but {}",
since,
match self.frozen_since {
None => format!("it is not frozen at all"),
Some(since) => format!("it is only frozen since {}", since),
}
- )))
- }
- Borrow::Mut(acc_m) => acc_m,
+ )
+ }),
+ Borrow::Mut(acc_m) => acc_m
};
- // We definitely have to unfreeze this, even if we use the topmost item.
- if self.frozen_since.is_some() {
- trace!("reactivate: Unfreezing");
- }
- self.frozen_since = None;
- // Pop until we see the one we are looking for.
- while let Some(&itm) = self.borrows.last() {
+ // This is where we would unfreeze.
+ for (idx, &itm) in self.borrows.iter().enumerate().rev() {
match itm {
- BorStackItem::FnBarrier(_) => {
- return err!(MachineError(format!("Trying to reactivate a borrow that lives behind a barrier")));
- }
+ BorStackItem::FnBarrier(_) =>
+ return Err(format!("Trying to reactivate a mutable borrow ({:?}) that lives behind a barrier", acc_m)),
BorStackItem::Mut(loc_m) => {
- if loc_m == acc_m { return Ok(()); }
- trace!("reactivate: Popping {:?}", itm);
- self.borrows.pop();
+ if loc_m == acc_m { return Ok(Some(idx)); }
}
}
}
// Nothing to be found.
- err!(MachineError(format!("Borrow-to-reactivate does not exist on the stack")))
+ Err(format!("Mutable borrow-to-reactivate ({:?}) does not exist on the stack", acc_m))
+ }
+
+ /// Reactive `bor` for this stack. If `force_mut` is set, we want to aggressively
+ /// unfreeze this location (because we are about to mutate, so a frozen `Raw` is not okay).
+ fn reactivate(&mut self, bor: Borrow, force_mut: bool) -> EvalResult<'tcx> {
+ let action = match self.reactivatable(bor, force_mut) {
+ Ok(action) => action,
+ Err(err) => return err!(MachineError(err)),
+ };
+
+ match action {
+ None => {}, // nothing to do
+ Some(top) => {
+ self.frozen_since = None;
+ self.borrows.truncate(top+1);
+ }
+ }
+
+ Ok(())
}
/// Initiate `bor`; mostly this means freezing or pushing.
@@ -471,8 +460,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
// be shared reborrows that we are about to invalidate with this access.
// We cannot invalidate them aggressively here because the deref might also be
// to just create more shared refs.
- if !stack.reactivatable(ptr.tag) {
- return err!(MachineError(format!("Encountered {:?} reference with non-reactivatable tag {:?}", ref_kind, ptr.tag)))
+ if let Err(err) = stack.reactivatable(ptr.tag, /*force_mut*/false) {
+ return err!(MachineError(format!("Encountered {:?} reference with non-reactivatable tag: {}", ref_kind, err)))
}
}
// All is good.
diff --git a/tests/compile-fail/stacked_borrows/alias_through_mutation.rs b/tests/compile-fail/stacked_borrows/alias_through_mutation.rs
index 83132195fe4..7d56f30b3e6 100644
--- a/tests/compile-fail/stacked_borrows/alias_through_mutation.rs
+++ b/tests/compile-fail/stacked_borrows/alias_through_mutation.rs
@@ -14,5 +14,5 @@ fn main() {
retarget(&mut target_alias, target);
// now `target_alias` points to the same thing as `target`
*target = 13;
- let _val = *target_alias; //~ ERROR Shr reference with non-reactivatable tag Frz
+ let _val = *target_alias; //~ ERROR Shr reference with non-reactivatable tag
}
diff --git a/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs b/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs
index 9e94aa8885d..dc1e3cc81e9 100644
--- a/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs
+++ b/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs
@@ -17,6 +17,6 @@ fn main() {
let v = vec![0,1,2];
let v1 = safe::as_mut_slice(&v);
let v2 = safe::as_mut_slice(&v);
- v1[1] = 5; //~ ERROR Mut reference with non-reactivatable tag Mut(Uniq
+ v1[1] = 5; //~ ERROR Mut reference with non-reactivatable tag
v1[1] = 6;
}
diff --git a/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs b/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs
index 9fbcec4a8ef..a697ba9167c 100644
--- a/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs
+++ b/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs
@@ -14,7 +14,7 @@ mod safe {
assert!(mid <= len);
(from_raw_parts_mut(ptr, len - mid), // BUG: should be "mid" instead of "len - mid"
- //~^ ERROR Mut reference with non-reactivatable tag Mut(Uniq
+ //~^ ERROR Mut reference with non-reactivatable tag
from_raw_parts_mut(ptr.offset(mid as isize), len - mid))
}
}
diff --git a/tests/compile-fail/stacked_borrows/illegal_write1.rs b/tests/compile-fail/stacked_borrows/illegal_write1.rs
index ff3a3cd8224..131e89572a5 100644
--- a/tests/compile-fail/stacked_borrows/illegal_write1.rs
+++ b/tests/compile-fail/stacked_borrows/illegal_write1.rs
@@ -8,5 +8,5 @@ fn main() {
let target = Box::new(42); // has an implicit raw
let ref_ = &*target;
evil(ref_); // invalidates shared ref, activates raw
- let _x = *ref_; //~ ERROR Shr reference with non-reactivatable tag Frz
+ let _x = *ref_; //~ ERROR Shr reference with non-reactivatable tag
}
diff --git a/tests/compile-fail/stacked_borrows/load_invalid_mut.rs b/tests/compile-fail/stacked_borrows/load_invalid_mut.rs
index e52b84a907e..060cec962c4 100644
--- a/tests/compile-fail/stacked_borrows/load_invalid_mut.rs
+++ b/tests/compile-fail/stacked_borrows/load_invalid_mut.rs
@@ -5,5 +5,5 @@ fn main() {
let xref = unsafe { &mut *xraw };
let xref_in_mem = Box::new(xref);
let _val = *x; // invalidate xraw
- let _val = *xref_in_mem; //~ ERROR Mut reference with non-reactivatable tag Mut(Uniq
+ let _val = *xref_in_mem; //~ ERROR Mut reference with non-reactivatable tag
}
diff --git a/tests/compile-fail/stacked_borrows/pass_invalid_mut.rs b/tests/compile-fail/stacked_borrows/pass_invalid_mut.rs
index f3de3f0c4ac..bc950771add 100644
--- a/tests/compile-fail/stacked_borrows/pass_invalid_mut.rs
+++ b/tests/compile-fail/stacked_borrows/pass_invalid_mut.rs
@@ -6,5 +6,5 @@ fn main() {
let xraw = x as *mut _;
let xref = unsafe { &mut *xraw };
let _val = *x; // invalidate xraw
- foo(xref); //~ ERROR Mut reference with non-reactivatable tag Mut(Uniq
+ foo(xref); //~ ERROR Mut reference with non-reactivatable tag
}
diff --git a/tests/compile-fail/stacked_borrows/return_invalid_mut.rs b/tests/compile-fail/stacked_borrows/return_invalid_mut.rs
index 6a4123d325b..c02892671e9 100644
--- a/tests/compile-fail/stacked_borrows/return_invalid_mut.rs
+++ b/tests/compile-fail/stacked_borrows/return_invalid_mut.rs
@@ -3,7 +3,7 @@ fn foo(x: &mut (i32, i32)) -> &mut i32 {
let xraw = x as *mut (i32, i32);
let ret = unsafe { &mut (*xraw).1 };
let _val = *x; // invalidate xraw and its children
- ret //~ ERROR Mut reference with non-reactivatable tag Mut(Uniq
+ ret //~ ERROR Mut reference with non-reactivatable tag
}
fn main() {
diff --git a/tests/compile-fail/stacked_borrows/shared_confusion.rs b/tests/compile-fail/stacked_borrows/shared_confusion.rs
index 5098f493ccd..e3e4c4da776 100644
--- a/tests/compile-fail/stacked_borrows/shared_confusion.rs
+++ b/tests/compile-fail/stacked_borrows/shared_confusion.rs
@@ -13,7 +13,7 @@ fn test(r: &mut RefCell<i32>) {
}
// Our old raw should be dead by now
unsafe { *x_evil = 0; } // this falls back to some Raw higher up the stack
- *x_inner = 12; //~ ERROR Mut reference with non-reactivatable tag Mut(Uniq
+ *x_inner = 12; //~ ERROR Mut reference with non-reactivatable tag
}
fn main() {