Option could have a function that returns &mut

Hi,

In the docs I didn't find a function that returns &mut https://doc.rust-lang.org/std/option/enum.Option.html

So I have to as_mut().unwrap()

Why don't you want to use as_mut().unwrap()?

What's the problem you're trying to solve? This might be a xy problem.

3 Likes

How do you know the unwrap() is not going to fail?

If you have a pattern like:

if foo.is_some() {
   *foo.as_mut().unwrap() += 1;
}

usually you can change it to:

if let Some(foo) = &mut foo {
    *foo += 1;
}
6 Likes

I think it's a very basic and valid need to change what's inside an Option without consuming it, and you know that the Option is Some, so no need for a match or if let.

It could be like:

    pub fn get(&mut self) -> &mut T { // or get_as_mut_ref
        match *self {
            Some(ref mut val) => val,
            Null => panic!("called `Option::get()` on a `None` value"),
        }
    }

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=292edbccc50ef5719764f1844e766d74

I saw that some important projects use as_mut().unwrap().

tokio (master)

tokio/src/io/async_fd.rs:        self.inner.as_mut().unwrap()
tokio/src/io/driver/mod.rs:            self.resources.as_mut().unwrap().compact()
tokio/src/io/driver/mod.rs:        let resources = self.resources.as_mut().unwrap();
tokio/src/runtime/basic_scheduler.rs:        self.inner.as_mut().unwrap().block_on(future)
tokio/src/runtime/shell.rs:        let driver = self.inner.as_mut().unwrap();
tokio-util/src/sync/intrusive_double_linked_list.rs:        self.tail.as_mut().unwrap().as_mut().next = Some(node.into());

hyper (master)

