Tracking Issue for Option::get_or_insert_default

[camsteffen]

Feature gate: #![feature(option_get_or_insert_default)]

This is a tracking issue for adding Option::get_or_insert_default.

Basically, it is a shorthand for option.get_or_insert_with(Default::default). It differs with unwrap_or_default since it does not consume the Option. This is useful, for example, when you have an Option as a struct field or inside a data structure like Vec<Option<T>>.

Public API

impl<T> Option<T> {
    pub fn get_or_insert_default(&mut self) -> &mut T where T: Default;
}

Steps / History

• Original issue: Option should have method get_or_default #55042
• Implementation: Add Option::get_or_default #82849
• Renamed from get_or_default to get_or_insert_default: Rename Option::get_or_default to get_or_insert_default #82977
• Final commenting period (FCP)
• Stabilization PR

Unresolved Questions

• None yet.

[m-ou-se]

It's not obvious to me at all from the name that this modifies the Option. It sounds like it'd be the same as unwrap_or_default. Something like get_or_insert_default would be more consistent with the other method names.

[camsteffen]

Hmm I hadn't considered that.

[sffc]

+1 on the name get_or_insert_default

I wanted to use this today, but it's still experimental. 😆

[insou22]

+1 -- was looking for this exact method today

[djc]

@rustbot ping libs

Let's see if this works now? Would be nice to have this FCP'ed.

cc @joshtriplett

[rustbot]

Error: Only Rust team members can ping teams.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[joshtriplett]

LGTM

@rfcbot merge

[rfcbot]

Team member @joshtriplett 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
• @yaahc

Concerns:

