Tracking issue: Provide Entry-like API for Option #39288

Closed
shahn opened this Issue Jan 25, 2017 · 17 comments

Comments

Projects
None yet
@shahn
Contributor

shahn commented Jan 25, 2017

Often I have code where Option is used to indicate whether a value has been initialized or not. In these instances I often have code that either wants to use the value directly or initialize it and then use the value. So what I'm looking for is similar to as_ref()/as_mut() except they should return &T/&mut T respectively, and take either a value or a closure.

I think I would call the functions get{,_mut}_or_insert{,with}() marrying the get functions (for example in hashmaps) for getting references with the or_insert Entry like functions.

shahn added a commit to shahn/rust that referenced this issue Jan 25, 2017

@Thinkofname

This comment has been minimized.

Show comment
Hide comment
@Thinkofname

Thinkofname Jan 25, 2017

Isn't this just as_ref/mut().unwrap_or(val) and as_ref/mut().unwrap_or_else(|| val) ?

Edit: These don't modify the original Option which I guess it what you wanted

Thinkofname commented Jan 25, 2017

Isn't this just as_ref/mut().unwrap_or(val) and as_ref/mut().unwrap_or_else(|| val) ?

Edit: These don't modify the original Option which I guess it what you wanted

@shahn

This comment has been minimized.

Show comment
Hide comment
@shahn

shahn Jan 25, 2017

Contributor

indeed

Contributor

shahn commented Jan 25, 2017

indeed

@frewsxcv frewsxcv added the A-libs label Jan 25, 2017

shahn added a commit to shahn/rust that referenced this issue Feb 4, 2017

Provide Entry-like API for Option
This implements #39288.

Thanks to @steveklabnik and @oli-obk for their review comments :)

frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 5, 2017

Rollup merge of #39289 - shahn:option_entry, r=alexcrichton
Provide Entry-like API for Option

This implements #39288.

I am wondering whether to use std::intrinsics::unreachable!() here. Both seems fine to me (the second match optimizes away in release mode).

frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 5, 2017

Rollup merge of #39289 - shahn:option_entry, r=alexcrichton
Provide Entry-like API for Option

This implements #39288.

I am wondering whether to use std::intrinsics::unreachable!() here. Both seems fine to me (the second match optimizes away in release mode).
@tbu-

This comment has been minimized.

Show comment
Hide comment
@tbu-

tbu- Feb 6, 2017

Contributor

See also #25149 and #29203. Also, this seems fixed by #39289.

Contributor

tbu- commented Feb 6, 2017

See also #25149 and #29203. Also, this seems fixed by #39289.

@shahn

This comment has been minimized.

Show comment
Hide comment
@shahn

shahn Feb 6, 2017

Contributor

AIUI, this is not fixed by #39289, because this issue is referenced as the tracking issue for the implementation and should stay open until it's decided whether this is to be stabilized or backed out.

Contributor

shahn commented Feb 6, 2017

AIUI, this is not fixed by #39289, because this issue is referenced as the tracking issue for the implementation and should stay open until it's decided whether this is to be stabilized or backed out.

@tbu-

This comment has been minimized.

Show comment
Hide comment
@tbu-

tbu- Feb 6, 2017

Contributor

Ah, right. Maybe you could put "Tracking issue" in the title.

Contributor

tbu- commented Feb 6, 2017

Ah, right. Maybe you could put "Tracking issue" in the title.

@shahn shahn changed the title from Provide Entry-like API for Option to Tracking issue: Provide Entry-like API for Option Feb 6, 2017

@steveklabnik steveklabnik removed the A-libs label Mar 24, 2017

@oberien

This comment has been minimized.

Show comment
Hide comment
@oberien

oberien May 22, 2017

Contributor

As this issue is quite old already (over 3 months), would it be possible to stabilize these two functions soon?

Contributor

oberien commented May 22, 2017

As this issue is quite old already (over 3 months), would it be possible to stabilize these two functions soon?

@brson brson added the I-nominated label May 22, 2017

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson May 23, 2017

Contributor

@rfcbot merge

Contributor

brson commented May 23, 2017

@rfcbot merge

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson May 23, 2017

Contributor

@rfcbot fcp merge

Contributor

brson commented May 23, 2017

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

Show comment
Hide comment
@rfcbot

rfcbot May 23, 2017

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

No concerns currently listed.

Once these reviewers reach consensus, 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 commented May 23, 2017

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

No concerns currently listed.

Once these reviewers reach consensus, 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.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton May 24, 2017

Member

To be clear, the APIs proposed here for stabilization are:

