Tracking issue for `bool::then_some`

[varkor]

Tracking issue for the bool::then_some method, which abstract the following pattern:

if some_condition {
    Some(t)
} else {
    None
}

bool:then has previously been stabilised as part of this feature gate: https://github.com/rust-lang/rust/pull/79299

RFC: https://github.com/rust-lang/rfcs/pull/2757 Implementation PR: https://github.com/rust-lang/rust/pull/64255

[U007D]

In trying this out, I was puzzled why the eager/lazy argument evaluation versions are then/then_with and not and/and_then, which seems more consistent with established patterns in std?

[varkor]

There's a long discussion in the RFC about the naming. The current names are essentially placeholders.

[jplatte]

As a datapoint, this feature turns out pretty useful in proc macros, and I do think the current naming reads nicely: https://github.com/ruma/ruma-api/commit/cb00188e0371c544c4b8622d5a32731f2f6c54b3

[varkor]

@jplatte: that would be helpful to share on the RFC!

[c410-f3r]

It's been sometime since the PR implementation. Is there anything preventing stabilization? If not, I can write a stabilization report for this feature.

[varkor]

@c410-f3r: the names were only decided upon quite recently. It was decided that we would wait a few versions before deciding to stabilise: https://github.com/rust-lang/rfcs/pull/2757#issuecomment-564683344.

[U007D]

Just came back to give this a test drive in a library I am working on. If we look at common current established patterns in std:

Option: map<U, F> whereF: FnOnce(T) -> U and and_then<U, F> whereF: FnOnce(T) -> Option<U>

Result: map<U, F> whereF: FnOnce(T) -> U and and_then<U, F> whereF: FnOnce(T) -> Option<U>

And looking at the current implementation of the bool_to_option feature:

bool: then_somewhereF: FnOnce() -> T

The inconsistency is pretty jarring/unexpected, to my mind. Specifically, 1) for consistency shouldn't bool's F: FnOnce() -> T be called map, and F: FnOnce() -> Option<T> be some then derivative (and_then, then_some, etc.)? 2) shouldn't bool_to_option also provide both an F: FnOnce() -> T and an F: FnOnce() -> Option<T> implementation?

This is not just hypothetical; I am working on a library with a type that has a two-step fallible constructor. The intent is to be able to write something along the lines of:

...
impl Foo {
    fn new() -> Option<Self> {
        step_1_check().and_then(|| step_2_check().map(|| Self { /* ... */ }))
    }
}

but with the current plan developers would still have to resort to writing a custom combinator or pull in an external crate like boolinator for this simple case.

Thoughts?

[RustyYato]

You can call bool::then followed by Option::flatten to get the same effect. But it does make sense to have a fallible version of bool::then

[jplatte]

The inconsistency is pretty jarring/unexpected, to my mind.

@U007D: Your comparison is between bool → Option<T> conversion functions and Option<T> → Option<U> (+ Result<T, E> → Result<U, E>) conversion functions. Personally I find that given the input and output types are completely distinct in the new functions when they're not in the ones you're comparing them with is a good reason to consider different names.

[Boscop]

@U007D I completely agree. It was already tripping me up many times when using the boolinator crate, which also has inconsistent naming!

IMO these functions should be named consistently with Option/Result. And not just map and and_then, but it would also make sense to have and, or and or_else.

