Rollup merge of #69722 - estebank:negative-impl-span-ast, r=Centril

Tweak output for invalid negative impl AST errors

Use more accurate spans for negative `impl` errors.

r? @Centril

src/librustc_ast/ast.rs

@@ -2118,14 +2118,14 @@ pub enum ImplPolarity {
     /// `impl Trait for Type`
     Positive,
     /// `impl !Trait for Type`
-    Negative,
+    Negative(Span),
 }
 
 impl fmt::Debug for ImplPolarity {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match *self {
             ImplPolarity::Positive => "positive".fmt(f),
-            ImplPolarity::Negative => "negative".fmt(f),
+            ImplPolarity::Negative(_) => "negative".fmt(f),
         }
     }
 }

src/librustc_ast_passes/ast_validation.rs

@@ -9,6 +9,7 @@
 use rustc_ast::ast::*;
 use rustc_ast::attr;
 use rustc_ast::expand::is_proc_macro_attr;
+use rustc_ast::ptr::P;
 use rustc_ast::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor};
 use rustc_ast::walk_list;
 use rustc_ast_pretty::pprust;
@@ -594,6 +595,54 @@ impl<'a> AstValidator<'a> {
             .span_label(ident.span, format!("`_` is not a valid name for this `{}` item", kind))
             .emit();
     }
+
+    fn deny_generic_params(&self, generics: &Generics, ident_span: Span) {
+        if !generics.params.is_empty() {
+            struct_span_err!(
+                self.session,
+                generics.span,
+                E0567,
+                "auto traits cannot have generic parameters"
+            )
+            .span_label(ident_span, "auto trait cannot have generic parameters")
+            .span_suggestion(
+                generics.span,
+                "remove the parameters",
+                String::new(),
+                Applicability::MachineApplicable,
+            )
+            .emit();
+        }
+    }
+
+    fn deny_super_traits(&self, bounds: &GenericBounds, ident_span: Span) {
+        if let [first @ last] | [first, .., last] = &bounds[..] {
+            let span = first.span().to(last.span());
+            struct_span_err!(self.session, span, E0568, "auto traits cannot have super traits")
+                .span_label(ident_span, "auto trait cannot have super traits")
+                .span_suggestion(
+                    span,
+                    "remove the super traits",
+                    String::new(),
+                    Applicability::MachineApplicable,
+                )
+                .emit();
+        }
+    }
+
+    fn deny_items(&self, trait_items: &[P<AssocItem>], ident_span: Span) {
+        if !trait_items.is_empty() {
+            let spans: Vec<_> = trait_items.iter().map(|i| i.ident.span).collect();
+            struct_span_err!(
+                self.session,
+                spans,
+                E0380,
+                "auto traits cannot have methods or associated items"
+            )
+            .span_label(ident_span, "auto trait cannot have items")
+            .emit();
+        }
+    }
 }
 
 fn validate_generic_param_order<'a>(
@@ -779,7 +828,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 defaultness: _,
                 constness: _,
                 generics: _,
-                of_trait: Some(_),
+                of_trait: Some(ref t),
                 ref self_ty,
                 items: _,
             } => {
@@ -794,13 +843,14 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                             .help("use `auto trait Trait {}` instead")
                             .emit();
                     }