impl<T> Option<T> {
    fn get_or_insert(&mut self, v: T) -> &mut T;
    fn get_or_insert_with<F: FnOnce() -> T>(&mut self, f: F) -> &mut T;
}
Member

alexcrichton commented May 24, 2017

To be clear, the APIs proposed here for stabilization are:

impl<T> Option<T> {
    fn get_or_insert(&mut self, v: T) -> &mut T;
    fn get_or_insert_with<F: FnOnce() -> T>(&mut self, f: F) -> &mut T;
}
@rfcbot

This comment has been minimized.

Show comment
Hide comment
@rfcbot

rfcbot Jun 6, 2017

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

rfcbot commented Jun 6, 2017

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

@rfcbot

This comment has been minimized.

Show comment
Hide comment
@rfcbot

rfcbot Jun 16, 2017

The final comment period is now complete.

rfcbot commented Jun 16, 2017

The final comment period is now complete.

@dhardy

This comment has been minimized.

Show comment
Hide comment
@dhardy

dhardy Jun 26, 2017

Contributor

This isn't Entry-like. If it was, it would be possible to do the following:

match opt_value.entry() {
    VacantEntry(entry) => entry.insert(x),
    OccupiedEntry(entry) => { assert_eq!(entry.get(), x); }
}
Contributor

dhardy commented Jun 26, 2017

This isn't Entry-like. If it was, it would be possible to do the following:

match opt_value.entry() {
    VacantEntry(entry) => entry.insert(x),
    OccupiedEntry(entry) => { assert_eq!(entry.get(), x); }
}
@SimonSapin

This comment has been minimized.

Show comment
Hide comment
@SimonSapin

SimonSapin Jun 27, 2017

Contributor

@dhardy That particular example can be written as:

match opt_value {
    ref mut entry @ None => *entry = Some(x),
    Some(ref entry) => assert_eq!(entry, x),
}
Contributor

SimonSapin commented Jun 27, 2017

@dhardy That particular example can be written as:

match opt_value {
    ref mut entry @ None => *entry = Some(x),
    Some(ref entry) => assert_eq!(entry, x),
}

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 20, 2017

std: Stabilize `option_entry` feature
Stabilized:

* `Option::get_or_insert`
* `Option::get_or_insert_with`

Closes #39288

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 23, 2017

std: Stabilize `option_entry` feature
Stabilized:

* `Option::get_or_insert`
* `Option::get_or_insert_with`

Closes #39288

@stuhood stuhood referenced this issue in rust-lang/rfcs Jul 25, 2017

Closed

Add `unwrap_or_put` method to `Option<T>` #1405

@bors bors closed this in 64c1b23 Jul 27, 2017

mattico added a commit to mattico/rust that referenced this issue Jul 29, 2017

std: Stabilize `option_entry` feature
Stabilized:

* `Option::get_or_insert`
* `Option::get_or_insert_with`

Closes #39288

matthewhammer added a commit to matthewhammer/rust that referenced this issue Aug 3, 2017

std: Stabilize `option_entry` feature
Stabilized:

* `Option::get_or_insert`
* `Option::get_or_insert_with`

Closes #39288

alexcrichton added a commit to alexcrichton/rust that referenced this issue Aug 12, 2017

std: Stabilize `option_entry` feature
Stabilized:

* `Option::get_or_insert`
* `Option::get_or_insert_with`

Closes #39288
@raphlinus

This comment has been minimized.

Show comment
Hide comment
@raphlinus

raphlinus Nov 1, 2017

Contributor

To save time for people researching this, the feature landed in stable at Rust 1.20 (source: 64c1b23).

Contributor

raphlinus commented Nov 1, 2017

To save time for people researching this, the feature landed in stable at Rust 1.20 (source: 64c1b23).

@shepmaster

This comment has been minimized.

Show comment
Hide comment
@shepmaster

shepmaster Nov 1, 2017

Member

@raphlinus I think that commit is wrong :-) The function in question doesn't show up in the 1.20.0 docs but does in the 1.21.0 docs. Nope, never mind; I just forgot how to search, sigh.

GitHub does say that commit is only in the 1.21.0 tag though...

Member

shepmaster commented Nov 1, 2017

@raphlinus I think that commit is wrong :-) The function in question doesn't show up in the 1.20.0 docs but does in the 1.21.0 docs. Nope, never mind; I just forgot how to search, sigh.

GitHub does say that commit is only in the 1.21.0 tag though...

@shepmaster

This comment has been minimized.

Show comment
Hide comment
@shepmaster

shepmaster Nov 1, 2017

Member

Ah, the 1.20.0 commit was a backport, thus why it's a different tag.

Member

shepmaster commented Nov 1, 2017

Ah, the 1.20.0 commit was a backport, thus why it's a different tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment