Better way to replace a a field

There's a type Foo that takes a self parameter to do something, like this:

struct Foo {}
impl Drop for Foo {
    fn drop(&mut self) {
        todo!()
    }
}
impl Foo {
    fn consume_and_regenerate(self) -> Foo {
        Foo {} // it is complicated in the real case
    }
}
}

And a Bar type designed by me that hold a Foo:

struct Bar {
    foo: Foo,
}

impl Bar {
    fn update_foo(&mut self) {
       self.foo = self.foo.consume_and_regenerate();
    }
}

The code will not compile, because I can own the foo from a &mut foo, and here's my solution:

fn update_foo(&mut self) {
        let foo = replace(&mut self.foo, unsafe { MaybeUninit::uninit().assume_init() });
        let foo = foo.consume_and_regenerate();
        let foo = replace(&mut self.foo, foo);
        std::mem::forget(foo);
    }

Is there a better (safe) way to do this?

P.S. I don't use an foo: Option<Foo> because there's no case that the option can be none, using an Option<Foo> makes other codes bloated。

You can use replace_with to temporarily take out the value.

Note that doing this has caveats (read the documentation). If you can't stand them, then you must take the Option<Foo> approach.

3 Likes

Your solution may runs Foo's destructor with dangling reference when consume_and_regenerate panic and unwind. take_mut crate provides safe workaround by immediately kill the process on such unwinding.

1 Like

May off-topic here, do you choose "panic=abort" or not when you're writing rust programs?

Regarding choosing panic=abort:

panic=abort can be useful to reduce code size when that is a critical factor (because then the compiler doesn't need to generate code to clean up during unwinding), and there are some platforms (embedded and Wasm) where panic=unwind is not supported.

Regarding supporting panic=abort:

If you are writing a library for others to use, then you should, if at all possible, make your code panic=abort compatible by:

  • Not panicking (use Result etc. instead) if there is any chance that the error condition is something the caller wants to recover from and it isn't evidence of a bug in your library's code.
  • Not using catch_unwind or relying on thread JoinHandles being able to catch panics.
5 Likes

I think replace_with has ub.

An important note

On panic (or to be more precise, unwinding) of the closure f, default will be called to provide a > replacement value. default should not panic – doing so will constitute a double panic and will most likely abort the process.

Afaik. rust doesn't guarantee that panicing while unwinding aborts, so the following example has double free.

let mut string = String::new();

replace_with(
    &mut string, 
    || panic!(),  // if this panic doesn't cause abort `string` will be freed twice
    |s| { 
        drop(s); // `s´ dropped here
        
        panic!()
    }
);

If so, then perhaps you could file an issue.

I thought it did, since that's certainly what it's always done.

That seems to be what https://github.com/rust-lang/rfcs/pull/3288 says it does today too, though of course a proposed RFC isn't at all normative.

(I tried to look for it in the reference, but couldn't find any text about it.)

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.