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

Async blocks are leaky about wording wrt. ? in diagnostics #62570

Closed
Centril opened this issue Jul 10, 2019 · 9 comments · Fixed by #65902
Closed

Async blocks are leaky about wording wrt. ? in diagnostics #62570

Centril opened this issue Jul 10, 2019 · 9 comments · Fixed by #65902
Assignees
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints AsyncAwait-Polish Async-await issues that are part of the "polish" area AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. F-on_unimplemented Error messages that can be tackled with `#[rustc_on_unimplemented]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented Jul 10, 2019

E.g. consider:

#![feature(async_await)]

fn foo() {
    let x = async move {
        Err(())?;

        1
    };
}

which yields the diagnostic:

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

Ostensibly the user might think of fn foo as "the function" but it is in fact the async block that is "the function" here. That seems like something which would confuse users who are not familiar with the lowering and whatnot (most users).

@Centril Centril added A-diagnostics Area: Messages for errors, warnings, and lints A-async-await Area: Async & Await AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 10, 2019
@theduke
Copy link
Contributor

theduke commented Jul 11, 2019

I ran into this.

It's much more confusing if the outer function actually returns a result.

In a more complicated function the error message can be a real head scratcher.

#![feature(async_await)]

async fn f() -> Result<bool, ()> {
    async {
        Err(())?
    }.await;
    
    Ok(true)
}

@estebank estebank added the F-on_unimplemented Error messages that can be tackled with `#[rustc_on_unimplemented]` label Aug 4, 2019
@estebank
Copy link
Contributor

This can be fixed as a rewording to "cannot use the ? operator in a function or async block that returns {integer}".

@cramertj
Copy link
Member

This same error message appears for closures:

fn main() {
    let _ = || {
        Err(5)?;
        1
    };
}
error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `std::ops::Try`)
 --> src/main.rs:3:9
  |
3 |         Err(5)?;
  |         ^^^^^^^ cannot use the `?` operator in a function that returns `{integer}`
  |
  = help: the trait `std::ops::Try` is not implemented for `{integer}`
  = note: required by `std::ops::Try::from_error`

@estebank
Copy link
Contributor

At this point there are enough cases that we would want to disambiguate between that it makes sense to extend on_unimplemented to have some way of obtaining the enclosing scope's description (closure/function/method/async block, etc.).

@Centril
Copy link
Contributor Author

Centril commented Oct 8, 2019

I think something like:

#[rustc_on_unimplemented(
   on(all(
       any(from_method="from_error", from_method="from_ok"),
       from_desugaring="QuestionMark"),
      message="the `?` operator can only be used in {ItemCtx} \
               that returns `Result` or `Option` \
               (or another type that implements `{Try}`)",
      label="cannot use the `?` operator in {ItemCtx} that returns `{Self}`"),
   on(all(from_method="into_result", from_desugaring="QuestionMark"),
      message="the `?` operator can only be applied to values \
               that implement `{Try}`",
      label="the `?` operator cannot be applied to type `{Self}`")
)]

would make sense, which is basically @estebank's idea.

@gilescope
Copy link
Contributor

Almost there. Will have a PR for review soon.

@gilescope
Copy link
Contributor

There's almost no difference between an async closure and an async function call in the hir. For now async functions may also have to be reported as being an async closure...

@gilescope
Copy link
Contributor

E.g.

async fn an_async_function() -> u32 {
    let x: Option<u32> = None;
    x?; //~ ERROR the `?` operator
    22
}

vs:

async fn an_async_block() -> u32 {
    async {
        let x: Option<u32> = None;
        x?; //~ ERROR the `?` operator
        22
    }.await
}

All the other cases seem fine, but not sure for this one how to proceed...

@estebank
Copy link
Contributor

estebank commented Oct 28, 2019

Can't you add a field to the relevant hir element to signal the difference?

Edit: Looking at the code, make_async_expr has an argument async_gen_kind: hir::AsyncGeneratorKind which is

pub enum AsyncGeneratorKind {
    /// An explicit `async` block written by the user.
    Block,

    /// An explicit `async` block written by the user.
    Closure,

    /// The `async` block generated as the body of an async function.
    Fn,
}

I'm not sure if that gets propagated to the right place where you'd need it, but we already seem to be differentiating between the three cases, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints AsyncAwait-Polish Async-await issues that are part of the "polish" area AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. F-on_unimplemented Error messages that can be tackled with `#[rustc_on_unimplemented]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants