Skip to content

Conversation

fpervaiz
Copy link

@fpervaiz fpervaiz commented Mar 5, 2025

Adds Result::map_or_default and Option::map_or_default as discussed extensively at rust-lang/rfcs#3148

@rustbot
Copy link
Collaborator

rustbot commented Mar 5, 2025

r? @Amanieu

rustbot has assigned @Amanieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 5, 2025
@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@Sky9x Sky9x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may need rfc approval or an ACP
also needs a tracking issue

implementation LGTM

Comment on lines +833 to +835
/// Maps a `Result<T, E>` to a `U` by applying function `f` if the contained value
/// is [`Ok`], otherwise if [`Err`], returns the [default value] for the type `U`.
///
Copy link
Contributor

@tkr-sh tkr-sh Apr 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the conversion of the previous RFC into a tracking issue! ❤️


Just, for the documentation, I think that something like:

+ by applying function `f` to the contained value if the result is [`Ok`]
- by applying function `f` if the contained is [`Ok`]

( or something like that ) would be better because, with the previous sentence, you don't know if f takes Result<T, E>/Option<T> or T. (and same for Option)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please apply this suggestion too.

@rustbot rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 16, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 16, 2025

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git pull --rebase https://github.com/rust-lang/rust.git master
$ git push --force-with-lease

The following commits are merge commits:

@Amanieu Amanieu added I-libs-api-nominated Nominated for discussion during a libs-api team meeting. and removed I-libs-api-nominated Nominated for discussion during a libs-api team meeting. labels May 3, 2025
@Amanieu
Copy link
Member

Amanieu commented May 3, 2025

@fpervaiz Could you please do a rebase to remove merge commits?

I will approve the PR once this is done.

@tkr-sh
Copy link
Contributor

tkr-sh commented May 3, 2025

@Amanieu what about #138068 ?

@tkr-sh
Copy link
Contributor

tkr-sh commented May 13, 2025

@fpervaiz Could you either do the merge change and #138068 (comment) since the RFC has been accepted or give right of writing to other people on your PR ? Thanks :) !

@tkr-sh
Copy link
Contributor

tkr-sh commented May 24, 2025

@Amanieu Do you have the power to make changes to the PR ?
What do we do if the author of the PR doesn't answer anymore ?

@Amanieu
Copy link
Member

Amanieu commented May 27, 2025

@tkr-sh Since the author is not responding, you can open a separate PR and I can merge that one. I will then close this one.

@bors bors closed this in da61494 May 28, 2025
rust-timer added a commit that referenced this pull request May 28, 2025
Rollup merge of #141659 - tkr-sh:map-or-default, r=Amanieu

Add `Result::map_or_default` and `Option::map_or_default`

Closes: #138068

_This PR has been recreated because of the inactivity of the author (Cf. #138068 (comment)
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request May 28, 2025
Add `Result::map_or_default` and `Option::map_or_default`

Closes: rust-lang/rust#138068

_This PR has been recreated because of the inactivity of the author (Cf. rust-lang/rust#138068 (comment)
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request May 30, 2025
Add `Result::map_or_default` and `Option::map_or_default`

Closes: rust-lang#138068

_This PR has been recreated because of the inactivity of the author (Cf. rust-lang#138068 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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.

7 participants