-                    if let (Unsafe::Yes(span), ImplPolarity::Negative) = (unsafety, polarity) {
+                    if let (Unsafe::Yes(span), ImplPolarity::Negative(sp)) = (unsafety, polarity) {
                         struct_span_err!(
                             this.session,
-                            item.span,
+                            sp.to(t.path.span),
                             E0198,
                             "negative impls cannot be unsafe"
                         )
+                        .span_label(sp, "negative because of this")
                         .span_label(span, "unsafe because of this")
                         .emit();
                     }
@@ -816,38 +866,36 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 constness,
                 generics: _,
                 of_trait: None,
-                self_ty: _,
+                ref self_ty,
                 items: _,
             } => {
+                let error = |annotation_span, annotation| {
+                    let mut err = self.err_handler().struct_span_err(
+                        self_ty.span,
+                        &format!("inherent impls cannot be {}", annotation),
+                    );
+                    err.span_label(annotation_span, &format!("{} because of this", annotation));
+                    err.span_label(self_ty.span, "inherent impl for this type");
+                    err
+                };
+
                 self.invalid_visibility(
                     &item.vis,
                     Some("place qualifiers on individual impl items instead"),
                 );
                 if let Unsafe::Yes(span) = unsafety {
-                    struct_span_err!(
-                        self.session,
-                        item.span,
-                        E0197,
-                        "inherent impls cannot be unsafe"
-                    )
-                    .span_label(span, "unsafe because of this")
-                    .emit();
+                    error(span, "unsafe").code(error_code!(E0197)).emit();
                 }
-                if polarity == ImplPolarity::Negative {
-                    self.err_handler().span_err(item.span, "inherent impls cannot be negative");
+                if let ImplPolarity::Negative(span) = polarity {
+                    error(span, "negative").emit();
                 }
                 if let Defaultness::Default(def_span) = defaultness {
-                    let span = self.session.source_map().def_span(item.span);
-                    self.err_handler()
-                        .struct_span_err(span, "inherent impls cannot be `default`")
-                        .span_label(def_span, "`default` because of this")
+                    error(def_span, "`default`")
                         .note("only trait implementations may be annotated with `default`")
                         .emit();
                 }
                 if let Const::Yes(span) = constness {
-                    self.err_handler()
-                        .struct_span_err(item.span, "inherent impls cannot be `const`")
-                        .span_label(span, "`const` because of this")
+                    error(span, "`const`")
                         .note("only trait implementations may be annotated with `const`")
                         .emit();
                 }
@@ -882,33 +930,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
             ItemKind::Trait(is_auto, _, ref generics, ref bounds, ref trait_items) => {
                 if is_auto == IsAuto::Yes {
                     // Auto traits cannot have generics, super traits nor contain items.
-                    if !generics.params.is_empty() {
-                        struct_span_err!(
-                            self.session,
-                            item.span,
-                            E0567,
-                            "auto traits cannot have generic parameters"
-                        )
-                        .emit();
-                    }
-                    if !bounds.is_empty() {
-                        struct_span_err!(
-                            self.session,
-                            item.span,
-                            E0568,
-                            "auto traits cannot have super traits"
-                        )
-                        .emit();
-                    }
-                    if !trait_items.is_empty() {
-                        struct_span_err!(
-                            self.session,
-                            item.span,
-                            E0380,
-                            "auto traits cannot have methods or associated items"
-                        )
-                        .emit();
-                    }
+                    self.deny_generic_params(generics, item.ident.span);
+                    self.deny_super_traits(bounds, item.ident.span);
+                    self.deny_items(trait_items, item.ident.span);
                 }
                 self.no_questions_in_bounds(bounds, "supertraits", true);
 
@@ -1153,9 +1177,13 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         }) = fk.header()
         {
             self.err_handler()
-                .struct_span_err(span, "functions cannot be both `const` and `async`")
+                .struct_span_err(
+                    vec![*cspan, *aspan],
+                    "functions cannot be both `const` and `async`",
+                )
                 .span_label(*cspan, "`const` because of this")
                 .span_label(*aspan, "`async` because of this")
+                .span_label(span, "") // Point at the fn header.
                 .emit();
         }
 

src/librustc_ast_passes/feature_gate.rs

@@ -337,14 +337,14 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
                 }
             }
 
