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

catch lacking both type annotation and trailing expr yields confusing diagnostic #44886

Closed
pnkfelix opened this issue Sep 27, 2017 · 2 comments
Closed
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. F-try_blocks `#![feature(try_blocks)]`

Comments

@pnkfelix
Copy link
Member

If you:

  1. make a catch block, and
  2. do not realize that essentially all catch blocks have to end in some sort of trailing expression of type Result<.., ..>, and
  3. you also leave out the type annotation stating that the return type should indeed by Result<.., ..>,

then you are likely to end up getting an error diagnostic from the compiler that is pretty confusing.

Consider the following example:

#![feature(catch_expr)]

pub fn f() -> Result<i32, i32> {
    let collapse = do catch {
        g()?;
        h()?;
        // Ok(()) // (error message becomes usable if you uncomment this.)
    };
    Ok(3)
}

fn g() -> Result<(), i32> { Err(0) }

fn h() -> Result<(), i32> { Err(1) }

fn main() { f().unwrap(); }

From today's nightly, this produces the following diagnostic output (playground):

   Compiling playground v0.0.1 (file:///playground)
error[E0277]: the `?` operator can only be used in a function that returns `Result` (or another type that implements `std::ops::Try`)
 --> src/main.rs:6:9
  |
6 |         h()?;
  |         ----
  |         |
  |         cannot use the `?` operator in a function that returns `()`
  |         in this macro invocation
  |
  = help: the trait `std::ops::Try` is not implemented for `()`
  = note: required by `std::ops::Try::from_error`

error[E0277]: the `?` operator can only be used in a function that returns `Result` (or another type that implements `std::ops::Try`)
 --> src/main.rs:5:9
  |
5 |         g()?;
  |         ----
  |         |
  |         cannot use the `?` operator in a function that returns `()`
  |         in this macro invocation
  |
  = help: the trait `std::ops::Try` is not implemented for `()`
  = note: required by `std::ops::Try::from_error`

error: aborting due to 2 previous errors

error: Could not compile `playground`.

Reading that is pretty frustrating: it talks about ? only being usable in a function that returns Result, but this function does return Result.

(The error message is much improved if one actually provides a trailing expression of Result type in the catch-block; the worst-case scenario then is it just tells you to provide an explicit type annotation for the catch-block itself.)


I assume this issue is arising because the type inference system is first using the lack of a trailing expression to infer that the type of the block's contet is (). Then, since we do not auto-wrap catch-blocks with Ok (see #41414), it concludes that the type of the catch expression must be () (even though that is a nonsensical type for a catch expression...). And then it uses that expected type (()) as the basis for its complaints about the uses of the ?-operator.


So, an idea:

Assuming we are planning to neither (1.) implement std::ops::Try for () nor (2.) Ok-wrap the trailing expression of catch-blocks, then I would suggest that we force catch-blocks to always have some trailing expression. That wouldn't fix every instance of this problem, but its a pretty simple change and I cannot imagine any downside (since, the way things stand, all catch-blocks that can reach their end always need an explicit trailing expression to compile)

(To handle the more general case, we may want to consider changing how type inference is handled for catch-blocks. But to be honest, given how general-purpose catch/? are since they are overloaded via the Try trait, I am not sure whether there is a good strategy for handling the general case here.)

@aidanhs aidanhs added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Sep 28, 2017
@scottmcm
Copy link
Member

The current form of this is https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=bff0588c06e779b1410554eb3b4b4e43

#![feature(try_blocks)]

pub fn f() -> Result<i32, i32> {
    let collapse = try {
        g()?;
        h()?;
    };
    Ok(3)
}

fn g() -> Result<(), i32> { Err(0) }

fn h() -> Result<(), i32> { Err(1) }

fn main() { f().unwrap(); }

Because it's spelled try now, and it always ok-wraps the result (#41414).

It looks like the warning for this is now pretty good:

error[E0282]: type annotations needed
 --> src/main.rs:4:9
  |
4 |     let collapse = try {
  |         ^^^^^^^^
  |
help: consider giving `collapse` an explicit type
  |
4 |     let collapse: /* Type */ = try {
  |                 ++++++++++++

For more information about this error, try `rustc --explain E0282`.

So I'll close this.

@fmease fmease added F-try_blocks `#![feature(try_blocks)]` and removed A-catch labels Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. F-try_blocks `#![feature(try_blocks)]`
Projects
None yet
Development

No branches or pull requests

6 participants
@pnkfelix @aidanhs @Mark-Simulacrum @fmease @scottmcm and others