Add Option::take_if

[kellerkindt]

Feature gate:
#![feature(option_take_if)]

This is a tracking issue for adding Option::retain_if similar to Option::take but requires an additional predicate.
It takes() the value if the predicate evaluates to true. If the predicate evaluates to false, the value of the Option remains Some(...). The predicate (Fn(&mut T) -> bool) also has the chance to modify the value.

struct MyStruct {
    pub some_value: Option<u32>,
    // ...
}

fn main() {
    let mut my_struct: MyStruct = out_of_thin_air();

    do_something_before(&my_struct);

    // on non-copy values, this would require a multiple line long match or if-let instead
    my_struct.some_value.take_if(|x| *x == 42);

    // also allows this
    if let Some(prev) = my_struct.some_value.take_if(|x| *x != 42) {
        // do something with the previous value
    }

    do_something_after(&mut my_struct);
    pass_on(my_struct);
}

Steps / History

• ACP: Add Option::retain libs-team#70
• Implementation: Implement Option::take_if #98935
• Final comment period (FCP)¹
• Stabilization PR

Unresolved Questions

• Provide both immutable and mutable retain methods (keeping it analogue to Vec)?
  Consensus seems to be to only implement retain take_if but with &mut T

Footnotes

1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html ↩

[ivan770]

Is there a reason to have both retain and retain_mut when we already have &mut self in retain?

[kellerkindt]

To be honest, personally I can't think of a concrete reason at the moment. Vec has both (although Vec::retain redirects to Vec::retain_mut internally) and I used it as impl orientation.

The question would then also be which name to keep: retain or retain_mut. A *_mut suffixed method without the corresponding not-suffixed one seems odd, but retain and a &mut T in the callback might be unexpected after using Vec::retain{,_mut}?

I am also not sure about all the ~const stuff which I copied over from the Option::filter impl.

edit: I am totally fine with only retain and &mut T

[zirconium-n]

Vec::retain is a mistake and should not be followed. I prefer keep 'retain' only with '&mut' signature.​

[kellerkindt]

Done :)

[kellerkindt]

@ivan770 Do you think there are further changes needed?

[ivan770]

@ivan770 Do you think there are further changes needed?

Signature looks good now, can't say anything about trait bounds though.

[mirror-kt]

Hi! Any progress on this unstable-feature?
It's not that big a feature and I'd like to use it so much that I'd like to move forward with the stabilization process if there are no major issues. Any concerns come to mind?

[mirror-kt]

Hi @cuviper, I can't get a comment on whether it's ready to be stabilized, so I had to ping a member of the libs team! I'd be happy to have them review it.

[cuviper]

The implementation is straightforward, but API goes to a different team -- anyone from @rust-lang/libs-api want to comment?

[Amanieu]

I think this is a generally useful utility function that can make some use cases easier to write & more readable.

@rfcbot fcp merge

[rfcbot]

Team member @Amanieu 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

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.

[krh]

What about Option::replace_if? Like Option::take_if but for Option::replace:

    if let Some(prev) = my_struct.some_value.replace_if(84, |x| *x != 42) {
        // current value is now 84
        // do something with the previous value
    }

[mirror-kt]

What about Option::replace_if

I guess replace_if would be useful as well.
It seems consistent that Option::take_if and Option::replace_if be provided, just as Option::take and Option::replace are provided respectively.

[EndilWayfare]

+1 for Option::replace_if

I've encountered several occasions recently when the implementation of next for an Iterator combinator would have been much more concise and readable in these terms, to the point where I actually did write a crate-local ReplaceIf trait. Glad to see it proposed in an unstable feature!

[EndilWayfare]

This may be starting to cross the "out-of-scope event horizon", but, similarly, I would like to see bool::and_then.

The pattern of

fn maybe_do_a_thing(&mut self) -> Option<ThingStatus> {
   ...
    // Arbitrary operations and state
    // maybe capturable in the subsequent closures
   ...

    if self.owned_type
        .as_ref()
        .some_lazy_calculation()
        .is_some_and(predicate)  // <-- "collapse" lazy calculation, end immutable borrow
    {
        owned_type
            .somehow_get_mut_ref()
            .this_would_be_bad_unless_predicate()
            .by_the_way_this_could_also_be_a_lazy_calculation_now()  // <-- nice for composable APIs
    } else {   //
        None   // I'd rather not see this
    }          //
}

has a habit of cropping up independently in other places. The as_ref really could be take_any_borrow and the somehow_get_mut_ref could be now_pass_by_value.

For instance, when conditionally mutating a shared object behind an Rc, I'd rather not make_mut and potentially force a copy just to make the check to see if I have to.

Or, when using one-or-more-layers of Option to avoid "ambiguous interacting sets of bool" or something. I want to do something in the Some case, but only when a predicate, but I need to pass on an Option, the else is always going to be None, the calculation-after-the-predicate also returns an Option already, and I'd rather not break out of my method chain.

Or (and I'm not joking), a function that returns an iterator in an io::Result which itself yields items in io::Results and loops until exhausted then loops to call the original function again and loops until exhausted but breaks the loop and returns the Err of any of the io::Results should that happen. (That may sound insane, but, due to external constraints, I'm reluctantly rather confident that this is actually the most elegant/optimal it can get...)

I'm a big fan of bool::then and bool::then_some. I will concede that it's possible to abuse/overuse them, but in some circumstances it really is the cleanest and most readable way to express what's happening. I think and_then nicely rounds out the family.

• then_some: Eager Option
• then: Lazy Option
• and_then: Lazy-but-could-be-None Option

You can sort of do this today, with

an_option
    .is_some_and(predicate)
    .then_some(())
    .and_then(maybe_returns_none)

but then_some(()) is silly, unsightly, and noise.

The pattern could then turn into

fn maybe_do_a_thing(&mut self) -> Option<ThingStatus> {
    self.owned_type
        .as_ref()
        .some_lazy_calculation()
        .is_some_and(predicate)
        .and_then(|| {
            owned_type
                .somehow_get_mut_ref()
                .this_would_be_bad_unless_predicate()
                .by_the_way_this_could_also_be_a_lazy_calculation_now()
        })
}

Option::take_if could then be expressible as

// if self.as_mut().map_or(false, predicate) { self.take() } else { None }

self.as_mut().is_some_and(predicate).and_then(|| self.take)

and Option::replace_if

self.as_mut().is_some_and(predicate).and_then(|| self.replace(value))

So, in a sense, bool::and_then may actually be a more fundamental construct than Option::take_if. So if take_if gets in the language, I think it makes sense that and_then would also.

[EndilWayfare]

Actually, I think and_then may be the wrong semantics. and_then closure takes an argument. It's a mapping from the type parameter T of some type M to M<U>. That doesn't really seem analogous to what's going on here at all. I guess bool::then got me thinking in the then direction, and Option has a method called and_then with a function parameter that returns self-but-with-a-different-parameter, so I it seemed like the first obvious choice.

The implementation looks like

fn and_then<F, T>(self, f: F) -> Option<T> where F: FnOnce() -> Option<T> {
    self.then(f).flatten()
}

Looks rather more like the map(f).flatten of flat_map, so... maybe flat_then?