Skip to content

Conversation

RalfJung
Copy link
Member

In #69839 we turned a closure into a try block, but it turns out that does not work with our throw_ macros, which return so they skip the try.

Here we fix that. For some reason that means we also have to remove some ;.

r? @oli-obk

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 19, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Mar 19, 2020

wtf... how does

fn foo() -> Result<String, ()> {
    Err(())?
}

compile?! I can only guess that it defaults the Ok type of the Result to !, there's no other explanation I can come up with. (Also Result::<String, ()>::Err(())? errors).

@oli-obk
Copy link
Contributor

oli-obk commented Mar 19, 2020

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 19, 2020

📌 Commit a39875b has been approved by oli-obk

@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 19, 2020
@RalfJung
Copy link
Member Author

@bors r-
Waiting until tmr to make sure we get a nightly with Miri.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 19, 2020
@tspiteri
Copy link
Contributor

tspiteri commented Mar 19, 2020

@oli-obk wrote:

how does [Err(())?] compile?!

Result::<Result<String, ()>, ()>::Err(())? works, so I'm guessing it infers that the unwrapped Ok variant would have the type of the return value.

@RalfJung
Copy link
Member Author

@bors r=oli-obk

@bors
Copy link
Collaborator

bors commented Mar 20, 2020

📌 Commit a39875b has been approved by oli-obk

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 20, 2020
@goffrie
Copy link
Contributor

goffrie commented Mar 20, 2020

Here we fix that. For some reason that means we also have to remove some ;.

If you change the macro to Err::<!, _>(...)? then the compiler will be able to see that it doesn't return.

@RalfJung
Copy link
Member Author

@goffrie ah, that makes sense! I added that type annotation, and checked that then I could re-add the semicolons.

However, I kept the part that removes them here, matching (I think) the usual Rust policy for trailing expressions.

@bors r=oli-obk

@bors
Copy link
Collaborator

bors commented Mar 20, 2020

📌 Commit e46b3c2 has been approved by oli-obk

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 21, 2020
Rollup of 8 pull requests

Successful merges:

 - rust-lang#67888 (Prefetch some queries used by the metadata encoder)
 - rust-lang#69934 (Update the mir inline costs)
 - rust-lang#69965 (Refactorings to get rid of rustc_codegen_utils)
 - rust-lang#70054 (Build dist-android with --enable-profiler)
 - rust-lang#70089 (rustc_infer: remove InferCtxt::closure_sig as the FnSig is always shallowly known.)
 - rust-lang#70092 (hir: replace "items" terminology with "nodes" where appropriate.)
 - rust-lang#70138 (do not 'return' in 'throw_' macros)
 - rust-lang#70151 (Update stdarch submodule)

Failed merges:

 - rust-lang#70074 (Expand: nix all fatal errors)

r? @ghost
@bors bors merged commit 16f607f into rust-lang:master Mar 21, 2020
@RalfJung RalfJung deleted the throw-not-return branch March 21, 2020 11:45
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.

6 participants