Tracking Issue for feature: "option_insert"

[Canop]

This is a tracking issue for the feature "option_insert".
The feature gate for the issue is #[unstable(feature = "option_insert")].

This feature adds a insert method to any Option, setting its value and returning a mutable reference to this value.
This method removes a cause of unwrap and code complexity.
It allows replacing

option_value = Some(build());
option_value.as_mut().unwrap()

with

option_value.insert(build())

Steps / History

• Implementation: add insert to Option #77392
• Decide on a name. Suggestions:
  • insert (As in the original PR.)
  • insert_and_get
  • replace
  • replace_and_get
  • set
  • ...
• FCP
• Stabilization PR: Stabilize option_insert. #84088

[ThePuzzlemaker]

I'd like to work on stabilizing this.

As for a name, I think that either insert_and_get_mut or replace_and_get_mut would be better than insert_and_get and replace_and_get as it implies that you are getting a mutable reference just from the name, without having to look at any signatures or documentation.

[kupiakos]

insert seems like the best name for this to me, as that matches the existing naming of get_or_insert and get_or_insert_with. insert is just the unconditional version of get_or_insert.

[m-ou-se]

@rfcbot merge

[rfcbot]

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @sfackler

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

[et342]

Should this be called get_replace?

Say, if we have a box and if we put a thing into it, then what we get is a box filled with the thing, and not the thing itself. But, we do get the thing here, so prefix get_ seems appropriate. But, get_insert does not seems appropriate, because if box is filled − then we cannot insert, we can only replace and replacement is what is what's happening here, so get_replace.

[Amanieu]

I feel that insert is the least confusing way to describe the operation that is happening here. We're inserting a value and replacing the old one (just like HashMap::insert). Returning a reference to the inserted element makes sense: that's the only reason you'd use this method instead of just doing foo = Some(bar).

[et342]

I'm not really against it. insert is stretchable enough and kind of makes sense − but also not exactly?

Returning a reference to the inserted element makes sense: that's the only reason you'd use this method instead of just doing foo = Some(bar).

Yes, that's exactly the point. The reference you get is the primary reason to call this method, not the fact that the value gets replaced − that's secondary, because if it's a replacement action that you want, then just perform an assignment. So maybe this should be reflected in the name, like with the other get_ methods of Option.

HashMap::insert

I don't think HashMap::insert is particularly an example in favor of insert here, because in case of HashMap, new value is pushed into the slot, and the old one is pushed out. And it makes sense if we imagine it like we're inserting something somewhere, and what was in there already − falls out from the other side. But in case of insert here − we're not getting the old thing back, it just disappears into nowhere.

Perhaps, get_inserted? We're geting a reference to an unconditionally inserted value.

[m-ou-se]

We discussed this yesterday in the library team meeting, and we agreed to continue with our original decision: insert.

[tmandry]

Semantics aside, I find it confusing that Option::insert returns a reference to the new value, while HashMap::insert returns the old value. That inconsistency makes it harder to remember what both methods do.

(Edit: Sorry for commenting on this after FCP, but since no one brought up this specific point I feel like it's still worth commenting. I'm thinking that this issue didn't get as much attention because there were a lot of FCPs (18!) in the TWiR edition where it was listed.)

[m-ou-se]

There's already Option::replace that returns the old value. I'd argue that HashMap::insert should've been called HashMap::replace (and HashMap::try_insert just HashMap::insert), but that ship has sailed. :(

[WaffleLapkin]

I wonder if it would be useful to have a method on Option which would both return the old value and a reference to the new one. I.e.

fn insert_replace(self, new: T) -> (Option<T>, &mut T) {...}

🤔

[Amanieu]

This seems overly complicated when you can just do:

let old = option.take();
let new_ref = option.insert(new);