Or (if that's too much to include into std): Just have a method on bool that turns a bool into an option fn opt(self) -> Option<()>; (should have a short name), so that we can then use all the above option methods: b.opt().map(|_| 42), b.opt().and_then(|_| Some(42)), b.opt().and(Some(42)), b.opt().or(Some(42)), b.opt().or_else(|| Some(42)). Although I'd also prefer to have these as direct methods on bool (or maybe we can let bool be borrowable as &Option<()>, so that we could call Option's methods on a bool?). But please use consistent naming!

Currently, according to the docs, b.then_some(..) corresponds to b.opt().map(..) whereas b.then(..) corresponds to b.opt().and_then(..), which is not consistent at all.

[djc]

How is the libs team feeling about this? Just ran into another case where I'd have liked to use this (on stable).

[djc]

@rustbot ping libs

[rustbot]

Error: Only Rust team members can ping teams.

Please let @rust-lang/release know if you're having trouble with this bot.

[varkor]

cc @rust-lang/libs regarding naming and stabilisation: https://github.com/rust-lang/rust/issues/64260#issuecomment-596187828.

[SimonSapin]

@varkor Did you mean to link some other comment? This one does not discuss naming.

I honestly don’t remember the current status of this API.

The current names are essentially placeholders.

Is this still the case or has some consensus been found since?

[varkor]

@SimonSapin: sorry, I should have also linked to the comments above. I think the status is that, modulo outstanding concerns from some users about the naming, these methods are probably suitable for stabilisation.

Is this still the case or has some consensus been found since?

The current names were suggested by @dtolnay. I'm personally happy with the existing names, but there have been some complaints: https://github.com/rust-lang/rust/issues/64260#issuecomment-578509401, https://github.com/rust-lang/rust/issues/64260#issuecomment-579997086. However, I think considering how much previous discussion we had on these names, without coming to a clear consensus, I think it's probably worth stabilising with the existing names, so these methods can be used.

Perhaps starting an FCP would be the best way forward with these methods now: if any libs team members decide there are better alternatives, these could be raised during the FCP — otherwise, it would be good to be able to use these on stable.

[SimonSapin]

As a reminder, this issue currently tracks:

impl bool {
    pub fn then_some<T>(self, t: T) -> Option<T> {…}
    pub fn then<T, F: FnOnce() -> T>(self, f: F) -> Option<T> {…}
}

I can live with this, and more time likely won’t bring new information or ideas at this point.

@rfcbot fcp merge

[rfcbot]

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

• [x] @Amanieu
• [x] @KodrAus
• [x] @SimonSapin
• [x] @dtolnay
• [x] @sfackler
• [ ] @withoutboats

Concerns:

• name (https://github.com/rust-lang/rust/issues/64260#issuecomment-608401756)

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]

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

[Boscop]

IMO if we only have these 2 methods, there should definitely be another method to turn a bool into an Option<()> (shorter than b.then_some(())). I.E.:

impl bool {
    pub fn opt(self) -> Option<()> {...}
}

This corresponds to Boolinator::as_option. If we only have then_some (aka Boolinator::as_some) and then (aka Boolinator::as_some_from), then boolinator's b.and_option(opt) can be expressed as opt.filter(|_| b) but we have no equivalent to Boolinator::and_option_from:

fn and_option_from<T, F: FnOnce() -> Option<T>>(self, f: F) -> Option<T> 

If we had the opt method above, we could express this as b.opt().and_then(|_| f()). Without this opt method, we'd have to write b.then_some(()).and_then(|_| f()) which is much less readable, so it'd be less ergonomic than boolinator :/

[dtolnay]

@Boscop, be aware that we are not aiming to make boolinator obsolete. It isn't a goal of this issue or in general to absorb everything in boolinator into the standard library, or to expose at least as concise standard library equivalents of everything that boolinator exposes. In general external libraries get to cater to quite different tradeoffs than a standard library can (on many dimensions: nicheness, quirkiness, etc) so those people who have been happy with boolinator should go on using boolinator.

[voidc]

How about also adding a method with bool and T switched?

trait TakeIf {
    fn take_if(self, b: bool) -> Option<Self> where Self: Sized;
}

impl<T> TakeIf for T {
    fn take_if(self, b: bool) -> Option<Self> where Self: Sized {
        b.then_some(self)
    }
}

This would improve the ergonomics of using this feature in method chains.

[SimonSapin]

@voidc The bar for adding a method to every single type ever should be very high. This is rather niche functionality, so I feel it doesn’t deserve to be in the prelude. If importing a trait is required, this might negate the convenience you would find in the switched order.

[withoutboats]

@rfcbot concern name

I do feel like users (including myself) may have trouble remembering these APIs because in contrast to and/and_then and or/or_else, the long form is the non-closure form. However, I'm not clear what the alternative would be.

Assuming that we are adding these two methods only, can anyone come up with a pair of names that seem fitting and appropriate for boolean methods, where the one that takes T by value is a substring of the one that takes a closure?

[djc]

Summarizing previous discussion from the RFC PR. They were originally called then() and then_with(), then to_option() and to_option_with() (both of these would satisfy your concern). @dtolnay proposed the current names in https://github.com/rust-lang/rfcs/pull/2757#issuecomment-544277420 and elaborated on the choice in https://github.com/rust-lang/rfcs/pull/2757#issuecomment-544581104.

Closely related combinators as summarized by @troiganto in https://github.com/rust-lang/rfcs/pull/2757#issuecomment-529151770:

x.ok();  // Result<T, E> → Option<T>
x.err(); // Result<T, E> → Option<E>

x.ok_or(y);            // Option<T> → Result<T, E>
x.ok_or_else(closure); // Option<T> → Result<T, E>

x.and(y); x.and_then(closure); // Result<T, E> → Result<U, E>
x.or(y); x.or_else(closure);   // Result<T, E> → Result<T, F>

x.and(y); x.and_then(closure); // Option<T> → Option<U>
x.or(y); x.or_else(closure);   // Option<T> → Option<T>

[Boscop]

@withoutboats The names from boolinator fulfill your criteria as well: as_some and as_some_from. Also adhering to the convention that *_from will create it from a given closure.

But I think the names then/then_with are even better (and there won't be confusion since we won't have something like and_then for bool).

[withoutboats]

Thanks for linking to the previous discussion @djc. Lookin at the code I found it actually fairly intuitive in practice and I'm not as concerned about remembering which is correct. I think in practice, the notion of "then " and "then_some " is good to grok.

But I don't really see a strong argument made in the old thread between then/then_with and then_some/then. I strongly agree with @dtolnay's argument about names like to_option, but is there a reason to prefer then_some/then over then/then_with?

[jplatte]

Personally, I like the shorter variant being the one that takes a closure, as when I tried out the feature it was the more common one by a large margin (I'm not even sure I had any .then_somes in the end).

[withoutboats]

@jplatte Yea, that seems like what I would expect as well. I think of this as replacing this common and annoying pattern:

if foobar {
   // code block evaluationg to Option<T>
} else {
   None
}

Rarely is the if case just literally Some(thing), it almost always involves running some sort of code, no?

I wonder what people think of a third option: only adding then and expecting people to write foobar.then(|| Some(thing)) instead of foobar.then_some(thing)?

[varkor]

Rarely is the if case just literally Some(thing), it almost always involves running some sort of code, no?

It my experience, it's frequently just Some(thing). You can see this in rustc for instance, although that's certainly an under-approximation. I think both methods are convenient in practice.

[djc]

But we can do then(|| thing) without the Some() right? At least the RFC seems to mention that the closure parameter F would just return T directly.

I do think that the closure form would be quite a bit more likely, and the syntactic overhead of || here seems very limited. It does cause some asymmetry with other pairs of methods. What is the history of Option::unwrap_or() vs Option::unwrap_or_else()? If the latter had been there first, would we still have the shorter version?

Also, is there a performance impact of the extra method call, or will LLVM know to inline it basically always in the common case?

[dtolnay]

I agree with https://github.com/rust-lang/rust/issues/64260#issuecomment-609033148 that both forms are frequently applicable in my code. I would end up using then_some(thing) wherever applicable because it reads off more clearly than then(|| thing) -- it means if ... then Some(thing). I think then is better fit to blocks of code where it means "if ... then do the following".

is_true.then(|| {
    ...
    ...
})

[Boscop]

So we cannot call then if the closure returns an Option. What's the suggested way to shorten this then?

if cond {
    f()
} else {
    None
} 

This pattern also occurs often.

[dtolnay]

cond.then(f).flatten()

[U007D]

I find then and then_some difficult to remember, because they do not follow the patterns of existing similar combinators, as @djc illustrated above. Further, I find their current names to be named backward--when programming, I tend to reach for the lazily-evaluated version as opposed to the eager one. Everywhere else in std, the shorter name is eager, and the name with the suffix (*_then, *_with, etc.) is lazy. Unfortunately the bool ones are currently reversed.

I prefer names that require low cognitive load e.g. to_option/to_option_with. Failing that, providing the extended suffix name to the lazy version e.g. and/and_then (or then/then_with if we must) would at least be consistent with the rest of std.

Thanks for reading; I know this can be draining with everyone having a strong opinion, but names do matter--we'll be living with the choices we make for a long time.

[dtolnay]

I am going to lock this for a while because it doesn't seem like new arguments or new ideas are still being introduced.

There's a good chance we'll remove these methods; as described in https://github.com/rust-lang/rfcs/pull/2757#issuecomment-552343795, they were landed unstable to find out whether we might settle on sufficiently clear names for the behavior based on actual use, which I guess hasn't happened.

[varkor]

I am going to lock this for a while because it doesn't seem like new arguments or new ideas are still being introduced.

While I appreciate that comments reiterating previous points are not helpful, locking the thread is a very effective way to ensure that no new arguments or ideas can be presented.

There's a good chance we'll remove these methods; as described in rust-lang/rfcs#2757 (comment), they were landed unstable to find out whether we might settle on sufficiently clear names

While it seems increasingly clear that there do not exist names that makes everyone happy, I think it's also plain to see that these methods are desirable to have in the standard library. As a case in point, many of the existing combinators have entirely unenlightening names, but are nevertheless useful; though I still forget some of the combinators, I am happier to have to check every so often, than to have to rely on a library or my own implementation for such a common idiom. The same is true of the methods here.

I would imagine that even those with objections to the current names would still prefer to have the methods as-is than not at all (though this would be a little unfair to claim when they do not have the chance to respond).

[LukasKalbertodt]

I agree with @varkor and don't think locking this thread really helps the discussion. There have been threads way worse than this in Rust's history ;-) I will just go ahead and unlock this thread again. If the majority of the libs or moderation team is in favor of locking, just undo that.

---

On the topic: I truly appreciate you (@dtolnay) expressing the often unpopular opinion of removing a feature instead of stabilizing. And I even agree that a feature which is mainly added for convenience and brevity of code shouldn't be added if it makes code notably less readable. However, in this case, I think we gain more by having the method than we lose by the non-perfect name.

In your example, we replace a few perfectly clear lines with one line that the reader has to briefly think about. Removing lines is not just about less typing, but also about removing boilerplate code that does not carry information. Lots of boilerplate code also makes functions hard to understand and exhausting to read. So in my book, using the new method is not worse than the old code.

[dtolnay]

Locking is valuable as a triage tool -- if a feature is relatively low value but looks to have disproportionately high cost to reach a consensus on, there are going to be more productive uses of time. Higher value features than this get to have proportionately more intense discussion.

I guess I would just remind participants that repeating previous comments without adding new information is not useful. You would need to acknowledge (ideally link) and address some of the counterpoints that have already been made against your opinion in a way that hasn't already been discussed.

[Boscop]

I would imagine that even those with objections to the current names would still prefer to have the methods as-is than not at all (though this would be a little unfair to claim when they do not have the chance to respond).

I totally agree. I'd rather have these methods as-is in std than not have them. In fact, after re-reading this thread and using these methods in my own code, I've now come around to liking these names and got used to them. So yeah, please add them :)

[withoutboats]

There's a good chance we'll remove these methods; as described in rust-lang/rfcs#2757 (comment), they were landed unstable to find out whether we might settle on sufficiently clear names for the behavior based on actual use, which I guess hasn't happened.

I wouldn't want that to be the outcome.

I actually think this is a relatively high impact change - I suspect there's a good chance that using this API will become a very common pattern, part of the essential toolbox of methods all users are expected to know. That's why I think quibbling over these very small differences is worthwhile.

I see three outcomes that are, in my mind, "on the table:"

1. Stabilize as is: then_some/then
2. Change the names to then/then_with
3. Only stabilize then

3 is forward compatible with 1, otherwise these are incompatible. So we need to make a decision.

I agree with @Boscop's comment that seeing these in actual code made option 1 much more appealing than hearing it in the abstract - both names are rather clear about what they do.

I feel uncertain that then_some pulls it weight, though. Here are the downsides, to me, of adding then_some:

1. It is another method in this core toolbox of methods that everyone is expected to learn. We've very rarely added methods to this toolbox since 1.0, and among those we have added, many I think we should've been more cautious about (like Option::xor).
2. It breaks the pairing of short:value/long:closure that the pair methods on Option and Result have. I think that's been a big boost for making learning these methods easier, because they pair off in this consistent way.

(In fact, then alone would boost the effect of these pairings by tightly linking the idea of then to the idea of taking a closure in the std APIs.)

I agree that is_foo.then_some(x) is a bit clearer than is_foo.then(|| x), but I'm not completely convinced that outweighs these downsides. What do you think, @dtolnay, about only stabilizing then and revisiting later if it feels like the pattern then_some supports needs this further clarity enough to add this method?

[dtolnay]

I would be on board with stabilizing then and deferring until later whether to remove or stabilize then_some.

[pnkfelix]

I already accidentally wrote a note on the related RFC (https://github.com/rust-lang/rfcs/pull/2757#issuecomment-614126309); I just wanted to chime in saying that I have independently encountered the scenario outlined by @U007D up above, and that indicates to me that we should consider making bool::then take an FnOnce() -> Option<T> instead of an FnOnce() -> T.

In my aforementioned note, I suggested that both bool::then and bool::then_some should take closures (and the difference is just that then_some takes an FnOnce() -> T).

(I did see the response pointing out that one can use Option::flatten to get around the issue of passing an FnOnce -> Option<T> into the current bool::then. I haven't made up my mind yet about whether that would be a reasonable solution in most cases; for the one that I was facing most recently, it made me abandon using these combinators entirely.)

[varkor]

that indicates to me that we should consider making bool::then take an FnOnce() -> Option instead of an FnOnce() -> T.

It indicates to me that, if this is such a common scenario, we ought to have a third method to avoid .flatten(). However, part of the motivation for these combinators is to avoid having to write Some everywhere, which would no longer be the case if the combinators return Option<T>.

[Boscop]

It indicates to me that, if this is such a common scenario, we ought to have a third method to avoid .flatten().

I think it makes sense to add this third method. It could be called and_then.

[traviscross]

Speaking as someone who trips over the absence of this feature somewhat frequently, I'd be happy to have this under any reasonably concise name. But I agree with @withoutboats that this will be an important idiom in the language, so we should get it right.

Also agreeing with @withoutboats, I could live a happy life without .then_some. Writing the closure is fine as our closure syntax is concise. (Even better, not having the method means clippy won't warn about using .then(|| x) instead of .then_some(x)).

However, as @varkor, @Boscop, and @pnkfelix have pointed out, it would be nice to have a method that does not implicitly wrap with Some. (I would run up against this in some code I'm working on today.) So if we want to add two methods and check our happiness with the consistency of those names, it seems those are the two that we should consider.

One (perhaps radical) option would be to add just one method, .then, and have it not do implicit wrapping. Personally, I could live with writing the explicit Some. Saying is_foo.then(|| Some(42)) is still infinitely better than today's idiomatic code. (Naming this .and_then would also be fine.)

That also solves the worst problem with using the name .then for a method that wraps its result with Some, which is that our most prominent other "then-like" method, .and_then, doesn't implicitly wrap.

We could always decide later, on the basis of experience, to add a method that does implicit Some-wrapping. (I'd suggest .map as the name of that method.)

[traviscross]

For concreteness, here's a proposal that maximizes parsimony with the rest of Rust:

fn and_then<U, F: FnOnce() -> Option<U>>(self, f: F) -> Option<U> {
    if self { f() } else { None }
}
fn map<U, F: FnOnce() -> U>(self, f: F) -> Option<U> {
    self.and_then(|| Some(f()))
}

Play with them here: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=259b06d354e96e9e29fb883e25c98215

The way that I justify these names to myself is that they first implicitly lift the bool into the Option monad. The only other reasonable behavior for a .map method would be to map bool -> bool, and it just doesn't seem likely that we'd ever add that (cf. is_foo.and_then(|| None).is_some()).

[traviscross]

Thinking about this further, if there were a method named then, here's what I would expect it to do:

fn then<U, F: FnOnce(Self) -> U>(self, f: F) -> U {
    f(self)
}

E.g. is_foo.then(|x| if x { Ok(42) } else { Err(()) }). I.e., it would allow arbitrary logic within a method chain, similar to expression-oriented keywords (e.g. is_foo.if {...} else {...}). Intuitively, and_then calls its argument only on a true/present/success value, so then should call its argument on all values.

Note that I'm not proposing this method here for inclusion. It's just what I would expect a then method on a Rust type to do given other method names in the language. It's one more reason why and_then is probably the right method name.

[zakarumych]

For the case mentioned above:

impl Foo {
    fn new() -> Option<Self> {
        step_1_check().and_then(|| step_2_check().map(|| Self { /* ... */ }))
    }
}

wouldn't it look nice to write this instead

impl Foo {
    fn new() -> Option<Self> {
        step_1_check()?;
        step_2_check()?;
        Self { /* ... */ }.into()
    }
}

Which would be possible by implementing std::ops::Try for bool

[jdahlstrom]

If it's too difficult to come up with good names for these methods, couldn't we just have a bool -> Option<()> conversion function as a first step at least? The two types are famously isomorphic (and even have identical representation), so something like bool::as_option shouldn't be too far-fetched. With one method call you get access to everything Option has to offer.