• is this in the same category as resize_default (Tracking Issue for Option::get_or_insert_default #82901 (comment))

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. 🔔

[dtolnay]

@rfcbot concern is this in the same category as resize_default

Many years ago we deleted vec.resize_default(n) because it just wasn't worth having when vec.resize_with(n, T::default) is equally suitable.

• Tracking issue for Vec::resize_with and resize_default #41758
• Deprecate the unstable Vec::resize_default #57656
• Remove deprecated unstable Vec::resize_default #77850

It seems like it came down to just not being worth dedicating a whole new method given how rarely resize_with is used in the first place. This is in contrast to option.unwrap_or_default() as shorthand for option.unwrap_or_else(T::default), which is the only existing precedent for T::default getting a dedicated convenience that I know of, and is justified by being used literally over 100× more than resize_with.

I suppose option.get_or_insert_default() is much closer to the resize_with end of this spectrum than the unwrap_or_default end, and option.get_or_insert_with(T::default) would be good enough.

[imp]

My own experience (which may or may not be representative) tells that in every single case I wanted option.get_or_insert_-like functionality I actually wanted option.get_or_insert_default(). So in my books it is much closer to unwrap_or_default than to resize_with. Just my 2 shekels.

[dtolnay]

Be aware that whether or not you want the default value is not the only consideration here. You also require a situation in which T is completely inferrable. The combination of those 2 is less common in my experience (especially if I also take into account experience from similar situations in the Entry API). Extrapolating "every single case I wanted the default" to "therefore I would have found get_or_insert_default valuable if available" is not correct.

use std::collections::BTreeSet;
use std::iter;

fn main() {
    let mut junks = None;
    for (id, value) in iter::repeat(("junk", 99)).take(10) {
        if id == "junk" {
            junks.get_or_insert_with(BTreeSet::default).insert(value);
        }
    }
    println!("{:?}", junks);
}

With get_or_insert_default:

error[E0282]: type annotations needed for `Option<T>`
 --> src/main.rs:5:9
  |
5 |     let mut junks = None;
  |         ^^^^^^^^^
...
8 |             junks.get_or_insert_default().insert(value);
  |                                           ------ type must be known at this point

[imp]

Very good example. However, in my opinion it doesn't lower the value of get_or_insert_default. Just my opinion - I realize others can feel differently.

[Rua]

Is there anything blocking stabilisation of this?

[SmnTin]

What is the current status? Can we stabilize it?

[nyurik]

Are there steps to move this forward and stabilize? I just encountered tons of these cases in PGRX - and was surprised it wasn't stable yet

[dtolnay]

My opinion remains #82901 (comment) and I would prefer not to grow the API of Option with this method. I think this is a frivolous wrapper around an existing good, usable method. The operation that this new method implements is not so widespread that it deserves a dedicated method on this foundational type, on top of the already available one. The writeability gain is minuscule or negative and the readability gain is minuscule or negative.

I am open to being overruled 4:1 if I'm wrong about this. Among the unchecked boxes in the FCP proposal, Jane has departed the team since then but I am interested to hear @BurntSushi's opinion (and if the other team members could reaffirm that they still like this method, since those approvals all happened before the critical comments — as did mine before I eventually changed my mind).

[BurntSushi]

I think for me, Option (and Result) have become so big, that the marginal cost of another combinator has become rather small. And in this case, I perceived it as an extension of a pattern that we've already established with unwrap_or_default (which I understand you addressed by pointing out that it is exceptionally common) and Entry::or_default. What that means is that this isn't just a whole new method as it is relying on an established pattern that is pretty common. So the additional conceptual burden it brings seems somewhat small to me.

With that said, if most uses of this new routine require inference help, then I would agree that the juice does not seem worth squeeze. @nyurik in particular linked to a code sample where the Option type was already written down, which I think is probably the uncommon case. So I think one thing folks could do here is show more real world examples where this method would work without inference failures.

I would say my current opinion is "weakly in favor if inference failures don't happen in most cases." But if using this method just means that you'll likely need to write down the type anyway, then I think my position would be "not strongly opposed."

[dtolnay]

In the compiler, there are 28 calls of Option::get_or_insert_with and Option::get_or_insert_default currently. 79% need to stick with get_or_insert_with because they are inserting with something that is not the default value. (This is quite divergent from #82901 (comment). But it closely matches the ratio of unwrap_or_else to unwrap_or_default in the compiler: 83% of those calls are unwrap_or_else.)

Of the remainder, the 2 in compiler/rustc_session/src/options.rs and the 3 in compiler/rustc_middle/src/mir/interpret/allocation/provenance_map.rs are inserting default into a struct field or function argument, where the type already needs to have been declared anyway.

And 1 in compiler/rustc_mir_transform/src/elaborate_box_derefs.rs uses get_or_insert_default but requires inference help.

     if let VarDebugInfoContents::Place(place) = &mut debug_info.value {
-        let mut new_projections = None;
+        let mut new_projections: Option<Vec<_>> = None;
         let mut last_deref = 0;

         for (i, (base, elem)) in place.iter_projections().enumerate() {
             let base_ty = base.ty(&body.local_decls, tcx).ty;

             if elem == PlaceElem::Deref && base_ty.is_box() {
-                let new_projections = new_projections.get_or_insert_with(Vec::new);
+                let new_projections = new_projections.get_or_insert_default();

                 let (unique_ty, nonnull_ty, ptr_ty) =
                     build_ptr_tys(tcx, base_ty.boxed_ty(), unique_did, nonnull_did);

[dtolnay]

In PGRX which was linked above as a motivating use case, there are 3 functions that call Option::get_or_insert_with using a default, of which 2 (here and here) would require adding a type to the declaration of the Option local variable in order to be able to use get_or_insert_default with it, while 1 (here) would not.

[camsteffen]

would require adding a type to the declaration of the Option local variable in order to be able to use get_or_insert_default with it

My personal take on that scenario is that adding a type annotation to the variable is a readability win. I prefer to know/define the types of things upfront for clarity, and then value-modifying code is not responsible for providing type information to the reader or to the compiler.

[nyurik]

I just did a quick github search for get_or_insert_with(Default, and there are 1.8k instances. I think this is fairly significant, but obviously this is not the most accurate metric. Doing some spot checks does appear they would benefit from this new feature. Note that the search result already implies known type -- e.g. it acts on a struct field or a method param.