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

Unhelpful "cannot infer type" error for ? in async block with unclear Result Err type #77880

Closed
joshtriplett opened this issue Oct 13, 2020 · 13 comments
Assignees
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints A-inference Area: Type inference AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. D-terse A diagnostic that doesn't give enough information about the problem at hand E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Experience needed to fix: Medium / intermediate E-mentor Call for participation: This issue has a mentor. Use RustcContributor::new on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@joshtriplett
Copy link
Member

joshtriplett commented Oct 13, 2020

Minimal reproduction:

fn may_error() -> Result<(), ()> { unimplemented!() }

fn main() {
    async {
        may_error()?;
        Ok(())
    };
}

1.47.0 gives the following error:

error[E0282]: type annotations needed
 --> src/main.rs:5:9
  |
5 |         may_error()?;
  |         ^^^^^^^^^^^^ cannot infer type

Nightly gives a marginally more useful error:

error[E0282]: type annotations needed
 --> src/main.rs:5:20
  |
5 |         may_error()?;
  |                    ^ cannot infer type

I would expect an explanation of what type precisely couldn't be inferred, along with a pointer to the containing async block as the thing whose type couldn't be inferred.

rustc --version --verbose (for 1.47.0):

rustc 1.47.0 (18bf6b4f0 2020-10-07)
binary: rustc
commit-hash: 18bf6b4f01a6feaf7259ba7cdae58031af1b7b39
commit-date: 2020-10-07
host: x86_64-unknown-linux-gnu
release: 1.47.0
LLVM version: 11.0

rustc --version --verbose (for nightly):

rustc 1.49.0-nightly (c71248b70 2020-10-11)
binary: rustc
commit-hash: c71248b70870960af9993de4f31d3cba9bbce7e8
commit-date: 2020-10-11
host: x86_64-unknown-linux-gnu
release: 1.49.0-nightly
LLVM version: 11.0
@joshtriplett joshtriplett added the C-bug Category: This is a bug. label Oct 13, 2020
@jyn514 jyn514 added A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints D-terse A diagnostic that doesn't give enough information about the problem at hand A-inference Area: Type inference T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 13, 2020
@jyn514
Copy link
Member

jyn514 commented Oct 13, 2020

Turning this into a closure gives a more helpful error:

error[E0282]: type annotations needed for the closure `fn() -> std::result::Result<(), _>`
 --> src/lib.rs:5:9
  |
5 |         may_error()?;
  |         ^^^^^^^^^^^^ cannot infer type
  |
help: give this closure an explicit return type without `_` placeholders
  |
