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

"introduce a type parameter with a trait bound instead of using impl Trait" diagnostic is malformed for async fns where the previous parameter is a borrow #79843

Closed
Arnavion opened this issue Dec 8, 2020 · 1 comment · Fixed by #80211
Labels
A-async-await Area: Async & Await A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate.

Comments

@Arnavion
Copy link

Arnavion commented Dec 8, 2020

(Edit: Actually, being an inherent fn doesn't matter. It happens with top-level fns too.)

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=bc52a6f5c708462abbd91b4dd7d9b1ba

trait Foo {
    type Bar;
    fn bar(&self) -> Self::Bar;
}

async fn run(_: &(), foo: impl Foo) -> std::io::Result<()> {
    fn assert_is_send<T: Send>(_: &T) {}

    let bar = foo.bar();
    assert_is_send(&bar);

    Ok(())
}

This prints the diagnostic:

error[E0277]: `<impl Foo as Foo>::Bar` cannot be sent between threads safely
  --> src/lib.rs:10:20
   |
7  |     fn assert_is_send<T: Send>(_: &T) {}
   |                          ---- required by this bound in `assert_is_send`
...
10 |     assert_is_send(&bar);
   |                    ^^^^ `<impl Foo as Foo>::Bar` cannot be sent between threads safely
   |
   = help: the trait `Send` is not implemented for `<impl Foo as Foo>::Bar`
help: introduce a type parameter with a trait bound instead of using `impl Trait`
   |
6  | async fn run(_: &, F: Foo(), foo: F) -> std::io::Result<()> where <F as Foo>::Bar: Send {
   |                  ^^^^^^^^         ^                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^

The suggestion in the last line is not valid syntax (_: &,). It also emits malformed syntax if the first parameter is a &self or &mut self parameter.

The diagnostic becomes well-formed if any of the following is done:

  • The first parameter is changed to _: () instead of _: &()

  • The fn is not an async fn.

In either case, the diagnostic correctly says:

6  | fn run<F: Foo>(_: &(), foo: F) -> std::io::Result<()> where <F as Foo>::Bar: Send {
   |       ^^^^^^^^              ^                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^

Meta

Happens on both nightly:

rustc 1.50.0-nightly (e792288df 2020-12-05)
binary: rustc
commit-hash: e792288df31636ca28108516c63a00ce4267063a
commit-date: 2020-12-05
host: x86_64-unknown-linux-gnu
release: 1.50.0-nightly

... and stable:

rustc 1.48.0 (7eac88abb 2020-11-16)
binary: rustc
commit-hash: 7eac88abb2e57e752f3302f02be5f3ce3d7adfb4
commit-date: 2020-11-16
host: x86_64-unknown-linux-gnu
release: 1.48.0
LLVM version: 11.0
@Arnavion Arnavion added the C-bug Category: This is a bug. label Dec 8, 2020
@Arnavion Arnavion changed the title "introduce a type parameter with a trait bound instead of using impl Trait" diagnostic is malformed for async inherent fns where the previous parameter is a borrow "introduce a type parameter with a trait bound instead of using impl Trait" diagnostic is malformed for async fns where the previous parameter is a borrow Dec 8, 2020
@camelid camelid added A-async-await Area: Async & Await A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. labels Dec 8, 2020
@tmandry tmandry added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Dec 17, 2020
@tmandry
Copy link
Member

tmandry commented Dec 17, 2020

This should hopefully be a fairly straightforward patch to the diagnostics code that generates the suggestion.

@tmandry tmandry added E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels Dec 17, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 21, 2020
…ion, r=petrochenkov

Handle desugaring in impl trait bound suggestion

Fixes rust-lang#79843.

When an associated type of a generic function parameter needs extra bounds, the diagnostics may suggest replacing an `impl Trait` with a named type parameter so that it can be referenced in the where clause. On stable and nightly, the suggestion can be malformed, for instance transforming:

```rust
async fn run(_: &(), foo: impl Foo) -> std::io::Result<()>
```

Into:

```rust
async fn run(_: &, F: Foo(), foo: F) -> std::io::Result<()> where <F as Foo>::Bar: Send
                 ^^^^^^^^         ^                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

Where we want something like:

```rust
async fn run<F: Foo>(_: &(), foo: F) -> std::io::Result<()> where <F as Foo>::Bar: Send
            ^^^^^^^^              ^                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

The problem is that the elided lifetime of `&()` is added as a generic parameter when desugaring the async fn; the suggestion code sees this as an existing generic parameter and tries to use its span as an anchor to inject `F` into the parameter list. There doesn't seem to be an entirely principled way to check which generic parameters in the HIR were explicitly named in the source, so this commit changes the heuristics when generating the suggestion to only consider type parameters whose spans are contained within the span of the `Generics` when determining how to insert an additional type parameter into the declaration. (And to be safe it also excludes parameters whose spans are marked as originating from desugaring, although that doesn't seem to handle this elided lifetime.)
@bors bors closed this as completed in 2528acb Dec 21, 2020
@tmandry tmandry moved this to Done in wg-async work Dec 8, 2022
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-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants