Why does `set` method defined on `Cell<T>` explicitly drops the old value? (Rust)
Asked Answered
A

1

14

Interested why does set method defined on Cell, on the last line explicitly drops old value. Shouldn't it be implicitly dropped (memory freed) anyways when the function returns?

use std::mem;
use std::cell::UnsafeCell;

pub struct Cell<T> {
    value: UnsafeCell<T>
}

impl<T> Cell<T> {
    pub fn set(&self, val: T) {
        let old = self.replace(val);
        drop(old); // Is this needed?
    } // old would drop here anyways?
    
    pub fn replace(&self, val: T) -> T {
        mem::replace(unsafe { &mut *self.value.get() }, val)
    }
}

So why not have set do this only:

pub fn set(&self, val: T) {
    self.replace(val);
}

or std::ptr::read does something I don't understand.

Achromatic answered 19/10, 2022 at 10:19 Comment(0)
L
10

It is not needed, but calling drop explicitly can help make code easier to read in some cases. If we only wrote it as a call to replace, it would look like a wrapper function for replace and a reader might lose the context that it does an additional action on top of calling the replace method (dropping the previous value). At the end of the day though it is somewhat subjective on which version to use and it makes no functional difference.

That being said, the real reason is that it did not always drop the previous value when set. Cell<T> previously implemented set to overwrite the existing value via unsafe pointer operations. It was later modified in rust-lang/rust#39264: Extend Cell to non-Copy types so that the previous value would always be dropped. The writer (wesleywiser) likely wanted to more explicitly show that the previous value was being dropped when a new value is written to the cell so the pull request would be easier to review.

Personally, I think this is a good usage of drop since it helps to convey what we intend to do with the result of the replace method.

Lysander answered 19/10, 2022 at 11:3 Comment(1)
Also, in debug builds, if something panics, having two different source line numbers may help pin-point the cause. Though I don't think replace can panic.Shaniceshanie

© 2022 - 2024 — McMap. All rights reserved.