Rfcs: Option::get_or_insert_default()

[asafigan]

For example:

let x = None;
*x.get_or_insert_default() += "Hello";

instead of:

let x = None;
*x.get_or_insert_with(Default::default) += "Hello";

This is similar to unwrap_or_default.

[RustyYato]

This seems really specialized, and I don't think that we really need this.

That said, there is some precedent for this:

• Option::unwrap_or_default
• Result::unwrap_or_default
• hash_map::Entry::or_default
• btree_map::Entry::or_default

Note, the most similar api, the HashSet's on nightly (rust-lang/rust#60896) doesn't include a get_or_insert_default, even though it has a get_or_insert_with. But this may be an oversight.

[phaazon]

That seems like a fine addition. However, does not using it still imply a bigger object when we use trait objects?

[RustyYato]

@phaazon How are trait objects involved?

Note: Default::default does not involve trait objects (Default isn't even object-safe!), it is just sugar for <_ as Default>::default

[phaazon]

If we add get_or_insert_default to Option, it means that we need to add a pointer to its vtable for when we pass a dyn Option<T>… right? So if we don’t use that function, we’re getting an extra cost there.

EDIT: I brainfarted.

I was just trying to brainstorm out loud. 😄

[burdges]

There is no trait here @phaazon only the type Option along with this inherent method

impl<T: Default> Option<T> {
    pub fn get_or_insert_default(&mut self) -> &mut T {
        self.get_or_insert(Default::default())
    }
}

We've discussed expanding unsized type handling so that one could work more with bare trait objects, but I think here Option<dyn Trait> comes with different issues. In this case, you're maybe asking if one could use some vtable for Option<dyn Default> provided

impl<T, U> CoerceUnsized<Option<U>> for Option<T> where T: Unsize<U> + ?Sized, U: ?Sized, 

We've no CoerceUnsized enums right not, so maybe enum layout niches requires special care, but maybe it works since we already have

impl<T, U> CoerceUnsized<NonNull<U>> for NonNull<T> where T: Unsize<U> + ?Sized, U: ?Sized, 

Worse, there are presently no CoerceUnsized types that really "interact" with the underlying type, so we surely lack the expanded vtables for types like Option<dyn Default>. If we had them. then Result::ok must transform from the Result<dyn Default,_> vtable into the Option<dyn Default> vtable in this:

(dyn_result: Result<dyn Default,_>).ok().get_or_insert_default()

Awful lot of compiler complexity for quite a niche use case. ;)

[Pauan]

@phaazon If we add get_or_insert_default to Option, it means that we need to add a pointer to its vtable for when we pass a dyn Option… right?

Option isn't a trait, you cannot use dyn Option<T>, there is no pointer, there is no vtable, there is no runtime cost to adding this method.

[phaazon]

Damn, what the heck was I thinking… 😆

[burdges]

I agree with @KrishnaSannasi that this seems overly specialized because:

• If you're programming generically then actually maybe writing out get_or_insert_with(Default::default) gives clarity.. maybe.
• If not then you'd often enter the default manually for clarity.

There is however one argument for doing this: We'd often do get_or_insert(Default::default()) but we should prefer the lazy get_or_insert_with(Default::default) instead, so maybe this shifts that tendency. I'd think a comment about this in the docs suffices, but whatever.

[Ixrec]

The get_or_insert(Default::default()) mistake seems likely to happen whether or not this method exists, so I think the right solution for that is just a clippy lint. I believe they have at least one lint for "accidental non-laziness", but I don't know if that already covers this specific case.

[scottmcm]

FWIW, Vec::resize_default was deprecated in favour of .resize_with(Default::default), so there's precedent in both directions.

I'm going to close this because small inherent API additions like this don't need an RFC, and this isn't a complicated method to implement. If anyone thinks it should be in core, make a PR and get the libs team's feedback that way.