Annotate derived spans and move span suggestion code

* Annotate `derive`d spans from the user's code with the appropciate context
* Add `Span::can_be_used_for_suggestion` to query if the underlying span
  at the users' code

Author: estebank

compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

@@ -17,7 +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, ExpnKind, MacroKind, 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;
 
@@ -525,20 +525,21 @@ pub struct GenericParam<'hir> {
 }
 
 impl GenericParam<'hir> {
-    pub fn bounds_span(&self) -> Option<Span> {
-        self.bounds.iter().fold(None, |span, bound| {
-            if let ExpnKind::Macro(MacroKind::Derive, _) =
-                bound.span().ctxt().outer_expn_data().kind
-            {
-                // We ignore bounds that come from exclusively from a `#[derive(_)]`, because we
-                // can't really point at them, and we sometimes use this method to get a span
-                // appropriate for suggestions.
-                span
-            } else {
-                let span = span.map(|s| s.to(bound.span())).unwrap_or_else(|| bound.span());
-                Some(span)
-            }
-        })
+    pub fn bounds_span_for_suggestions(&self) -> Option<Span> {
+        self.bounds
+            .iter()
+            .fold(None, |span: Option<Span>, bound| {
+                // We include bounds that come from a `#[derive(_)]` but point at the user's code,
+                // as we use this method to get a span appropriate for suggestions.
+                // FIXME: a more "principled" approach should be taken here.
+                if !bound.span().can_be_used_for_suggestions() {
+                    None
+                } else {
+                    let span = span.map(|s| s.to(bound.span())).unwrap_or_else(|| bound.span());
+                    Some(span)
+                }
+            })
+            .map(|sp| sp.shrink_to_hi())
     }
 }
 

compiler/rustc_hir/src/hir.rs

@@ -270,7 +270,7 @@ pub fn suggest_constraining_type_param(
         // `where` clause instead of `trait Base<T: Copy = String>: Super<T>`.
         && !matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. })
     {
-        if let Some(bounds_span) = param.bounds_span() {
+        if let Some(span) = param.bounds_span_for_suggestions() {
             // If user has provided some bounds, suggest restricting them:
             //
             //   fn foo<T: Foo>(t: T) { ... }
@@ -284,7 +284,7 @@ pub fn suggest_constraining_type_param(
             //          --
             //          |
             //          replace with: `T: Bar +`
-            suggest_restrict(bounds_span.shrink_to_hi());
+            suggest_restrict(span);
         } else {
             // If user hasn't provided any bounds, suggest adding a new one:
             //

compiler/rustc_middle/src/ty/diagnostics.rs

@@ -1735,7 +1735,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
                         (generics.span, format!("<{}>", ident))
                     };
                     // Do not suggest if this is coming from macro expansion.
-                    if !span.from_expansion() {
+                    if span.can_be_used_for_suggestions() {
                         return Some((
                             span.shrink_to_hi(),
                             msg,
@@ -1825,7 +1825,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
                         continue;
                     }
                     suggested_spans.push(span);
-                    if !span.from_expansion() {
+                    if span.can_be_used_for_suggestions() {
                         err.span_suggestion(
                             span,
                             &format!("consider introducing lifetime `{}` here", lifetime_ref),

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -551,6 +551,16 @@ impl Span {
         matches!(self.ctxt().outer_expn_data().kind, ExpnKind::Macro(MacroKind::Derive, _))
     }
 
+    /// Gate suggestions that would not be appropriate in a context the user didn't write.
+    pub fn can_be_used_for_suggestions(self) -> bool {
+        !self.from_expansion()
+        // FIXME: If this span comes from a `derive` macro but it points at code the user wrote,
+        // the callsite span and the span will be pointing at different places. It also means that
+        // we can safely provide suggestions on this span.
+            || (matches!(self.ctxt().outer_expn_data().kind, ExpnKind::Macro(MacroKind::Derive, _))
+                && self.parent_callsite().map(|p| (p.lo(), p.hi())) != Some((self.lo(), self.hi())))
+    }
+
     #[inline]
     pub fn with_root_ctxt(lo: BytePos, hi: BytePos) -> Span {
         Span::new(lo, hi, SyntaxContext::root(), None)

compiler/rustc_span/src/lib.rs

@@ -262,16 +262,16 @@ fn suggest_restriction(
             match generics
                 .params
                 .iter()
-                .map(|p| p.bounds_span().unwrap_or(p.span))
-                .filter(|&span| generics.span.contains(span) && span.desugaring_kind().is_none())
+                .map(|p| p.bounds_span_for_suggestions().unwrap_or(p.span.shrink_to_hi()))
+                .filter(|&span| generics.span.contains(span) && span.can_be_used_for_suggestions())
                 .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(span) => (span.shrink_to_hi(), format!(", {}", type_param)),
+                Some(span) => (span, format!(", {}", type_param)),
             },
             // `fn foo(t: impl Trait)`
             //                       ^ suggest `where <T as Trait>::A: Bound`

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -177,11 +177,9 @@ crate fn placeholder_type_error(
         sugg.push((arg.span, (*type_name).to_string()));
     } else {
         let last = generics.iter().last().unwrap();
-        sugg.push((
-            // Account for bounds, we want `fn foo<T: E, K>(_: K)` not `fn foo<T, K: E>(_: K)`.
-            last.bounds_span().unwrap_or(last.span).shrink_to_hi(),
-            format!(", {}", type_name),
-        ));
+        // Account for bounds, we want `fn foo<T: E, K>(_: K)` not `fn foo<T, K: E>(_: K)`.
+        let span = last.bounds_span_for_suggestions().unwrap_or(last.span.shrink_to_hi());
+        sugg.push((span, format!(", {}", type_name)));
     }
 
     let mut err = bad_placeholder_type(tcx, placeholder_types, kind);

compiler/rustc_typeck/src/collect.rs

@@ -12,8 +12,8 @@ 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
-   |                                                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+LL |     Expr: Expression<SqlType=<Col::SqlType as IntoNullable>::Nullable>, <Col as Expression>::SqlType: NotNull,
+   |                                                                       +++++++++++++++++++++++++++++++++++++++
 
 error: aborting due to previous error