4 |     let g = || -> std::result::Result<(), _> {
  |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@joshtriplett
Copy link
Member Author

@jyn514 Yeah, I found that several different code changes led to better errors.

I'd guess part of the issue is that there's no stable way to add a type annotation directly to an async block.

@tmandry
Copy link
Contributor

tmandry commented Oct 13, 2020

We should update the part of the code that emits the help text for closures (search for give this closure an explicit return type) to fall back to another error message for async blocks.

Maybe we can still print the type as in the closure case, just to point to the _ as the part we can't infer? It might take some experimenting to get the presentation right.

@tmandry tmandry added E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use RustcContributor::new on Zulip for discussion. E-medium Call for participation: Experience needed to fix: Medium / intermediate labels Oct 13, 2020
@hameerabbasi
Copy link
Contributor

I'm willing to take a shot at this.

@rustbot claim

@tmandry tmandry added this to On deck in wg-async work via automation Oct 16, 2020
@tmandry tmandry moved this from On deck to Claimed in wg-async work Oct 16, 2020
@tmandry tmandry added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Oct 20, 2020
@tmandry
Copy link
Contributor

tmandry commented Dec 3, 2020

@hameerabbasi Are you still planning to work on this?

@hameerabbasi
Copy link
Contributor

Yes, I am, it fell off my radar.

@hameerabbasi
Copy link
Contributor

Ah, I recall what happened here. Here's the Zulip stream: https://rust-lang.zulipchat.com/#narrow/stream/187312-wg-async-foundations/topic/Picking.20up.20issue.20.20.2377880

Basically, I wasn't familiar enough with the internals of the compiler to fix this, unfortunately.

@hameerabbasi
Copy link
Contributor

I'm willing to work on this, but I'll unassign myself so it's more discoverable.

@rustbot release-assignment

@wabain
Copy link
Contributor

wabain commented Dec 20, 2020

It looks like there are a few factors here. One is that impl Trait bindings can't have types annotated explicitly. With #![feature(impl_trait_in_bindings)] we can get at least some details in the error, although not as much as would be desirable:

error[E0282]: type annotations needed for `impl Future`
  --> $DIR/cannot-infer-async-enabled-impl-trait-bindings.rs:13:20
   |
LL |     let fut = async {
   |         --- consider giving `fut` the explicit type `impl Future`, with the type parameters specified
LL |         make_unit()?;
   |                    ^ cannot infer type

The other is that the diagnostics code is oriented around identifying the type of a local variable or function parameter which can be annotated—even when the annotation which will be suggested doesn't involve that binding, like specifying the return type of a closure. This means that the type of an async block will never be described directly because in the desugaring it's wrapped in a GenFuture. It's also possible to get a similar uninformative error message with a closure:

fn may_error() -> Result<(), ()> { Err(()) }

fn consume_closure<T, F: Fn() -> T>(_: F) {}

fn f() {
    consume_closure(|| {
        may_error()?;
        Ok(())
    });
}

Gives:

error[E0282]: type annotations needed
 --> src/lib.rs:7:20
  |
7 |         may_error()?;
  |                    ^ cannot infer type

If the closure is assigned to a local variable before being passed to consume_closure, the error is much better:

error[E0283]: type annotations needed for the closure `fn() -> std::result::Result<(), _>`
  --> src/lib.rs:14:20
   |
14 |         may_error()?;
   |                    ^ cannot infer type
   |
   = note: cannot satisfy `_: From<()>`
   = note: required by `from`
help: give this closure an explicit return type without `_` placeholders
   |
13 |     let v = || -> std::result::Result<(), _> {
   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I'm interested in trying to clean this up a bit. @rustbot claim.

@wabain
Copy link
Contributor

wabain commented Dec 28, 2020

I’d assumed fixing this would involve making the diagnostics aware of generators, but I can’t actually see how much that could be leveraged for async blocks. After looking at this some more I can think of two things we can do here, neither of which are actually specific to async blocks.

First, I think it could be helpful to have an additional note in general when inference fails for the ? operator return, explaining the From conversion applied; I think this is probably a fairly common source of confusion for people learning Rust. We can print the partially inferred From bound which needs to be satisfied. (I haven’t looked at exactly how to implement this, but I’m imagining that we’d check the desguaring kind for the primary span and then walk the HIR to recover the pre-coercion type, in this case the return of may_error().) There might be some work needed to keep it from being too redundant or verbose (or maybe too terse, if the error type of the value ? is invoked on isn't inferred) but I think it would be doable.

Where possible, we can also suggest annotating another expression which would disambiguate the return type. This is actually already supported in some cases, like if a Result returned without ?-coercion is first bound to a local variable, or if the error type occurs as a parameter in a method (but not function!) call. It should be possible to give a similar suggestion to annotate an Ok constructor with the error type.

Putting the suggestions together, the output for the initial example in this issue could be something like:

error[E0282]: type annotations needed
 --> src/main.rs:5:21
  |
5 |         may_error()?;
  |                    ^ cannot infer type of `?` return
  |
  = note: The `?` operator can convert the error value to any type which implements `From<std::io::Error>`
help: consider specifying the type arguments when constructing `std::result::Result::Ok`
  |
6 |         Ok::<T, E>(())
  |           ^^^^^^^^

@wabain
Copy link
Contributor

wabain commented Dec 28, 2020

It looks like there are some good mismatched type diagnostics to piggyback off of.

@tmandry
Copy link
Contributor

tmandry commented Dec 29, 2020

Both of these suggestions sound really nice, I hope we can implement them!

bors added a commit to rust-lang-ci/rust that referenced this issue Jan 12, 2021
…nversion-msg, r=davidtwco

Enhance type inference errors involving the `?` operator

This patch adds a special-cased note on type inference errors when the error span points to a `?` return. It also makes the primary label for such errors "cannot infer type of `?` error" in cases where before we would have only said "cannot infer type".

One beneficiary of this change is async blocks, where we can't explicitly annotate the return type and so may not generate any other help (rust-lang#77880); this lets us at least print the error type we're converting from and anything we know about the type we can't fully infer. More generally, it signposts that an implicit conversion is happening that may have impeded type inference the user was expecting. We already do something similar for [mismatched type errors](https://github.com/rust-lang/rust/blob/2987785df3d46d5ff144a5c67fbb8f5cca798d78/src/test/ui/try-block/try-block-bad-type.stderr#L7).

The check for a relevant `?` operator is built into the existing HIR traversal which looks for places that could be annotated to resolve the error. That means we could identify `?` uses anywhere in the function that output the type we can't infer, but this patch just sticks to adding the note if the primary span given for the error has the operator; if there are other expressions where the type occurs and one of them is selected for the error instead, it's more likely that the `?` operator's implicit conversion isn't the sole cause of the inference failure and that adding an additional diagnostic would just be noise. I added a ui test for one such case.

The data about the `?` conversion is passed around in a `UseDiagnostic` enum that in theory could be used to add more of this kind of note in the future. It was also just easier to pass around than something with a more specific name. There are some follow-up refactoring commits for the code that generates the error label, which was already pretty involved and made a bit more complicated by this change.
@tmandry
Copy link
Contributor

tmandry commented Jan 28, 2021

Triage: nightly now gives

error[E0282]: type annotations needed
 --> src/main.rs:5:20
  |
5 |         may_error()?;
  |                    ^ cannot infer type of error for `?` operator
  |
  = note: `?` implicitly converts the error value into a type implementing `From<()>`

thanks to #80517 (thanks @wabain)! I think this means the issue is fixed.

@tmandry tmandry closed this as completed Jan 28, 2021
wg-async work automation moved this from Claimed to Done Jan 28, 2021
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 A-inference Area: Type inference AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. D-terse A diagnostic that doesn't give enough information about the problem at hand E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Experience needed to fix: Medium / intermediate E-mentor Call for participation: This issue has a mentor. Use RustcContributor::new on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Archived in project
Development

No branches or pull requests

5 participants