Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implemented map_or_else for Result<T, E> #53777

Merged
merged 6 commits into from
Sep 12, 2018
Merged

Implemented map_or_else for Result<T, E> #53777

merged 6 commits into from
Sep 12, 2018

Conversation

ivanbakel
Copy link
Contributor

Fulfills #53268
The example is ripped from Option::map_or_else, with the types corrected.

@rust-highfive
Copy link
Collaborator

r? @aidanhs

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2018
@rust-highfive

This comment has been minimized.

Sign in to view
@frewsxcv frewsxcv added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Aug 29, 2018
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
killercup
killercup reviewed Aug 29, 2018
View reviewed changes
src/libcore/result.rs Outdated
/// assert_eq!(x.map_or_else(|e| k * 2, |v| v.len()), 42);
/// ```
#[inline]
#[stable(feature = "result_map_or_else", since = "1.30.0")]
Copy link
Member

@killercup killercup Aug 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@rust-lang/libs should probably sign this off before this is added as a stable feature (unless that happened and I missed it -- sorry)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing like that has happened, as far as I know - I just slapped a value in there to make it compile.

@ivanbakel
Corrected bad typing in Result::map_or_else doc
71b16d8 
The error value now includes the type of the success.
The success value now includes the type of the error.
@withoutboats
Copy link
Contributor

My understanding of libs team policy is this: this method should be marked as unstable, rather than stable, and a tracking issue created. Then, for small single method additions like this, it can be r+'d without an fcp process (the fcp will come when we decide to stabilize it).

@ivanbakel
Copy link
Contributor Author

Oh, good to know. Where should the tracking issue be created - is the current issue in rust-lang/rust enough? Is there a particular style or recommended value for the feature name?

@withoutboats
Copy link
Contributor

Yep, we create tracking issues on this repository. In fact, you can probably just make #53268 the tracking issue and we'll label it properly.

The unstable attribute takes an issue field, instead of a since field: you want to make that issue="53268", this way the compiler will report where to find the tracking issue for this feature.

You can chose the feature name: the one you've chosen seems fine.

@withoutboats withoutboats mentioned this pull request Aug 29, 2018
Closed
@ivanbakel
Corrected feature status of Result::map_or_else
3eda905 
map_or_else is now correctly labelled unstable and points to the
tracking issue on rust-lang/rust
@ivanbakel
Corrected type variable output T -> U in Result::map_or_else
6ff4f79
@rust-highfive

This comment has been minimized.

Sign in to view
@TimNN
Copy link
Contributor

TimNN commented Sep 11, 2018

Ping from triage @aidanhs / @rust-lang/libs: This PR requires your review.

@alexcrichton
Copy link
Member

@bors: r+

Thanks @ivanbakel and sorry for the delay!

@bors
Copy link
Contributor

bors commented Sep 11, 2018

📌 Commit 79408c3 has been approved by alexcrichton

@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 11, 2018
@kennytm
Copy link
Member

kennytm commented Sep 12, 2018

@bors rollup

kennytm added a commit to kennytm/rust that referenced this pull request Sep 12, 2018
@kennytm
Rollup merge of rust-lang#53777 - ivanbakel:result_map_or_else, r=ale…
4f62077 
…xcrichton

Implemented map_or_else for Result<T, E>

Fulfills rust-lang#53268
The example is ripped from `Option::map_or_else`, with the types corrected.
@kennytm kennytm mentioned this pull request Sep 12, 2018
Merged
bors added a commit that referenced this pull request Sep 12, 2018
@bors
Auto merge of #54146 - kennytm:rollup, r=kennytm
8586ec6 
Rollup of 15 pull requests

Successful merges:

 - #52514 (Fix a few AMDGPU related issues)
 - #53703 (Document .0 to unpack integer from Wrapping)
 - #53777 (Implemented map_or_else for Result<T, E>)
 - #54031 (A few cleanups and minor improvements to rustc_passes)
 - #54046 (Update documentation for fill_buf in std::io::BufRead)
 - #54064 (`&CStr`, not `CStr`, is the counterpart of `&str`)
 - #54072 (Stabilization change for mod.rs)
 - #54073 (docs: Use dollar sign for all bash prompts)
 - #54074 (simplify ordering for Kind)
 - #54085 (Remove documentation about proc_macro being bare-bones)
 - #54087 (rustdoc: Remove generated blanket impls from trait pages)
 - #54106 (Reexport CheckLintNameResult)
 - #54107 (Fix typos in libstd hash map)
 - #54136 (Update LLVM to fix GlobalISel dbg.declare)
 - #54142 (Recover proper regression test for issue #16278.)

Failed merges:

r? @ghost
@bors bors merged commit 79408c3 into rust-lang:master Sep 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@killercup killercup killercup left review comments

Assignees

@aidanhs aidanhs

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@ivanbakel @rust-highfive @withoutboats @TimNN @alexcrichton @bors @kennytm @killercup @frewsxcv @aidanhs