-            ast::ItemKind::Impl { polarity, defaultness, .. } => {
-                if polarity == ast::ImplPolarity::Negative {
+            ast::ItemKind::Impl { polarity, defaultness, ref of_trait, .. } => {
+                if let ast::ImplPolarity::Negative(span) = polarity {
                     gate_feature_post!(
                         &self,
                         optin_builtin_traits,
-                        i.span,
+                        span.to(of_trait.as_ref().map(|t| t.path.span).unwrap_or(span)),
                         "negative trait bounds are not yet fully implemented; \
-                                        use marker types for now"
+                         use marker types for now"
                     );
                 }
 

src/librustc_ast_passes/lib.rs

@@ -1,3 +1,4 @@
+#![feature(bindings_after_at)]
 //! The `rustc_ast_passes` crate contains passes which validate the AST in `syntax`
 //! parsed by `rustc_parse` and then lowered, after the passes in this crate,
 //! by `rustc_ast_lowering`.

src/librustc_ast_pretty/pprust.rs

@@ -1160,7 +1160,7 @@ impl<'a> State<'a> {
                     self.s.space();
                 }
 
-                if polarity == ast::ImplPolarity::Negative {
+                if let ast::ImplPolarity::Negative(_) = polarity {
                     self.s.word("!");
                 }
 

src/librustc_hir/print.rs

@@ -652,7 +652,7 @@ impl<'a> State<'a> {
                     self.word_nbsp("const");
                 }
 
-                if let hir::ImplPolarity::Negative = polarity {
+                if let hir::ImplPolarity::Negative(_) = polarity {
                     self.s.word("!");
                 }
 

src/librustc_parse/parser/item.rs

@@ -373,6 +373,16 @@ impl<'a> Parser<'a> {
         self.token.is_keyword(kw::Async) && self.is_keyword_ahead(1, &[kw::Fn])
     }
 
+    fn parse_polarity(&mut self) -> ast::ImplPolarity {
+        // Disambiguate `impl !Trait for Type { ... }` and `impl ! { ... }` for the never type.
+        if self.check(&token::Not) && self.look_ahead(1, |t| t.can_begin_type()) {
+            self.bump(); // `!`
+            ast::ImplPolarity::Negative(self.prev_token.span)
+        } else {
+            ast::ImplPolarity::Positive
+        }
+    }
+
     /// Parses an implementation item.
     ///
     /// ```
@@ -411,13 +421,7 @@ impl<'a> Parser<'a> {
             self.sess.gated_spans.gate(sym::const_trait_impl, span);
         }
 
-        // Disambiguate `impl !Trait for Type { ... }` and `impl ! { ... }` for the never type.
-        let polarity = if self.check(&token::Not) && self.look_ahead(1, |t| t.can_begin_type()) {
-            self.bump(); // `!`
-            ast::ImplPolarity::Negative
-        } else {
-            ast::ImplPolarity::Positive
-        };
+        let polarity = self.parse_polarity();
 
         // Parse both types and traits as a type, then reinterpret if necessary.
         let err_path = |span| ast::Path::from_ident(Ident::new(kw::Invalid, span));

src/librustc_save_analysis/sig.rs

@@ -519,7 +519,7 @@ impl Sig for ast::Item {
                 text.push(' ');
 
                 let trait_sig = if let Some(ref t) = *of_trait {
-                    if polarity == ast::ImplPolarity::Negative {
+                    if let ast::ImplPolarity::Negative(_) = polarity {
                         text.push('!');
                     }
                     let trait_sig = t.path.make(offset + text.len(), id, scx)?;

src/librustc_typeck/coherence/unsafety.rs

@@ -69,11 +69,11 @@ impl UnsafetyChecker<'tcx> {
                     .emit();
                 }
 
-                (_, _, Unsafety::Unsafe, hir::ImplPolarity::Negative) => {
+                (_, _, Unsafety::Unsafe, hir::ImplPolarity::Negative(_)) => {
                     // Reported in AST validation
                     self.tcx.sess.delay_span_bug(item.span, "unsafe negative impl");
                 }
-                (_, _, Unsafety::Normal, hir::ImplPolarity::Negative)
+                (_, _, Unsafety::Normal, hir::ImplPolarity::Negative(_))
                 | (Unsafety::Unsafe, _, Unsafety::Unsafe, hir::ImplPolarity::Positive)
                 | (Unsafety::Normal, Some(_), Unsafety::Unsafe, hir::ImplPolarity::Positive)
                 | (Unsafety::Normal, None, Unsafety::Normal, _) => {

src/librustc_typeck/collect.rs

@@ -1548,7 +1548,7 @@ fn impl_polarity(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ImplPolarity {
     let is_rustc_reservation = tcx.has_attr(def_id, sym::rustc_reservation_impl);
     let item = tcx.hir().expect_item(hir_id);
     match &item.kind {
-        hir::ItemKind::Impl { polarity: hir::ImplPolarity::Negative, .. } => {
+        hir::ItemKind::Impl { polarity: hir::ImplPolarity::Negative(_), .. } => {
             if is_rustc_reservation {
                 tcx.sess.span_err(item.span, "reservation impls can't be negative");
             }

src/test/ui/async-await/no-const-async.stderr

@@ -1,8 +1,8 @@
 error: functions cannot be both `const` and `async`
-  --> $DIR/no-const-async.rs:4:1
+  --> $DIR/no-const-async.rs:4:5
    |
 LL | pub const async fn x() {}
-   | ^^^^-----^-----^^^^^^^^^^
+   | ----^^^^^-^^^^^----------
    |     |     |
    |     |     `async` because of this
    |     `const` because of this

src/test/ui/auto-trait-validation.stderr

@@ -1,20 +1,26 @@
 error[E0567]: auto traits cannot have generic parameters
-  --> $DIR/auto-trait-validation.rs:3:1
+  --> $DIR/auto-trait-validation.rs:3:19
    |
 LL | auto trait Generic<T> {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^
+   |            -------^^^ help: remove the parameters
+   |            |
+   |            auto trait cannot have generic parameters
 
 error[E0568]: auto traits cannot have super traits
-  --> $DIR/auto-trait-validation.rs:5:1
+  --> $DIR/auto-trait-validation.rs:5:20
    |
 LL | auto trait Bound : Copy {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |            -----   ^^^^ help: remove the super traits
+   |            |
+   |            auto trait cannot have super traits
 
 error[E0380]: auto traits cannot have methods or associated items
-  --> $DIR/auto-trait-validation.rs:7:1
+  --> $DIR/auto-trait-validation.rs:7:25
    |
 LL | auto trait MyTrait { fn foo() {} }
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |            -------      ^^^
+   |            |
+   |            auto trait cannot have items
 
 error: aborting due to 3 previous errors
 

src/test/ui/coherence/coherence-negative-impls-safe.stderr

@@ -1,9 +1,10 @@
 error[E0198]: negative impls cannot be unsafe
-  --> $DIR/coherence-negative-impls-safe.rs:7:1
+  --> $DIR/coherence-negative-impls-safe.rs:7:13
    |
 LL | unsafe impl !Send for TestType {}
-   | ------^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   | |
+   | ------      -^^^^
+   | |           |
+   | |           negative because of this
    | unsafe because of this
 
 error: aborting due to previous error

src/test/ui/error-codes/E0197.stderr

@@ -1,8 +1,8 @@
 error[E0197]: inherent impls cannot be unsafe
-  --> $DIR/E0197.rs:3:1
+  --> $DIR/E0197.rs:3:13
    |
 LL | unsafe impl Foo { }
-   | ------^^^^^^^^^^^^^
+   | ------      ^^^ inherent impl for this type
    | |
    | unsafe because of this