Reputation: 7746
I am trying to implement and apply an optimized replace_with
helper for Option
. I have implemented it like this:
trait OptionExt<T> {
#[inline]
fn replace_with<F>(&mut self, f: F)
where
F: FnOnce(Option<T>) -> Option<T>;
}
impl<T> OptionExt<T> for Option<T> {
#[inline]
fn replace_with<F>(&mut self, f: F)
where
F: FnOnce(Option<T>) -> Option<T>,
{
let mut x = f(self.take());
mem::swap(self, &mut x);
debug_assert!(x.is_none());
mem::forget(x);
}
}
It works fine for a simple use-case.
Naive implementation:
let merged = merge(lower_node.right.take(), Some(greater_node));
lower_node.right = merged;
Optimized implementation (details about the trick):
let mut merged = merge(lower_node.right.take(), Some(greater_node));
mem::swap(&mut lower_node.right, &mut merged);
debug_assert!(merged.is_none());
mem::forget(merged);
Optimized implementation with a nicer interface:
lower_node.right.replace_with(|node| merge(node, Some(greater_node));
This doesn't play nicely when I want to implement a split_binary
function that would return a pair of nodes:
Naive implementation:
let (left_node, right_node) = split_binary(orig_node.right.take(), value);
orig_node.right = left_node;
(Some(orig_node), right_node)
Optimized implementation:
let (mut left_node, right_node) = split_binary(orig_node.right.take(), value);
mem::swap(&mut orig_node.right, &mut left_node);
debug_assert!(left_node.is_none());
mem::forget(left_node);
(Some(orig_node), right_node)
Optimized implementation with a nicer interface (does not compile since right_node
is now inside closure):
orig_node.right.replace_with(|node| {
let (left_node, right_node) = split_binary(node, value);
left_node
});
(Some(orig_node), right_node)
I can make it compile by defining a helper new_right_node
outside of the closure:
let mut new_right_node = None;
orig_node.right.replace_with(|node| {
let (left_node, right_node) = split_binary(node, value);
new_right_node = right_node;
left_node
});
(Some(orig_node), new_right_node)
I have to initialize new_right_node
with None
as otherwise Rust has an error "use of possibly uninitialized new_right_node
". Yet doing this initialization results in an unnecessary core::ptr::drop_in_place
call for the initial None
value of new_right_node
. Manually inlining the replace_with
, I can get away without a need to initialize new_right_node
upfront:
let new_right_node;
{
let (mut left_node, right_node) = split_binary(orig_node.right.take(), value);
new_right_node = right_node;
mem::swap(&mut orig_node.right, &mut left_node);
debug_assert!(left_node.is_none());
mem::forget(left_node);
}
(Some(orig_node), new_right_node)
Thus, it seems that I can solve this efficiently if I can convince Rust that the closure will be executed and thus new_right_node
will be initialized.
How can I solve this efficiently?
Upvotes: 1
Views: 145
Reputation: 27895
You can use unsafe
since you know the value will always be initialized. Two unsafe
functions come into play: mem::uninitialized
, which disables the "possibly uninitialized" error, and ptr::write
, which lets you write to new_right_node
without dropping the previous (uninitialized) value.
unsafe {
let mut new_right_node = std::mem::uninitialized();
orig_node.right.replace_with(|node| {
let (left_node, right_node) = split_binary(node, value);
std::ptr::write(&mut new_right_node, right_node);
left_node
});
}
I believe this is safe in the absence of panics because new_right_node
is always initialized and right_node
is moved into it without either one being dropped. However, to make it panic safe, you must also guarantee there's no way to observe new_right_node
in its uninitialized state if split_binary
panics.
Upvotes: 2