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

Entry APIs should have or_default for V: Default #44324

Closed
jonhoo opened this Issue Sep 4, 2017 · 8 comments

Comments

Projects
None yet
@jonhoo
Copy link
Contributor

jonhoo commented Sep 4, 2017

The various Entry APIs (e.g., hash_map::Entry, btree_map::Entry) currently have or_insert, and or_insert_with. However, when V: Default, I believe they should also have an or_default. This would make it nicer to work with things like maps of maps, maps of lists, etc. While it's true that this is equivalent to or_insert_with(Default::default), I would argue that or_default is much more readable.

I'd be happy to take a stab at implementing this, but wanted to air the idea first.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Sep 5, 2017

Generally, the way the libs team works is "open a PR if a change is small, open an RFC if a change is big." I'm not on the libs team, but this feels pretty small to me; you already almost have an implementation in the comment!

@jonhoo

This comment has been minimized.

Copy link
Contributor Author

jonhoo commented Sep 5, 2017

@steveklabnik PR submitted!

bors added a commit that referenced this issue Sep 12, 2017

Auto merge of #44344 - jonhoo:entry_or_default, r=BurntSushi
Add or_default to Entry APIs

As argued for in #44324, this PR adds a new `or_default` method to the various `Entry` APIs (currently just for `BTreeMap` and `HashMap`) when `V: Default`. This method is effectively a shorthand for `or_insert_with(Default::default)`.

@TimNN TimNN added the A-collections label Sep 17, 2017

@dtolnay

This comment has been minimized.

Copy link
Member

dtolnay commented Nov 15, 2017

An unstable or_default() method was added in #44344. This issue is now the tracking issue for stabilization.

@khuey

This comment has been minimized.

Copy link
Contributor

khuey commented Feb 20, 2018

Is there any reason not to just stabilize this?

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Mar 17, 2018

In Nightly for 6 months, looks good to me to stabilize.

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Mar 17, 2018

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), 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 comment has been minimized.

Copy link

rfcbot commented Mar 19, 2018

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

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Mar 29, 2018

The final comment period is now complete.

bors added a commit that referenced this issue Jun 10, 2018

Auto merge of #51079 - GuillaumeGomez:stabilize-entry-or-default, r=S…
…imonSapin

Stabilize entry-or-default

Fixes #44324.

cc @SimonSapin

@bors bors closed this in #51079 Jun 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.