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.

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

Add Result<Result<T, E>, E>::flatten -> Result<T, E> #70140

Merged
merged 1 commit into from
Mar 29, 2020

Conversation

Nemo157
Copy link
Member

@Nemo157 Nemo157 commented Mar 19, 2020

This PR makes this possible (modulo type inference):

assert_eq!(Ok(6), Ok(Ok(6)).flatten());

Tracking issue: #70142

largely cribbed directly from #60256

@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(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 Mar 19, 2020
@rust-highfive
Copy link
Collaborator

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-03-19T09:10:55.6594288Z ========================== Starting Command Output ===========================
2020-03-19T09:10:55.6599106Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/624fb5ef-bd23-4005-879d-83bf86977f01.sh
2020-03-19T09:10:55.6599806Z 
2020-03-19T09:10:55.6603741Z ##[section]Finishing: Disable git automatic line ending conversion
2020-03-19T09:10:55.6621082Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/70140/merge to s
2020-03-19T09:10:55.6623919Z Task         : Get sources
2020-03-19T09:10:55.6624172Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-03-19T09:10:55.6624415Z Version      : 1.0.0
2020-03-19T09:10:55.6624580Z Author       : Microsoft
---
2020-03-19T09:10:56.6504683Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-03-19T09:10:56.6510147Z ##[command]git config gc.auto 0
2020-03-19T09:10:56.6513711Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-03-19T09:10:56.6517385Z ##[command]git config --get-all http.proxy
2020-03-19T09:10:56.6524046Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/70140/merge:refs/remotes/pull/70140/merge
---
2020-03-19T09:16:02.3129320Z     Checking core v0.0.0 (/checkout/src/libcore)
2020-03-19T09:16:05.8626653Z error[E0545]: `issue` must be a non-zero numeric string or "none"
2020-03-19T09:16:05.8628026Z     --> src/libcore/result.rs:1243:48
2020-03-19T09:16:05.8628944Z      |
2020-03-19T09:16:05.8630345Z 1243 |      #[unstable(feature = "result_flattening", issue = "0")]
2020-03-19T09:16:05.8631986Z      |                                                        |
2020-03-19T09:16:05.8631986Z      |                                                        |
2020-03-19T09:16:05.8632800Z      |                                                        `issue` must not be "0", use "none" instead
2020-03-19T09:16:09.9538076Z    Compiling libc v0.2.66
2020-03-19T09:16:10.8687187Z    Compiling autocfg v0.1.7
2020-03-19T09:16:12.1470056Z error: aborting due to previous error
2020-03-19T09:16:12.1470434Z 
---
2020-03-19T09:16:12.6248172Z   local time: Thu Mar 19 09:16:12 UTC 2020
2020-03-19T09:16:13.1604900Z   network time: Thu, 19 Mar 2020 09:16:13 GMT
2020-03-19T09:16:13.1608705Z == end clock drift check ==
2020-03-19T09:16:17.9386695Z 
2020-03-19T09:16:17.9454879Z ##[error]Bash exited with code '1'.
2020-03-19T09:16:17.9471404Z ##[section]Finishing: Run build
2020-03-19T09:16:17.9521878Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/70140/merge to s
2020-03-19T09:16:17.9526171Z Task         : Get sources
2020-03-19T09:16:17.9526496Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-03-19T09:16:17.9526797Z Version      : 1.0.0
2020-03-19T09:16:17.9526996Z Author       : Microsoft
2020-03-19T09:16:17.9526996Z Author       : Microsoft
2020-03-19T09:16:17.9527324Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-03-19T09:16:17.9527852Z ==============================================================================
2020-03-19T09:16:18.2839146Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-03-19T09:16:18.2883229Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/70140/merge to s
2020-03-19T09:16:18.2974657Z Cleaning up task key
2020-03-19T09:16:18.2975746Z Start cleaning up orphan processes.
2020-03-19T09:16:18.3265403Z Terminate orphan process: pid (4450) (python)
2020-03-19T09:16:18.3288359Z ##[section]Finishing: Finalize Job

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @rust-lang/infra. (Feature Requests)

@@ -1214,6 +1214,38 @@ impl<T, E> Result<Option<T>, E> {
}
}

impl<T, E> Result<Result<T, E>, E> {
/// Converts from `Result<Result<T, E>, E>` to `Result<T, E>`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the documentation could be reworded a bit, to emphasize what happens if one of the results is an Err variant (which wasn't really needed for Option<T>::flatten). It should be noted that, in Rustdoc, you don't directly see the type of self when looking at the definition of a method.

@CryZe
Copy link
Contributor

CryZe commented Mar 23, 2020

Should this possibly also convert the error type like the try operator does?

@Dylan-DPC-zz
Copy link

r? @Amanieu

@Amanieu
Copy link
Member

Amanieu commented Mar 29, 2020

@Nemo157 Could you please address @CryZe's feedback:

Should this possibly also convert the error type like the try operator does?

@Centril
Copy link
Contributor

Centril commented Mar 29, 2020

It seems strange to me that an operation named flatten (implying monadic join) should do such implicit conversions, as Result<T, E> and Result<T, F> are not "the same monads". The equivalent for Option, Option::flatten doesn't, while it ostensibly could.

@Amanieu
Copy link
Member

Amanieu commented Mar 29, 2020

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 29, 2020

📌 Commit 6fe7867 has been approved by Amanieu

@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 Mar 29, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 29, 2020
Rollup of 3 pull requests

Successful merges:

 - rust-lang#70140 (Add Result<Result<T, E>, E>::flatten -> Result<T, E>)
 - rust-lang#70526 (reduce `rustc_attr` usage in places)
 - rust-lang#70527 (Update LLVM submodule)

Failed merges:

r? @ghost
@bors bors merged commit 8212a1c into rust-lang:master Mar 29, 2020
@Nemo157 Nemo157 deleted the result-flatten branch October 12, 2020 11:52
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants