Skip to content

Conversation

Adam-Gleave
Copy link

I encountered this earlier today and thought maybe bool::then_some could use a little addition to the documentation.

It's pretty obvious with familiarity and from looking at the implementation, but the argument for then_some is eagerly evaluated, which means if you do the following (as I did), you can have a problem:

// Oops!
let _ = something
    .has_another_thing()
    .then_some(something.another_thing_or_panic());

A note, similar to other methods with eagerly-evaluated arguments and a lazy alternative (Option::or, for example), could help point this out to people who forget (like me)!

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Sep 1, 2022
@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @scottmcm (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 1, 2022
@scottmcm
Copy link
Member

scottmcm commented Sep 6, 2022

This looks like essentially the same phrasing as is used on https://doc.rust-lang.org/std/option/enum.Option.html#method.map_or, so it makes sense to me. Thanks!

@bors r+ rollup

@Adam-Gleave Consider also suggesting a clippy lint for this, like there is for https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call

@bors
Copy link
Collaborator

bors commented Sep 6, 2022

📌 Commit 33afe97 has been approved by scottmcm

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 6, 2022
@Adam-Gleave
Copy link
Author

This looks like essentially the same phrasing as is used on https://doc.rust-lang.org/std/option/enum.Option.html#method.map_or, so it makes sense to me. Thanks!

@bors r+ rollup

@Adam-Gleave Consider also suggesting a clippy lint for this, like there is for https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call

Brilliant, thanks!

Good idea, a clippy lint would be nice to have.

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 6, 2022
Rollup of 5 pull requests

Successful merges:

 - rust-lang#100658 (TyCtxt::get_attr should check that no duplicates are allowed)
 - rust-lang#101021 (Migrate ``rustc_middle`` diagnostic)
 - rust-lang#101287 (Document eager evaluation of `bool::then_some` argument)
 - rust-lang#101412 (Some more cleanup in `core`)
 - rust-lang#101427 (Fix ICE, generalize 'move generics to trait' suggestion for >0 non-rcvr arguments)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 24f9989 into rust-lang:master Sep 6, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants