Vlad Frolov
Vlad Frolov

Reputation: 7746

How to hint that a FnOnce closure will be executed exactly once to avoid a "capture of possibly uninitialized variable" warning?

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

Answers (1)

trent
trent

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

Related Questions