about summary refs log tree commit diff
diff options
context:
space:
mode:
authorDylan DPC <dylan.dpc@gmail.com>2020-12-21 02:47:44 +0100
committerGitHub <noreply@github.com>2020-12-21 02:47:44 +0100
commit2528acb5f7fabd68fc951fda1f724640c38c1042 (patch)
tree0c2a09183ff6d62ef9aed9eb953f5c09a244aff4
parent000c51611cc441c7a879adb9667c11fab6608477 (diff)
parentb76c9be7f5fa10b95fe4f872400b4388b67a4733 (diff)
downloadrust-2528acb5f7fabd68fc951fda1f724640c38c1042.tar.gz
rust-2528acb5f7fabd68fc951fda1f724640c38c1042.zip
Rollup merge of #80211 - wabain:async-fn-trait-bound-suggestion, r=petrochenkov
Handle desugaring in impl trait bound suggestion

Fixes #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.)
-rw-r--r--compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs18
-rw-r--r--src/test/ui/suggestions/impl-trait-with-missing-bounds.rs8
-rw-r--r--src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr17
-rw-r--r--src/test/ui/suggestions/issue-79843-impl-trait-with-missing-bounds-on-async-fn.rs32
-rw-r--r--src/test/ui/suggestions/issue-79843-impl-trait-with-missing-bounds-on-async-fn.stderr33
5 files changed, 95 insertions, 13 deletions
diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
index b4b71a48ce9..79fea83a667 100644
--- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
+++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -254,27 +254,21 @@ fn suggest_restriction(
         let pred = trait_ref.without_const().to_predicate(tcx).to_string();
         let pred = pred.replace(&impl_trait_str, &type_param_name);
         let mut sugg = vec![
+            // Find the last of the generic parameters contained within the span of
+            // the generics
             match generics
                 .params
                 .iter()
-                .filter(|p| match p.kind {
-                    hir::GenericParamKind::Type {
-                        synthetic: Some(hir::SyntheticTyParamKind::ImplTrait),
-                        ..
-                    } => false,
-                    _ => true,
-                })
-                .last()
+                .map(|p| p.bounds_span().unwrap_or(p.span))
+                .filter(|&span| generics.span.contains(span) && span.desugaring_kind().is_none())
+                .max_by_key(|span| span.hi())
             {
                 // `fn foo(t: impl Trait)`
                 //        ^ suggest `<T: Trait>` here
                 None => (generics.span, format!("<{}>", type_param)),
                 // `fn foo<A>(t: impl Trait)`
                 //        ^^^ suggest `<A, T: Trait>` here
-                Some(param) => (
-                    param.bounds_span().unwrap_or(param.span).shrink_to_hi(),
-                    format!(", {}", type_param),
-                ),
+                Some(span) => (span.shrink_to_hi(), format!(", {}", type_param)),
             },
             // `fn foo(t: impl Trait)`
             //                       ^ suggest `where <T as Trait>::A: Bound`
diff --git a/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs b/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs
index d401328077a..949b2360071 100644
--- a/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs
+++ b/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs
@@ -39,6 +39,14 @@ fn bak(constraints: impl  Iterator + std::fmt::Debug) {
     }
 }
 
+#[rustfmt::skip]
+fn baw<>(constraints: impl Iterator) {
+    for constraint in constraints {
+        qux(constraint);
+//~^ ERROR `<impl Iterator as Iterator>::Item` doesn't implement `Debug`
+    }
+}
+
 fn qux(_: impl std::fmt::Debug) {}
 
 fn main() {}
diff --git a/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr b/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr
index 099eb1c9d00..0de3b9aec19 100644
--- a/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr
+++ b/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr
@@ -73,6 +73,21 @@ help: introduce a type parameter with a trait bound instead of using `impl Trait
 LL | fn bak<I: Iterator + std::fmt::Debug>(constraints: I) where <I as Iterator>::Item: Debug {
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^              ^  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 5 previous errors
+error[E0277]: `<impl Iterator as Iterator>::Item` doesn't implement `Debug`
+  --> $DIR/impl-trait-with-missing-bounds.rs:45:13
+   |
+LL |         qux(constraint);
+   |             ^^^^^^^^^^ `<impl Iterator as Iterator>::Item` cannot be formatted using `{:?}` because it doesn't implement `Debug`
+...
+LL | fn qux(_: impl std::fmt::Debug) {}
+   |                --------------- required by this bound in `qux`
+   |
+   = help: the trait `Debug` is not implemented for `<impl Iterator as Iterator>::Item`
+help: introduce a type parameter with a trait bound instead of using `impl Trait`
+   |
+LL | fn baw<I: Iterator>(constraints: I) where <I as Iterator>::Item: Debug {
+   |       ^^^^^^^^^^^^^              ^  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 6 previous errors
 
 For more information about this error, try `rustc --explain E0277`.
diff --git a/src/test/ui/suggestions/issue-79843-impl-trait-with-missing-bounds-on-async-fn.rs b/src/test/ui/suggestions/issue-79843-impl-trait-with-missing-bounds-on-async-fn.rs
new file mode 100644
index 00000000000..3cd6d336e13
--- /dev/null
+++ b/src/test/ui/suggestions/issue-79843-impl-trait-with-missing-bounds-on-async-fn.rs
@@ -0,0 +1,32 @@
+// Regression test: if we suggest replacing an `impl Trait` argument to an async
+// fn with a named type parameter in order to add bounds, the suggested function
+// signature should be well-formed.
+//
+// edition:2018
+
+trait Foo {
+    type Bar;
+    fn bar(&self) -> Self::Bar;
+}
+
+async fn run(_: &(), foo: impl Foo) -> std::io::Result<()> {
+    let bar = foo.bar();
+    assert_is_send(&bar);
+//~^ ERROR: `<impl Foo as Foo>::Bar` cannot be sent between threads safely
+
+    Ok(())
+}
+
+// Test our handling of cases where there is a generic parameter list in the
+// source, but only synthetic generic parameters
+async fn run2< >(_: &(), foo: impl Foo) -> std::io::Result<()> {
+    let bar = foo.bar();
+    assert_is_send(&bar);
+//~^ ERROR: `<impl Foo as Foo>::Bar` cannot be sent between threads safely
+
+    Ok(())
+}
+
+fn assert_is_send<T: Send>(_: &T) {}
+
+fn main() {}
diff --git a/src/test/ui/suggestions/issue-79843-impl-trait-with-missing-bounds-on-async-fn.stderr b/src/test/ui/suggestions/issue-79843-impl-trait-with-missing-bounds-on-async-fn.stderr
new file mode 100644
index 00000000000..9404c3bb583
--- /dev/null
+++ b/src/test/ui/suggestions/issue-79843-impl-trait-with-missing-bounds-on-async-fn.stderr
@@ -0,0 +1,33 @@
+error[E0277]: `<impl Foo as Foo>::Bar` cannot be sent between threads safely
+  --> $DIR/issue-79843-impl-trait-with-missing-bounds-on-async-fn.rs:14:20
+   |
+LL |     assert_is_send(&bar);
+   |                    ^^^^ `<impl Foo as Foo>::Bar` cannot be sent between threads safely
+...
+LL | fn assert_is_send<T: Send>(_: &T) {}
+   |                      ---- required by this bound in `assert_is_send`
+   |
+   = 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`
+   |
+LL | async fn run<F: Foo>(_: &(), foo: F) -> std::io::Result<()> where <F as Foo>::Bar: Send {
+   |             ^^^^^^^^              ^                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error[E0277]: `<impl Foo as Foo>::Bar` cannot be sent between threads safely
+  --> $DIR/issue-79843-impl-trait-with-missing-bounds-on-async-fn.rs:24:20
+   |
+LL |     assert_is_send(&bar);
+   |                    ^^^^ `<impl Foo as Foo>::Bar` cannot be sent between threads safely
+...
+LL | fn assert_is_send<T: Send>(_: &T) {}
+   |                      ---- required by this bound in `assert_is_send`
+   |
+   = 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`
+   |
+LL | async fn run2<F: Foo>(_: &(), foo: F) -> std::io::Result<()> where <F as Foo>::Bar: Send {
+   |              ^^^^^^^^              ^                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0277`.