Is this use of unsafe trivially safe?

user_1527420 0
Viewed 446

I'm running into a Rust borrow checker error that I believe is a limitation of the current implementation of non-lexical lifetimes. The code I want to write looks something like this:

struct Thing {
    value: i32
}

impl Thing {
    fn value(&self) -> &i32 {
        &self.value
    }
    fn increment(&mut self) {
        self.value += 1;
    }
}

/// Increments the value of `thing` if it is odd, and returns a reference to the value.
fn increment_if_odd(thing: &mut Thing) -> &i32 {
    let ref_to_value = thing.value();
    if (*ref_to_value % 2) == 0 {
        return ref_to_value;
    }
    thing.increment();  // fails to compile because the immutable borrow `ref_to_value` is still alive
    thing.value()
}

Rust Playground.

First question: am I right in thinking that this code is 100% safe and the borrow checker is being too conservative? The branch that returns ref_to_value doesn't mutate thing so the reference is guaranteed to be valid, and the other branch doesn't use ref_to_value at all. (I understand that if I replace return ref_to_value; with return thing.value(); it will compile, but in my actual code the value method is expensive.)

It seems I can fix this by "laundering" the reference through a pointer:

if (*ref_to_value % 2) == 0 {
    return unsafe {
        &*(ref_to_value as *const i32)
    }
}

Second question: is this trivially safe? I've never used unsafe before so I'm nervous.

I guess a third question: is there a way to rewrite this in safe Rust? The constaint is that value should only be called once on the non-mutating path.

borrow-checker rust unsafe
2 Answers

The reason the compiler won't allow your code is because ref_to_value must have a lifetime at least as long as the lifetime of the increment_if_odd in order to be returned.

If you add back in the elided lifetimes, ref_to_value must have lifetime 'a. And it's my understanding that the compiler can't change the lifetime of a reference. One way to write safe rust to get around this is to make ref_to_value mutable, and modify Thing::increment.

What your unsafe code does is allow the compiler to give ref_to_value a shorter lifetime, and the new reference, created by casting the pointer, lifetime 'a. I think your unsafe code is "safe" because none of rust's borrowing rules are broken, and we know that the new reference won't outlive the data.

struct Thing {
    value: i32
}

impl Thing {
    fn value(&self) -> &i32 {
        &self.value
    }
    fn mut_value(&mut self) -> &mut i32{
        &mut self.value
    }
    fn increment(val: &mut i32) {
        *val += 1;
    }
}

/// Increments the value of `thing` if it is odd, and returns a reference to the value.
fn increment_if_odd<'a>(thing: &'a mut Thing) -> &'a i32 {
    
    let ref_to_value : &'a mut i32 = thing.mut_value();
    if (*ref_to_value % 2) != 0 {
        Thing::increment(ref_to_value);
    }
    ref_to_value
}
user_83084360

Might be safe, haven't looked at it closely enough. On the other hand you could rewrite the function as:

fn increment_if_odd(thing: &mut Thing) -> &i32 {
    if (*thing.value() % 2) != 0 {
        thing.increment();
    }
    thing.value()
}
user_100487990
Related