Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Option.unwrap documentation more precise #23713

Closed
jviereck opened this issue Mar 25, 2015 · 10 comments · Fixed by #23791
Closed

Make Option.unwrap documentation more precise #23713

jviereck opened this issue Mar 25, 2015 · 10 comments · Fixed by #23791

Comments

@jviereck
Copy link
Contributor

Reading the current documentation about Option<T>.unwrap() at

http://doc.rust-lang.org/std/option/enum.Option.html#method.unwrap

it is not clear to me what the effect of unwrap() is. I was supposing calling this function returns the value stored in the Option type and replaces it by an None value. However, this seems not to be the case as the second assertion in the following example program passes fine:

fn main() {
    let 
    let x = Some("air");
    assert_eq!(x.unwrap(), "air");
    assert_eq!(x.is_some(), true);
}

(You can run the program online at http://goo.gl/F9OnFk).

In addition, the current documentation says:

Returns the inner T of a Some(T).

Does this means the value contained in the Some(T) is moved out or cloned or is a borrow taken? Also, T is a type and I don't think that the function really returns a type, but a value of type T. Therefore, should the quoted sentence maybe reworded towards something along the lines of:

Returns the value of type T as it is contained in the Some(T).

Please let me know what you think about these points. I am more than happy to provide a PR to improve the documentation, but I don't know all the answers to the questions I have raised above and I am also not sure if my claims are correct :/

@apasel422
Copy link
Contributor

unwrap moves the value out of the option if it is Some, or panics if it is None. In your example, the call to unwrap performs an implicit copy of x because x is of type Option<&str>, and Option<T> implements Copy when T implements Copy (as &str does).

This example will not compile, because Foo does not implement Copy:

#[derive(PartialEq, Debug)]
struct Foo;

fn main() {
    let x = Some(Foo);
    assert_eq!(x.unwrap(), Foo);
    assert_eq!(x.is_some(), true);
}

The error is:

foo.rs:7:16: 7:17 error: use of moved value: `x`
foo.rs:7     assert_eq!(x.is_some(), true);
                        ^
<std macros>:1:1: 9:39 note: in expansion of assert_eq!
foo.rs:7:5: 7:35 note: expansion site
foo.rs:6:16: 6:17 note: `x` moved here because it has type `core::option::Option<Foo>`, which is non-copyable
foo.rs:6     assert_eq!(x.unwrap(), Foo);
                        ^
<std macros>:1:1: 9:39 note: in expansion of assert_eq!
foo.rs:6:5: 6:33 note: expansion site

The documentation could be updated to mention moving, but this is implicit in the method's self parameter, which is by-value.

@jviereck
Copy link
Contributor Author

@apasel422 thanks a lot for your explanation! Still, I don't think I grasped how the unwrap works completely yet. Here is an example:

struct Item {
    id: isize
}

struct EntryMutBorrow<'a> {
    body: Option<&'a mut Item>
}

fn main() {
    let mut item = Item { id: 1 };
    let entry_ref = &mut EntryMutBorrow { body: Some(&mut item) };

    // ATTEMPT FAILED:
    // The following does not work as moving a value from a borrow
    // is not possible and `entry_ref.body` a borrow.
    //
    //   error: cannot move out of borrowed content
    //
    // let item_mut_borrow: &mut Item = entry_ref.body.unwrap();

    // ATTEMPT WORKS:
    // The following works, but it is not clear to me why: Why is moving the
    // value from an `Option<&mut &mut Item>` type not an problem but having
    // a single borrow as `Option<&mut Item>` is not enough?
    let item_as_ref: Option<&mut &mut Item> = entry_ref.body.as_mut();
    let item_mut_borrow: &mut &mut Item = item_as_ref.unwrap();

    // Update the id on the entry.item.id - the println! confirms the change succeeded.
    item_mut_borrow.id = 2;
    println!("entry.body.id={}", item_mut_borrow.id);

    // PROGRAM OUTPUT:
    // entry.body.id=2
}

I understand that moving a value from a borrowed object is not possible and that's why the first attempt fails. However, I don't see why the second attempt works. In a sense, the new type after the call to entry_ref.body can be written as Option<S> with S ≡ &mut &mut Item. In that case the call to Option<S>.unwrap() as done in entry_ref.body.as_mut().unwrap() looks to me again as a move. But as entry_ref.body is borrowed, I don't see why this move is different from the first move attempt in my code, which failed.

Is there a special rule to the Option<T>.unwrap() method, which makes the later case work but the first not or am I missing something on how the borrow/move rules in rust work?