src/client/conn.rs:            ready!(me.as_mut().unwrap().poll_ready(cx))?;
src/client/conn.rs:            ready!(conn.as_mut().unwrap().poll_without_shutdown(cx))?;
src/client/conn.rs:        match ready!(Pin::new(self.inner.as_mut().unwrap()).poll(cx))? {
src/client/dispatch.rs:                    ready!(cb.as_mut().unwrap().poll_canceled(cx));
src/client/tests.rs:        try_ready!(sock1.as_mut().unwrap().read(&mut [0u8; 512]));
src/ffi/task.rs:        match Pin::new(&mut self.task.as_mut().unwrap().future).poll(cx) {
src/server/conn.rs:            match *self.conn.as_mut().unwrap() {
src/server/conn.rs:            ready!(conn.as_mut().unwrap().poll_without_shutdown(cx))?;
src/server/conn.rs:            match ready!(Pin::new(self.conn.as_mut().unwrap()).poll(cx)) {
src/server/conn.rs:                match ready!(Pin::new(self.inner.conn.as_mut().unwrap()).poll(cx)) {

wasmtime (main)

cranelift/object/src/backend.rs:        let &mut (symbol, ref mut defined) = self.functions[func_id].as_mut().unwrap();
cranelift/object/src/backend.rs:        let &mut (symbol, ref mut defined) = self.data_objects[data_id].as_mut().unwrap();
crates/bench-api/src/lib.rs:    let state = unsafe { (state as *mut BenchState).as_mut().unwrap() };
crates/bench-api/src/lib.rs:    let state = unsafe { (state as *mut BenchState).as_mut().unwrap() };
crates/bench-api/src/lib.rs:    let state = unsafe { (state as *mut BenchState).as_mut().unwrap() };
crates/fuzzing/src/oracles.rs:                config.as_mut().unwrap().debug_info(b);
crates/fuzzing/src/oracles.rs:                config.as_mut().unwrap().interruptable(b);
crates/jit/src/code_memory.rs:        let e = self.current.as_mut().unwrap();

deno (main)

cli/tests/integration_tests.rs:      let stderr = child.stderr.as_mut().unwrap();
...
core/runtime.rs:    self.v8_isolate.as_mut().unwrap()
core/runtime.rs:    let snapshot_creator = self.snapshot_creator.as_mut().unwrap();
runtime/inspector.rs:    self.v8_inspector.as_mut().unwrap()
runtime/ops/fs.rs:  let pos = (*fs_file).0.as_mut().unwrap().seek(seek_from).await?;
runtime/ops/fs.rs:  (*fs_file).0.as_mut().unwrap().sync_data().await?;
runtime/ops/fs.rs:  (*fs_file).0.as_mut().unwrap().sync_all().await?;
runtime/ops/fs.rs:  let metadata = (*fs_file).0.as_mut().unwrap().metadata().await?;
runtime/ops/fs.rs:  (*fs_file).0.as_mut().unwrap().set_len(len).await?;
runtime/ops/io.rs:      let nwritten = fs_file.0.as_mut().unwrap().read(buf).await?;
runtime/ops/io.rs:      let nwritten = fs_file.0.as_mut().unwrap().write(buf).await?;
runtime/ops/io.rs:      fs_file.0.as_mut().unwrap().flush().await?;
runtime/ops/plugin.rs:    self.fut.as_mut().unwrap().poll_unpin(ctx)
runtime/ops/tty.rs:    let maybe_tty_mode = &mut fs_file_resource.1.as_mut().unwrap().tty.mode;
runtime/worker.rs:    let inspector = self.inspector.as_mut().unwrap();
test_util/src/lib.rs:      let stdout = test_server.stdout.as_mut().unwrap();

rust (master)

src/test/ui/associated-types/issue-65774-1.rs:        let valref: &mut <S as MPU>::MpuConfig = val.as_mut().unwrap();
src/test/ui/associated-types/issue-65774-2.rs:        let valref: &mut <S as MPU>::MpuConfig = val.as_mut().unwrap();
src/test/ui/issues/issue-13304.rs:    p.stdin.as_mut().unwrap().write_all(b"test1\ntest2\ntest3").unwrap();
src/test/ui/issues/issue-41498.rs:    x.as_mut().unwrap()
src/test/ui/issues/issue-70746.rs:        (self.callback.as_mut().unwrap())(d)
src/tools/compiletest/src/runtest.rs:            child.stdin.as_mut().unwrap().write_all(input.as_bytes()).unwrap();

But would a new option.unwrap_as_mut() method be significantly better than the existing option.as_mut().unwrap()?

It doesn't add any functionality, so I think it's worth adding only if it's significantly easier to write (e.g., more discoverable) or to read (e.g., more clear what it does in both Some and None cases). Personally, I find the existing methods sufficient on both counts.

18 Likes

I understand you, but I don't agree:

  1. You don't need to be significantly better. Just a little bit is enough;
  2. It denifitely adds functionality. Currently Option has lots of functions, but none that returns &mut T, and I think this is wrong.

I think we should not focus on the length of the writing unwrap_as_mut() vs as_mut().unwrap().
It's important, but not the gist.

That's not even true by the letter, there is get_or_insert, get_or_insert_with, insert (unstable)β€”all of them panic free.

1 Like

Only literally.

Totally different functionality/use case.

Not really, plus if you want to be panic free you could also use .as_ref().map(|x| ...).

This seems to be a similar line of argument to the not_empty() thread.

I believe that should be .as_mut().map(...). Though I'm not sure about the benefit of not panicking. If I assume incorrectly that an Option can't None, I want that error caught.

1 Like

I feel like an Option::<T>::update<R>(&mut self, transform: impl FnOnce(&mut T) -> R) -> Option<R> would make more sense. It would be panic free and encodes the intended use the name.

Edit: but even then, I'm not sure how that would be better than a normal if let

1 Like

Only for chainable operations

Hm - I hadn't thought if chaining! It would be cool to be able to do foo.update(...).map(...), etc as opposed to

(if let Some(x) = foo { ... } else { None }).map(...)

Is there anywhere that this would be useful though? I can't really think of a time I've wanted to chain mutations like that

I see that some people worry about panic, but in this case (replacing as_mut().unwrap()) it's the correct behaviour, because it's assuming that it is Some and if it's not it's better to get a panic

If you are unsure if it's Some, then just use if let or match

How some other languages let you change the value inside an optional, eg:

struct Lang { txt: String }

C++

opt.value().txt = "C++";
// or
opt->txt = "C++";

Swift

opt?.txt = "Sweet"
// or
opt!.txt = "Nice"

Java

opt.get().txt = "Java";

Rust

opt.as_mut().unwrap().txt = "Rust".to_string();

I know that for people that are already are used to Rust, it's probably not a problem for them, just a minor thing. But care must be taken to not get blind because maybe it's not good for users, even more for new users.

So my newbie perspective:

I first looked at the docs and didn't find anything that returns &mut. (There are insert, get_or_insert, but that's a different use case, because you need to pass some T ...)

Then I thought: Why the hell there's nothing returning &mut? Well whatever, I will use if let even it being nonsense because I knew it was Some.

Then I found out that I could combine as_mut and unwrap to get what I wanted. But that is a workaround and not user friendly. Swift even created syntax sugars for that.

So, get is still available and I think it would be a good addition.

(and before someone say "if you know it is Some you are doing it wrong", etc. I already showed in my previous reply some important projects that are using as_mut().unwrap(), so there is a use case for that)

I would argue that .as_mut().unwrap() is not a workaround. It is exactly what you should use in situations like that. It is a good thing that I have to call .unwrap(), because it tells me that it might panic. It also allows me to customize the panic message:

name.as_mut().expect("a girl has no name")

which is useful when you're debugging a panic.

Note that other languages don't panic in this case, they throw an exception. The equivalent in Rust would therefore be

opt.as_mut()?
// or
opt.as_mut().ok_or_else(|| ...)?
5 Likes

Thank you Aloso.

But why do you know that unwrap panics? =)

from the docs right?

It would also be in the docs that get panics.

What do you think of

opt.update(|v| { v.txt = "Rust".to_string(); })

(I'm not sure whether the curly braces are necessary, and |v| might have to be |&mut v|.)

This would be another way to write

if let Some(&mut v) = opt { v.txt = "Rust".to_string(); }

and I think the main argument I'd make for adding it is that it puts the object being modified in a much more obvious position -- at the head of the sentence, instead of buried in the middle.

1 Like

Hi zackw,

Thank you.

So if I had to choose:

opt.update(|v| v.txt = "Rust".to_string());
if let Some(opt) = &mut v { v.txt = "Rust".to_string(); }

// or
opt.get().txt = "Rust".to_string();

I would prefer the last option. :slight_smile:

update is interesting, although nothing guarantees that the function you are passing is actually updating anything.

Even if there was update I think get would be still be go to have. Like, imagine someone new to Rust and you tell him he would have to use: opt.update(|v| v.txt = "Rust".to_string());
He is just learning the language and already would have know closures.

I tried your update here: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=db75167dd81c7179733b050b974dfcb9

I think probably what you're looking for is

opt.get_or_insert_with(Default::default).foo = "set".into()

I think this could be offered as a standalone method

fn get_or_default() -> &mut T where T : Default;
1 Like

If you don't know if there's Some in Option, then get() would panic. This should generally be avoided, as panics are disruptive for larger programs and are meant as a last-resort safety feature, not as a common error handling technique. Panicking methods are mostly for tests and short examples where "lazy" error handling can be forgiven.

OTOH if you know from context that opt.get() is fine to call, then you should probably not use Option at this point, e.g.

let mut val = opt?;
val.txt = "Rust".into();

There are two mutually-exclusive concepts here:

  1. If I don't know if a value exists, I use Option, and therefore I can't use it without checking
  2. If I know the object exists, I can use it without checking, and therefore I don't use Option

But if you have Option, but you can use it without checking (and panicking), it's a weird and unnecessary combination. It's the worst of both alternatives. You should refactor your code to avoid situations where such .get() would ever be useful.


Rust has lots of ways to gracefully unwrap options or to avoid them, so that operating on Option directly is not needed. If you have an iterator of optional values, there's filter_map(|x| x). If you're working with hashmaps and don't know if a value is there, instead of working with Option, there's Entry API. If a function takes an optional argument you can do (assuming it returns Result and you've defined such error type):

let val = opt.ok_or(Error::MissingArgument)?

so if you find yourself writing as_ref().unwrap() frequently, take a step back and see where you could have unwrapped earlier or avoided using Option.

4 Likes