Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutable: Modify values in place #52

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
@@ -1,4 +1,4 @@

target/
/target
**/*.rs.bk
Cargo.lock
35 changes: 35 additions & 0 deletions src/signal/mutable.rs
@@ -1,6 +1,7 @@
use super::Signal;
use std;
use std::fmt;
use std::panic;
use std::pin::Pin;
use std::marker::Unpin;
use std::ops::{Deref, DerefMut};
Expand Down Expand Up @@ -310,6 +311,40 @@ impl<A> Mutable<A> {
}
}

/// Conditionally modify the value in place
///
/// Borrows the value mutably and modifies it in place.
///
/// The value is only considered as modified if the modifying closure
/// returned `true`. Otherwise the value is assumed to be unmodified
/// despite the mutable borrow before.
///
/// Returns the result of the closure, i.e. if the value has been modified.
pub fn modify<F>(&self, modify: F) -> bool
where
F: FnOnce(&mut A) -> bool,
{
let mut locked_state = self.state().lock.write().unwrap();

let result =
panic::catch_unwind(panic::AssertUnwindSafe(|| modify(&mut locked_state.value)));
// If modify() panicked forward the panic to the caller without poisoning the lock.
match result {
Ok(modified) => {
if modified {
locked_state.notify(modified);
}
modified
}
Err(panicked) => {
// Drop the lock to avoid poisoning it.
drop(locked_state);
// Forward the panic to the caller.
panic::resume_unwind(panicked);
}
}
}

// TODO lots of unit tests to verify that it only notifies when the object is mutated
// TODO return Result ?
// TODO should this inline ?
Expand Down
51 changes: 51 additions & 0 deletions tests/signal.rs
Expand Up @@ -530,3 +530,54 @@ fn test_throttle_timing() {
assert_eq!(output.poll_change(cx), Poll::Ready(None));
});
}

#[test]
fn test_modify() {
#[derive(Debug, Clone, Copy, PartialEq)]
struct State {
counter: usize,
}

let inc_counter_if_odd = |state: &mut State| {
if state.counter % 2 == 1 {
state.counter += 1;
return true;
}
false
};

util::with_noop_context(|cx| {
let state = Mutable::new(State { counter: 1 });
let initial_state = state.get();
let modified_state = {
let mut modified_state = initial_state;
assert!(inc_counter_if_odd(&mut modified_state));
modified_state
};
assert_ne!(initial_state, modified_state);

let signal = state.signal();
pin_mut!(signal);

// Initial value
assert_eq!(
signal.as_mut().poll_change(cx),
Poll::Ready(Some(initial_state))
);

// Modified
assert_eq!(signal.as_mut().poll_change(cx), Poll::Pending);
assert!(state.modify(inc_counter_if_odd));
assert_eq!(state.get(), modified_state);
assert_eq!(
signal.as_mut().poll_change(cx),
Poll::Ready(Some(modified_state))
);

// Unmodified
assert_eq!(signal.as_mut().poll_change(cx), Poll::Pending);
assert!(!state.modify(inc_counter_if_odd));
assert_eq!(state.get(), modified_state);
assert_eq!(signal.as_mut().poll_change(cx), Poll::Pending);
});
}