@apasel422
Copy link
Contributor

Basically, in order to call unwrap, you need to have an owned Option. This is the case in your "ATTEMPT WORKS" code because as_mut goes from &mut Option<T> to Option<&mut T>.

The code under "ATTEMPT FAILED" doesn't compile because entry_ref is borrowed (let entry_ref = &mut EntryMutBorrow), not because entry_ref.body contains a borrowed value.

For example, the following code does compile:

struct Item {
    id: isize
}

struct EntryMutBorrow<'a> {
    body: Option<&'a mut Item>
}

fn main() {
    let mut item = Item { id: 1 };
    let entry_ref = EntryMutBorrow { body: Some(&mut item) };
    let item_mut_borrow: &mut Item = entry_ref.body.unwrap();
    item_mut_borrow.id = 2;
    println!("entry.body.id={}", item_mut_borrow.id);

    // PROGRAM OUTPUT:
    // entry.body.id=2
}

@jviereck
Copy link
Contributor Author

Thanks again @apasel422! The point that you need an owned Option makes sense to me and was something I was not that much aware about before :)

What I don't see at this point is why entry_ref.body in my example is not an owned Option already. As you can see in this code example:

fn test_05() {
    let mut item = Item { id: 1 };
    let entry_ref = &mut EntryMutBorrow { body: Some(&mut item) };

    // The following type checks, so I guess the type of `entry_ref.body`
    // is `Option<&mut Item>` here. The program is stil rejected because
    //
    //   error: cannot move out of borrowed content
    //
    let body_ref: Option<&mut Item> = entry_ref.body;
}

It seems that entry_ref.body is of the owning type Option<&mut Item> already. From reading your last comment it sounds to me that entry_ref.body is supposed to be of type &mut Option<T> with T≡&mut Item here. But that is something I don't see yet :/

Any comments is highly appreciated!

PS: @apasel422, I hope digging deeper on this issue is okay for you? From your comments so far I think I can fix the Option.unwrap documentation but still I would love to have a good overall understanding how the interaction with Option<T> really works :)

@apasel422
Copy link
Contributor

In that example, the type of entry_ref.body is indeed Option<&mut Item>, but it is not owned from the perspective of the compiler, because it is accessed through a reference (entry_ref).

@jviereck
Copy link
Contributor Author

Ahh, I think now I get it - thanks a lot @apasel422. Are these the (hidden) steps the compiler is doing when executing entry_ref.body.as_mut().unwrap();?

  • The compiler knows that entry_ref.body is of type Option<&mut Item>
  • The definition of Option<T>.as_mut() is given by fn as_mut<'r>(&'r mut self) -> Option<&'r mut T>, which means for calling the as_mut(), the self must be a mutable borrow.
  • As the entry_ref.body is NOT a mutable borrow, the compiler takes an implicit mutable borrow like self=(&mut entry_ref.body) and uses the result as the self argument when calling as_mut.
  • The call to Option.as_mut(&mut entry_ref.body) does type check and returns a fully owned Option<&'r mut T> type. Here I am using the term "fully owned" to differentiate to the case of the previous entry_ref.body which is not owned due to the access through the reference of entry_ref.
  • Because the result of as_mut is now fully owned, the call to Option<T>.unwrap() type checks.

I've got to say: I love and be impressed what the rust compiler is doing underneath here (once you understand it :) )!

@apasel422, thank you sooo much for these helpful and very educational comments!!!

@steveklabnik
Copy link
Member

So, given all this: is there anything actionable we can do to improve these docs? What would you suggest, @jviereck ?

@jviereck
Copy link
Contributor Author

@steveklabnik, how do you feel about this:

  • the description @apasel422 gave as unwrap moves the value out of the option if it is Some, or panics if it is None sounds much better and is more precise compared to the current description of Option<T>.unwrap(), which is Returns the inner T of a Some(T).. Therefore, I would love to exchange them.
  • do you think there is value in adding a section about .as_mut().unwrap() and how it works to the existing documentation?

@steveklabnik
Copy link
Member

Therefore, I would love to exchange them.

I like it! would you like to submit a PR or should I take care of it?

do you think there is value in adding a section about .as_mut().unwrap() and how it works to the existing documentation?

I'm not sure.

@apasel422
Copy link
Contributor

I don't think we need to add anything about as_mut().unwrap(), because there's nothing special about it. It just follows Rust's ownership and borrowing rules.

However, Option's documentation in general does need work, both convention-wise and content-wise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants