Keep spans for generics in #[derive(_)] desugaring

estebank · estebank · commit e70105f97193 · 2021-12-03T18:41:39.000Z
Keep the spans for generics coming from a `derive`d Item, so that errors and suggestions have better detail. Fix #84003.
diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
@@ -567,8 +567,10 @@ impl<'a> TraitDef<'a> {
             })
         });
 
-        let Generics { mut params, mut where_clause, span } =
+        let Generics { mut params, mut where_clause, .. } =
             self.generics.to_generics(cx, self.span, type_ident, generics);
+        where_clause.span = generics.where_clause.span;
+        let span = generics.span;
 
         // Create the generic parameters
         params.extend(generics.params.iter().map(|param| match &param.kind {
@@ -589,7 +591,7 @@ impl<'a> TraitDef<'a> {
                         param.bounds.iter().cloned()
                     ).collect();
 
-                cx.typaram(self.span, param.ident, vec![], bounds, None)
+                cx.typaram(param.ident.span, param.ident, vec![], bounds, None)
             }
             GenericParamKind::Const { ty, kw_span, .. } => {
                 let const_nodefault_kind = GenericParamKind::Const {
@@ -610,23 +612,23 @@ impl<'a> TraitDef<'a> {
             match *clause {
                 ast::WherePredicate::BoundPredicate(ref wb) => {
                     ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate {
-                        span: self.span,
+                        span: wb.span,
                         bound_generic_params: wb.bound_generic_params.clone(),
                         bounded_ty: wb.bounded_ty.clone(),
                         bounds: wb.bounds.to_vec(),
                     })
                 }
                 ast::WherePredicate::RegionPredicate(ref rb) => {
                     ast::WherePredicate::RegionPredicate(ast::WhereRegionPredicate {
-                        span: self.span,
+                        span: rb.span,
                         lifetime: rb.lifetime,
                         bounds: rb.bounds.to_vec(),
                     })
                 }
                 ast::WherePredicate::EqPredicate(ref we) => {
                     ast::WherePredicate::EqPredicate(ast::WhereEqPredicate {
                         id: ast::DUMMY_NODE_ID,
-                        span: self.span,
+                        span: we.span,
                         lhs_ty: we.lhs_ty.clone(),
                         rhs_ty: we.rhs_ty.clone(),
                     })
@@ -691,13 +693,13 @@ impl<'a> TraitDef<'a> {
             .iter()
             .map(|param| match param.kind {
                 GenericParamKind::Lifetime { .. } => {
-                    GenericArg::Lifetime(cx.lifetime(self.span, param.ident))
+                    GenericArg::Lifetime(cx.lifetime(param.ident.span, param.ident))
                 }
                 GenericParamKind::Type { .. } => {
-                    GenericArg::Type(cx.ty_ident(self.span, param.ident))
+                    GenericArg::Type(cx.ty_ident(param.ident.span, param.ident))
                 }
                 GenericParamKind::Const { .. } => {
-                    GenericArg::Const(cx.const_ident(self.span, param.ident))
+                    GenericArg::Const(cx.const_ident(param.ident.span, param.ident))
                 }
             })
             .collect();
@@ -1556,11 +1558,9 @@ impl<'a> TraitDef<'a> {
 
         let is_tuple = matches!(struct_def, ast::VariantData::Tuple(..));
         match (just_spans.is_empty(), named_idents.is_empty()) {
-            (false, false) => cx.span_bug(
-                self.span,
-                "a struct with named and unnamed \
-                                          fields in generic `derive`",
-            ),
+            (false, false) => {
+                cx.span_bug(self.span, "a struct with named and unnamed fields in generic `derive`")
+            }
             // named fields
             (_, false) => Named(named_idents),
             // unnamed fields
diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/ty.rs b/compiler/rustc_builtin_macros/src/deriving/generic/ty.rs
@@ -211,14 +211,6 @@ fn mk_ty_param(
     cx.typaram(span, Ident::new(name, span), attrs.to_owned(), bounds, None)
 }
 
-fn mk_generics(params: Vec<ast::GenericParam>, span: Span) -> Generics {
-    Generics {
-        params,
-        where_clause: ast::WhereClause { has_where_token: false, predicates: Vec::new(), span },
-        span,
-    }
-}
-
 /// Bounds on type parameters.
 #[derive(Clone)]
 pub struct Bounds {
@@ -236,7 +228,7 @@ impl Bounds {
         self_ty: Ident,
         self_generics: &Generics,
     ) -> Generics {
-        let generic_params = self
+        let params = self
             .bounds
             .iter()
             .map(|t| {
@@ -245,7 +237,11 @@ impl Bounds {
             })
             .collect();
 
-        mk_generics(generic_params, span)
+        Generics {
+            params,
+            where_clause: ast::WhereClause { has_where_token: false, predicates: Vec::new(), span },
+            span,
+        }
     }
 }
 
diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
@@ -17,8 +17,7 @@ use rustc_index::vec::IndexVec;
 use rustc_macros::HashStable_Generic;
 use rustc_span::source_map::Spanned;
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
-use rustc_span::{def_id::LocalDefId, BytePos};
-use rustc_span::{MultiSpan, Span, DUMMY_SP};
+use rustc_span::{def_id::LocalDefId, BytePos, MultiSpan, Span, DUMMY_SP};
 use rustc_target::asm::InlineAsmRegOrRegClass;
 use rustc_target::spec::abi::Abi;
 
@@ -529,7 +528,6 @@ impl GenericParam<'hir> {
     pub fn bounds_span(&self) -> Option<Span> {
         self.bounds.iter().fold(None, |span, bound| {
             let span = span.map(|s| s.to(bound.span())).unwrap_or_else(|| bound.span());
-
             Some(span)
         })
     }
diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs
@@ -1803,7 +1803,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
         );
         err.span_label(lifetime_ref.span, "undeclared lifetime");
         let mut suggests_in_band = false;
-        let mut suggest_note = true;
+        let mut suggested_spans = vec![];
         for missing in &self.missing_named_lifetime_spots {
             match missing {
                 MissingLifetimeSpot::Generics(generics) => {
@@ -1821,23 +1821,17 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
                         suggests_in_band = true;
                         (generics.span, format!("<{}>", lifetime_ref))
                     };
+                    if suggested_spans.contains(&span) {
+                        continue;
+                    }
+                    suggested_spans.push(span);
                     if !span.from_expansion() {
                         err.span_suggestion(
                             span,
                             &format!("consider introducing lifetime `{}` here", lifetime_ref),
                             sugg,
                             Applicability::MaybeIncorrect,
                         );
-                    } else if suggest_note {
-                        suggest_note = false; // Avoid displaying the same help multiple times.
-                        err.span_label(
-                            span,
-                            &format!(
-                                "lifetime `{}` is missing in item created through this procedural \
-                                 macro",
-                                lifetime_ref,
-                            ),
-                        );
                     }
                 }
                 MissingLifetimeSpot::HigherRanked { span, span_type } => {
diff --git a/src/test/ui/issues/issue-38821.stderr b/src/test/ui/issues/issue-38821.stderr
@@ -10,6 +10,10 @@ note: required because of the requirements on the impl of `IntoNullable` for `<C
 LL | impl<T: NotNull> IntoNullable for T {
    |                  ^^^^^^^^^^^^     ^
    = note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info)
+help: consider further restricting the associated type
+   |
+LL |     Expr: Expression<SqlType=<Col::SqlType as IntoNullable>::Nullable>, <Col as Expression>::SqlType: NotNull
+   |                                                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issues/issue-50480.rs b/src/test/ui/issues/issue-50480.rs
@@ -1,8 +1,17 @@
 #[derive(Clone, Copy)]
 //~^ ERROR the trait `Copy` may not be implemented for this type
-struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
 //~^ ERROR cannot find type `NotDefined` in this scope
 //~| ERROR cannot find type `NotDefined` in this scope
+//~| ERROR cannot find type `N` in this scope
+//~| ERROR cannot find type `N` in this scope
+//~| ERROR `i32` is not an iterator
+
+#[derive(Clone, Copy)]
+//~^ ERROR the trait `Copy` may not be implemented for this type
+struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+//~^ ERROR cannot find type `NotDefined` in this scope
+//~| ERROR cannot find type `N` in this scope
 //~| ERROR `i32` is not an iterator
 
 fn main() {}
diff --git a/src/test/ui/issues/issue-50480.stderr b/src/test/ui/issues/issue-50480.stderr
@@ -1,20 +1,61 @@
-error[E0412]: cannot find type `NotDefined` in this scope
+error[E0412]: cannot find type `N` in this scope
   --> $DIR/issue-50480.rs:3:12
    |
-LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
-   |            ^^^^^^^^^^ not found in this scope
+LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |           -^ not found in this scope
+   |           |
+   |           help: you might be missing a type parameter: `<N>`
 
 error[E0412]: cannot find type `NotDefined` in this scope
+  --> $DIR/issue-50480.rs:3:15
+   |
+LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |               ^^^^^^^^^^ not found in this scope
+
+error[E0412]: cannot find type `N` in this scope
   --> $DIR/issue-50480.rs:3:12
    |
-LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
-   |            ^^^^^^^^^^ not found in this scope
+LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |           -^ not found in this scope
+   |           |
+   |           help: you might be missing a type parameter: `<N>`
+
+error[E0412]: cannot find type `NotDefined` in this scope
+  --> $DIR/issue-50480.rs:3:15
+   |
+LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |           -   ^^^^^^^^^^ not found in this scope
+   |           |
+   |           help: you might be missing a type parameter: `<NotDefined>`
+
+error[E0412]: cannot find type `N` in this scope
+  --> $DIR/issue-50480.rs:12:18
+   |
+LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |            -     ^
+   |            |
+   |            similarly named type parameter `T` defined here
+   |
+help: a type parameter with a similar name exists
+   |
+LL | struct Bar<T>(T, T, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |                  ~
+help: you might be missing a type parameter
+   |
+LL | struct Bar<T, N>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |             +++
+
+error[E0412]: cannot find type `NotDefined` in this scope
+  --> $DIR/issue-50480.rs:12:21
+   |
+LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |                     ^^^^^^^^^^ not found in this scope
 
 error[E0277]: `i32` is not an iterator
-  --> $DIR/issue-50480.rs:3:24
+  --> $DIR/issue-50480.rs:3:27
    |
-LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
-   |                        ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator
+LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |                           ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator
    |
    = help: the trait `Iterator` is not implemented for `i32`
    = note: if you want to iterate between `start` until a value `end`, use the exclusive range syntax `start..end` or the inclusive range syntax `start..=end`
@@ -25,14 +66,36 @@ error[E0204]: the trait `Copy` may not be implemented for this type
 LL | #[derive(Clone, Copy)]
    |                 ^^^^
 LL |
-LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
-   |                                                 --------  ------ this field does not implement `Copy`
-   |                                                 |
-   |                                                 this field does not implement `Copy`
+LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |                                                    --------  ------ this field does not implement `Copy`
+   |                                                    |
+   |                                                    this field does not implement `Copy`
+   |
+   = note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0277]: `i32` is not an iterator
+  --> $DIR/issue-50480.rs:12:33
+   |
+LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |                                 ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator
+   |
+   = help: the trait `Iterator` is not implemented for `i32`
+   = note: if you want to iterate between `start` until a value `end`, use the exclusive range syntax `start..end` or the inclusive range syntax `start..=end`
+
+error[E0204]: the trait `Copy` may not be implemented for this type
+  --> $DIR/issue-50480.rs:10:17
+   |
+LL | #[derive(Clone, Copy)]
+   |                 ^^^^
+LL |
+LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String);
+   |                                                          --------  ------ this field does not implement `Copy`
+   |                                                          |
+   |                                                          this field does not implement `Copy`
    |
    = note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: aborting due to 4 previous errors
+error: aborting due to 10 previous errors
 
 Some errors have detailed explanations: E0204, E0277, E0412.
 For more information about an error, try `rustc --explain E0204`.
diff --git a/src/test/ui/lifetimes/undeclared-lifetime-used-in-debug-macro-issue-70152.stderr b/src/test/ui/lifetimes/undeclared-lifetime-used-in-debug-macro-issue-70152.stderr
@@ -11,9 +11,8 @@ LL |     a: &'b str,
 error[E0261]: use of undeclared lifetime name `'b`
   --> $DIR/undeclared-lifetime-used-in-debug-macro-issue-70152.rs:3:9
    |
-LL | #[derive(Eq, PartialEq)]
-   |          -- lifetime `'b` is missing in item created through this procedural macro
 LL | struct Test {
+   |            - help: consider introducing lifetime `'b` here: `<'b>`
 LL |     a: &'b str,
    |         ^^ undeclared lifetime
    |
diff --git a/src/test/ui/suggestions/derive-macro-missing-bounds.rs b/src/test/ui/suggestions/derive-macro-missing-bounds.rs
diff --git a/src/test/ui/suggestions/derive-macro-missing-bounds.stderr b/src/test/ui/suggestions/derive-macro-missing-bounds.stderr

Original file line number	Diff line number	Diff line change
`@@ -11,9 +11,8 @@ LL \| a: &'b str,`
`11`	`11`	error[E0261]: use of undeclared lifetime name `'b`
`12`	`12`	`--> $DIR/undeclared-lifetime-used-in-debug-macro-issue-70152.rs:3:9`
`13`	`13`	`\|`
`14`		`-LL \| #[derive(Eq, PartialEq)]`
`15`		- \| -- lifetime `'b` is missing in item created through this procedural macro
`16`	`14`	`LL \| struct Test {`
	`15`	+ \| - help: consider introducing lifetime `'b` here: `<'b>`
`17`	`16`	`LL \| a: &'b str,`
`18`	`17`	`\| ^^ undeclared lifetime`
`19`	`18`	`\|`