From 91525fd078bb6f3ec833aa42141ea027b37b26a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 4 Mar 2020 16:15:23 -0800 Subject: [PATCH 1/7] Tweak output for invalid negative impl AST errors --- src/librustc_ast/ast.rs | 4 +-- src/librustc_ast_passes/ast_validation.rs | 28 ++++++++++++------- src/librustc_ast_passes/feature_gate.rs | 8 +++--- src/librustc_ast_pretty/pprust.rs | 2 +- src/librustc_hir/print.rs | 2 +- src/librustc_parse/parser/item.rs | 2 +- src/librustc_save_analysis/sig.rs | 2 +- src/librustc_typeck/coherence/unsafety.rs | 4 +-- src/librustc_typeck/collect.rs | 2 +- .../coherence-negative-impls-safe.stderr | 4 +-- src/test/ui/error-codes/E0197.stderr | 2 +- src/test/ui/error-codes/E0198.stderr | 4 +-- .../feature-gate-optin-builtin-traits.stderr | 4 +-- .../inherent-impl.stderr | 8 +++--- .../defaultimpl/validation.stderr | 2 +- .../syntax-trait-polarity-feature-gate.stderr | 4 +-- src/test/ui/syntax-trait-polarity.stderr | 16 +++++------ .../traits/trait-safety-inherent-impl.stderr | 10 ++----- 18 files changed, 56 insertions(+), 52 deletions(-) diff --git a/src/librustc_ast/ast.rs b/src/librustc_ast/ast.rs index 88564647d61f9..e86ec2ebfa499 100644 --- a/src/librustc_ast/ast.rs +++ b/src/librustc_ast/ast.rs @@ -2117,14 +2117,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), } } } diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 9f04c01bfa8f4..1070043458a5a 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -779,7 +779,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { defaultness: _, constness: _, generics: _, - of_trait: Some(_), + of_trait: Some(ref t), ref self_ty, items: _, } => { @@ -794,10 +794,10 @@ 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" ) @@ -816,7 +816,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { constness, generics: _, of_trait: None, - self_ty: _, + ref self_ty, items: _, } => { self.invalid_visibility( @@ -826,28 +826,36 @@ impl<'a> Visitor<'a> for AstValidator<'a> { if let Unsafe::Yes(span) = unsafety { struct_span_err!( self.session, - item.span, + vec![span, self_ty.span], E0197, "inherent impls cannot be unsafe" ) .span_label(span, "unsafe because of this") + .span_label(self_ty.span, "inherent impl for this type") .emit(); } - if polarity == ImplPolarity::Negative { - self.err_handler().span_err(item.span, "inherent impls cannot be negative"); + if let ImplPolarity::Negative(span) = polarity { + self.err_handler().span_err(span, "inherent impls cannot be negative"); } 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`") + .struct_span_err( + vec![def_span, self_ty.span], + "inherent impls cannot be `default`", + ) .span_label(def_span, "`default` because of this") + .span_label(self_ty.span, "inherent impl for this type") .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`") + .struct_span_err( + vec![span, self_ty.span], + "inherent impls cannot be `const`", + ) .span_label(span, "`const` because of this") + .span_label(self_ty.span, "inherent impl for this type") .note("only trait implementations may be annotated with `const`") .emit(); } diff --git a/src/librustc_ast_passes/feature_gate.rs b/src/librustc_ast_passes/feature_gate.rs index 05e69d0cfd74e..3d9001d5d58bb 100644 --- a/src/librustc_ast_passes/feature_gate.rs +++ b/src/librustc_ast_passes/feature_gate.rs @@ -338,14 +338,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" ); } diff --git a/src/librustc_ast_pretty/pprust.rs b/src/librustc_ast_pretty/pprust.rs index f95c154bb3b5a..dbfe5d34bc0cb 100644 --- a/src/librustc_ast_pretty/pprust.rs +++ b/src/librustc_ast_pretty/pprust.rs @@ -1175,7 +1175,7 @@ impl<'a> State<'a> { self.s.space(); } - if polarity == ast::ImplPolarity::Negative { + if let ast::ImplPolarity::Negative(_) = polarity { self.s.word("!"); } diff --git a/src/librustc_hir/print.rs b/src/librustc_hir/print.rs index 8cbbef959ce75..f03ab41f12d0a 100644 --- a/src/librustc_hir/print.rs +++ b/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("!"); } diff --git a/src/librustc_parse/parser/item.rs b/src/librustc_parse/parser/item.rs index 9bca1d0990159..09c512f32546b 100644 --- a/src/librustc_parse/parser/item.rs +++ b/src/librustc_parse/parser/item.rs @@ -414,7 +414,7 @@ impl<'a> Parser<'a> { // 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 + ast::ImplPolarity::Negative(self.prev_token.span) } else { ast::ImplPolarity::Positive }; diff --git a/src/librustc_save_analysis/sig.rs b/src/librustc_save_analysis/sig.rs index 32da62adc3c02..4fa226712b78e 100644 --- a/src/librustc_save_analysis/sig.rs +++ b/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)?; diff --git a/src/librustc_typeck/coherence/unsafety.rs b/src/librustc_typeck/coherence/unsafety.rs index a604421740363..3b25f67aacc63 100644 --- a/src/librustc_typeck/coherence/unsafety.rs +++ b/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, _) => { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 2dad3d1d6d708..e6c3d5b8b9e9e 100644 --- a/src/librustc_typeck/collect.rs +++ b/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"); } diff --git a/src/test/ui/coherence/coherence-negative-impls-safe.stderr b/src/test/ui/coherence/coherence-negative-impls-safe.stderr index 4db66af6783ca..eebd1de277ecf 100644 --- a/src/test/ui/coherence/coherence-negative-impls-safe.stderr +++ b/src/test/ui/coherence/coherence-negative-impls-safe.stderr @@ -1,8 +1,8 @@ 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 {} - | ------^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ------ ^^^^^ | | | unsafe because of this diff --git a/src/test/ui/error-codes/E0197.stderr b/src/test/ui/error-codes/E0197.stderr index 51ed9c83bc999..bc3c2857958f9 100644 --- a/src/test/ui/error-codes/E0197.stderr +++ b/src/test/ui/error-codes/E0197.stderr @@ -2,7 +2,7 @@ error[E0197]: inherent impls cannot be unsafe --> $DIR/E0197.rs:3:1 | LL | unsafe impl Foo { } - | ------^^^^^^^^^^^^^ + | ^^^^^^ ^^^ inherent impl for this type | | | unsafe because of this diff --git a/src/test/ui/error-codes/E0198.stderr b/src/test/ui/error-codes/E0198.stderr index 90e8b4abd1296..4330476870037 100644 --- a/src/test/ui/error-codes/E0198.stderr +++ b/src/test/ui/error-codes/E0198.stderr @@ -1,8 +1,8 @@ error[E0198]: negative impls cannot be unsafe - --> $DIR/E0198.rs:5:1 + --> $DIR/E0198.rs:5:13 | LL | unsafe impl !Send for Foo { } - | ------^^^^^^^^^^^^^^^^^^^^^^^ + | ------ ^^^^^ | | | unsafe because of this diff --git a/src/test/ui/feature-gates/feature-gate-optin-builtin-traits.stderr b/src/test/ui/feature-gates/feature-gate-optin-builtin-traits.stderr index d29c373a33c95..490d29ad8a35f 100644 --- a/src/test/ui/feature-gates/feature-gate-optin-builtin-traits.stderr +++ b/src/test/ui/feature-gates/feature-gate-optin-builtin-traits.stderr @@ -8,10 +8,10 @@ LL | auto trait AutoDummyTrait {} = help: add `#![feature(optin_builtin_traits)]` to the crate attributes to enable error[E0658]: negative trait bounds are not yet fully implemented; use marker types for now - --> $DIR/feature-gate-optin-builtin-traits.rs:9:1 + --> $DIR/feature-gate-optin-builtin-traits.rs:9:6 | LL | impl !AutoDummyTrait for DummyStruct {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ | = note: see issue #13231 for more information = help: add `#![feature(optin_builtin_traits)]` to the crate attributes to enable diff --git a/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr b/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr index 3ea58a3728a5d..0a76c70b97d1e 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr +++ b/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr @@ -1,18 +1,18 @@ error: inherent impls cannot be `const` - --> $DIR/inherent-impl.rs:9:1 + --> $DIR/inherent-impl.rs:9:6 | LL | impl const S {} - | ^^^^^-----^^^^^ + | ^^^^^ ^ inherent impl for this type | | | `const` because of this | = note: only trait implementations may be annotated with `const` error: inherent impls cannot be `const` - --> $DIR/inherent-impl.rs:12:1 + --> $DIR/inherent-impl.rs:12:6 | LL | impl const T {} - | ^^^^^-----^^^^^ + | ^^^^^ ^ inherent impl for this type | | | `const` because of this | diff --git a/src/test/ui/specialization/defaultimpl/validation.stderr b/src/test/ui/specialization/defaultimpl/validation.stderr index 03b1ef69ca072..2a96f41a249fa 100644 --- a/src/test/ui/specialization/defaultimpl/validation.stderr +++ b/src/test/ui/specialization/defaultimpl/validation.stderr @@ -2,7 +2,7 @@ error: inherent impls cannot be `default` --> $DIR/validation.rs:7:1 | LL | default impl S {} - | -------^^^^^^^ + | ^^^^^^^ ^ inherent impl for this type | | | `default` because of this | diff --git a/src/test/ui/syntax-trait-polarity-feature-gate.stderr b/src/test/ui/syntax-trait-polarity-feature-gate.stderr index ed76377278b82..5d4c1b354f700 100644 --- a/src/test/ui/syntax-trait-polarity-feature-gate.stderr +++ b/src/test/ui/syntax-trait-polarity-feature-gate.stderr @@ -1,8 +1,8 @@ error[E0658]: negative trait bounds are not yet fully implemented; use marker types for now - --> $DIR/syntax-trait-polarity-feature-gate.rs:7:1 + --> $DIR/syntax-trait-polarity-feature-gate.rs:7:6 | LL | impl !Send for TestType {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ | = note: see issue #13231 for more information = help: add `#![feature(optin_builtin_traits)]` to the crate attributes to enable diff --git a/src/test/ui/syntax-trait-polarity.stderr b/src/test/ui/syntax-trait-polarity.stderr index fef3a65088855..b7d5b4570aacc 100644 --- a/src/test/ui/syntax-trait-polarity.stderr +++ b/src/test/ui/syntax-trait-polarity.stderr @@ -1,28 +1,28 @@ error: inherent impls cannot be negative - --> $DIR/syntax-trait-polarity.rs:7:1 + --> $DIR/syntax-trait-polarity.rs:7:6 | LL | impl !TestType {} - | ^^^^^^^^^^^^^^^^^ + | ^ error[E0198]: negative impls cannot be unsafe - --> $DIR/syntax-trait-polarity.rs:12:1 + --> $DIR/syntax-trait-polarity.rs:12:13 | LL | unsafe impl !Send for TestType {} - | ------^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ------ ^^^^^ | | | unsafe because of this error: inherent impls cannot be negative - --> $DIR/syntax-trait-polarity.rs:19:1 + --> $DIR/syntax-trait-polarity.rs:19:9 | LL | impl !TestType2 {} - | ^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ error[E0198]: negative impls cannot be unsafe - --> $DIR/syntax-trait-polarity.rs:22:1 + --> $DIR/syntax-trait-polarity.rs:22:16 | LL | unsafe impl !Send for TestType2 {} - | ------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ------ ^^^^^ | | | unsafe because of this diff --git a/src/test/ui/traits/trait-safety-inherent-impl.stderr b/src/test/ui/traits/trait-safety-inherent-impl.stderr index c398785d3949e..09ad4585ab156 100644 --- a/src/test/ui/traits/trait-safety-inherent-impl.stderr +++ b/src/test/ui/traits/trait-safety-inherent-impl.stderr @@ -1,14 +1,10 @@ error[E0197]: inherent impls cannot be unsafe --> $DIR/trait-safety-inherent-impl.rs:5:1 | -LL | unsafe impl SomeStruct { - | ^----- - | | - | _unsafe because of this +LL | unsafe impl SomeStruct { + | ^^^^^^ ^^^^^^^^^^ inherent impl for this type | | -LL | | fn foo(self) { } -LL | | } - | |_^ + | unsafe because of this error: aborting due to previous error From 713a291441d2c71e74141ddc9387166bd6755d9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 5 Mar 2020 15:39:35 -0800 Subject: [PATCH 2/7] review comments --- src/librustc_ast_passes/ast_validation.rs | 52 +++++++++---------- src/librustc_parse/parser/item.rs | 18 ++++--- .../coherence-negative-impls-safe.stderr | 5 +- src/test/ui/error-codes/E0197.stderr | 4 +- src/test/ui/error-codes/E0198.stderr | 5 +- .../inherent-impl.stderr | 8 +-- .../defaultimpl/validation.stderr | 4 +- src/test/ui/syntax-trait-polarity.stderr | 22 +++++--- .../traits/trait-safety-inherent-impl.stderr | 4 +- 9 files changed, 65 insertions(+), 57 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 1070043458a5a..d90e9c4df269c 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -801,6 +801,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { E0198, "negative impls cannot be unsafe" ) + .span_label(sp, "negative because of this") .span_label(span, "unsafe because of this") .emit(); } @@ -819,45 +820,40 @@ impl<'a> Visitor<'a> for AstValidator<'a> { ref self_ty, items: _, } => { + let error = |annotation_span, annotation, note, code| { + 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"); + if note { + err.note(&format!( + "only trait implementations may be annotated with {}", + annotation + )); + } + if code { + err.code(error_code!(E0197)); + } + err.emit(); + }; + self.invalid_visibility( &item.vis, Some("place qualifiers on individual impl items instead"), ); if let Unsafe::Yes(span) = unsafety { - struct_span_err!( - self.session, - vec![span, self_ty.span], - E0197, - "inherent impls cannot be unsafe" - ) - .span_label(span, "unsafe because of this") - .span_label(self_ty.span, "inherent impl for this type") - .emit(); + error(span, "unsafe", false, true) } if let ImplPolarity::Negative(span) = polarity { - self.err_handler().span_err(span, "inherent impls cannot be negative"); + error(span, "negative", false, false); } if let Defaultness::Default(def_span) = defaultness { - self.err_handler() - .struct_span_err( - vec![def_span, self_ty.span], - "inherent impls cannot be `default`", - ) - .span_label(def_span, "`default` because of this") - .span_label(self_ty.span, "inherent impl for this type") - .note("only trait implementations may be annotated with `default`") - .emit(); + error(def_span, "`default`", true, false); } if let Const::Yes(span) = constness { - self.err_handler() - .struct_span_err( - vec![span, self_ty.span], - "inherent impls cannot be `const`", - ) - .span_label(span, "`const` because of this") - .span_label(self_ty.span, "inherent impl for this type") - .note("only trait implementations may be annotated with `const`") - .emit(); + error(span, "`const`", true, false); } } ItemKind::Fn(def, ref sig, ref generics, ref body) => { diff --git a/src/librustc_parse/parser/item.rs b/src/librustc_parse/parser/item.rs index 09c512f32546b..85bb546b74806 100644 --- a/src/librustc_parse/parser/item.rs +++ b/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(self.prev_token.span) - } 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)); diff --git a/src/test/ui/coherence/coherence-negative-impls-safe.stderr b/src/test/ui/coherence/coherence-negative-impls-safe.stderr index eebd1de277ecf..1bd37f395902a 100644 --- a/src/test/ui/coherence/coherence-negative-impls-safe.stderr +++ b/src/test/ui/coherence/coherence-negative-impls-safe.stderr @@ -2,8 +2,9 @@ error[E0198]: negative impls cannot be unsafe --> $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 diff --git a/src/test/ui/error-codes/E0197.stderr b/src/test/ui/error-codes/E0197.stderr index bc3c2857958f9..35e1042649ef9 100644 --- a/src/test/ui/error-codes/E0197.stderr +++ b/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 + | ------ ^^^ inherent impl for this type | | | unsafe because of this diff --git a/src/test/ui/error-codes/E0198.stderr b/src/test/ui/error-codes/E0198.stderr index 4330476870037..bb2efefb427ba 100644 --- a/src/test/ui/error-codes/E0198.stderr +++ b/src/test/ui/error-codes/E0198.stderr @@ -2,8 +2,9 @@ error[E0198]: negative impls cannot be unsafe --> $DIR/E0198.rs:5:13 | LL | unsafe impl !Send for Foo { } - | ------ ^^^^^ - | | + | ------ -^^^^ + | | | + | | negative because of this | unsafe because of this error: aborting due to previous error diff --git a/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr b/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr index 0a76c70b97d1e..834f6a409f5b6 100644 --- a/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr +++ b/src/test/ui/rfc-2632-const-trait-impl/inherent-impl.stderr @@ -1,18 +1,18 @@ error: inherent impls cannot be `const` - --> $DIR/inherent-impl.rs:9:6 + --> $DIR/inherent-impl.rs:9:12 | LL | impl const S {} - | ^^^^^ ^ inherent impl for this type + | ----- ^ inherent impl for this type | | | `const` because of this | = note: only trait implementations may be annotated with `const` error: inherent impls cannot be `const` - --> $DIR/inherent-impl.rs:12:6 + --> $DIR/inherent-impl.rs:12:12 | LL | impl const T {} - | ^^^^^ ^ inherent impl for this type + | ----- ^ inherent impl for this type | | | `const` because of this | diff --git a/src/test/ui/specialization/defaultimpl/validation.stderr b/src/test/ui/specialization/defaultimpl/validation.stderr index 2a96f41a249fa..6e19d79e48f6b 100644 --- a/src/test/ui/specialization/defaultimpl/validation.stderr +++ b/src/test/ui/specialization/defaultimpl/validation.stderr @@ -1,8 +1,8 @@ error: inherent impls cannot be `default` - --> $DIR/validation.rs:7:1 + --> $DIR/validation.rs:7:14 | LL | default impl S {} - | ^^^^^^^ ^ inherent impl for this type + | ------- ^ inherent impl for this type | | | `default` because of this | diff --git a/src/test/ui/syntax-trait-polarity.stderr b/src/test/ui/syntax-trait-polarity.stderr index b7d5b4570aacc..5777e0ade908e 100644 --- a/src/test/ui/syntax-trait-polarity.stderr +++ b/src/test/ui/syntax-trait-polarity.stderr @@ -1,29 +1,35 @@ error: inherent impls cannot be negative - --> $DIR/syntax-trait-polarity.rs:7:6 + --> $DIR/syntax-trait-polarity.rs:7:7 | LL | impl !TestType {} - | ^ + | -^^^^^^^^ inherent impl for this type + | | + | negative because of this error[E0198]: negative impls cannot be unsafe --> $DIR/syntax-trait-polarity.rs:12:13 | LL | unsafe impl !Send for TestType {} - | ------ ^^^^^ - | | + | ------ -^^^^ + | | | + | | negative because of this | unsafe because of this error: inherent impls cannot be negative - --> $DIR/syntax-trait-polarity.rs:19:9 + --> $DIR/syntax-trait-polarity.rs:19:10 | LL | impl !TestType2 {} - | ^ + | -^^^^^^^^^^^^ inherent impl for this type + | | + | negative because of this error[E0198]: negative impls cannot be unsafe --> $DIR/syntax-trait-polarity.rs:22:16 | LL | unsafe impl !Send for TestType2 {} - | ------ ^^^^^ - | | + | ------ -^^^^ + | | | + | | negative because of this | unsafe because of this error[E0192]: negative impls are only allowed for auto traits (e.g., `Send` and `Sync`) diff --git a/src/test/ui/traits/trait-safety-inherent-impl.stderr b/src/test/ui/traits/trait-safety-inherent-impl.stderr index 09ad4585ab156..0738d2973e2d7 100644 --- a/src/test/ui/traits/trait-safety-inherent-impl.stderr +++ b/src/test/ui/traits/trait-safety-inherent-impl.stderr @@ -1,8 +1,8 @@ error[E0197]: inherent impls cannot be unsafe - --> $DIR/trait-safety-inherent-impl.rs:5:1 + --> $DIR/trait-safety-inherent-impl.rs:5:13 | LL | unsafe impl SomeStruct { - | ^^^^^^ ^^^^^^^^^^ inherent impl for this type + | ------ ^^^^^^^^^^ inherent impl for this type | | | unsafe because of this From 6fba412499e854d6c4cd8e6b22073d5684d549ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 6 Mar 2020 10:55:21 -0800 Subject: [PATCH 3/7] Further tweak spans in ast validation errors --- src/librustc_ast_passes/ast_validation.rs | 59 +++++++++++++++---- src/test/ui/async-await/no-const-async.stderr | 4 +- src/test/ui/auto-trait-validation.stderr | 18 ++++-- src/test/ui/issues/issue-23080-2.rs | 3 +- src/test/ui/issues/issue-23080-2.stderr | 11 ++-- src/test/ui/issues/issue-23080.rs | 3 +- src/test/ui/issues/issue-23080.stderr | 13 ++-- .../ui/parser/fn-header-semantic-fail.stderr | 10 ++-- ...inductive-overflow-supertrait-oibit.stderr | 6 +- .../typeck-auto-trait-no-supertraits-2.stderr | 6 +- .../typeck-auto-trait-no-supertraits.stderr | 6 +- 11 files changed, 91 insertions(+), 48 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index d90e9c4df269c..eff58ad4d9aab 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -13,7 +13,7 @@ use rustc_ast::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor}; use rustc_ast::walk_list; use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashMap; -use rustc_errors::{error_code, struct_span_err, Applicability}; +use rustc_errors::{error_code, pluralize, struct_span_err, Applicability}; use rustc_parse::validate_attr; use rustc_session::lint::builtin::PATTERNS_IN_FNS_WITHOUT_BODY; use rustc_session::lint::LintBuffer; @@ -887,30 +887,63 @@ impl<'a> Visitor<'a> for AstValidator<'a> { if is_auto == IsAuto::Yes { // Auto traits cannot have generics, super traits nor contain items. if !generics.params.is_empty() { - struct_span_err!( + let spans: Vec<_> = generics.params.iter().map(|i| i.ident.span).collect(); + let last = spans.iter().last().map(|s| *s); + let len = spans.len(); + let mut err = struct_span_err!( self.session, - item.span, + spans, E0567, "auto traits cannot have generic parameters" - ) - .emit(); + ); + if let Some(span) = last { + err.span_label( + span, + &format!( + "cannot have {these} generic parameter{s}", + these = if len == 1 { "this" } else { "these" }, + s = pluralize!(len) + ), + ); + } + err.span_label( + item.ident.span, + "auto trait cannot have generic parameters", + ); + err.emit(); } if !bounds.is_empty() { - struct_span_err!( + let spans: Vec<_> = bounds.iter().map(|b| b.span()).collect(); + let last = spans.iter().last().map(|s| *s); + let len = spans.len(); + let mut err = struct_span_err!( self.session, - item.span, + spans, E0568, "auto traits cannot have super traits" - ) - .emit(); + ); + if let Some(span) = last { + err.span_label( + span, + &format!( + "cannot have {these} super trait{s}", + these = if len == 1 { "this" } else { "these" }, + s = pluralize!(len) + ), + ); + } + err.span_label(item.ident.span, "auto trait cannot have super traits"); + err.emit(); } if !trait_items.is_empty() { + let spans: Vec<_> = trait_items.iter().map(|i| i.ident.span).collect(); struct_span_err!( self.session, - item.span, + spans, E0380, "auto traits cannot have methods or associated items" ) + .span_label(item.ident.span, "auto trait cannot have items") .emit(); } } @@ -1157,9 +1190,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(); } diff --git a/src/test/ui/async-await/no-const-async.stderr b/src/test/ui/async-await/no-const-async.stderr index 07559cd240bb6..4e59bb507676f 100644 --- a/src/test/ui/async-await/no-const-async.stderr +++ b/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 diff --git a/src/test/ui/auto-trait-validation.stderr b/src/test/ui/auto-trait-validation.stderr index 51422fab81fda..75919a1f9913f 100644 --- a/src/test/ui/auto-trait-validation.stderr +++ b/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:20 | LL | auto trait Generic {} - | ^^^^^^^^^^^^^^^^^^^^^^^^ + | ------- ^ cannot have this generic parameter + | | + | 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 {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ----- ^^^^ cannot have this super trait + | | + | 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 diff --git a/src/test/ui/issues/issue-23080-2.rs b/src/test/ui/issues/issue-23080-2.rs index 319aa2a5cce9e..d20bb4bd90726 100644 --- a/src/test/ui/issues/issue-23080-2.rs +++ b/src/test/ui/issues/issue-23080-2.rs @@ -3,8 +3,7 @@ #![feature(optin_builtin_traits)] unsafe auto trait Trait { -//~^ ERROR E0380 - type Output; + type Output; //~ ERROR E0380 } fn call_method(x: T) {} diff --git a/src/test/ui/issues/issue-23080-2.stderr b/src/test/ui/issues/issue-23080-2.stderr index 1103de0d91043..fcd1ecfa98288 100644 --- a/src/test/ui/issues/issue-23080-2.stderr +++ b/src/test/ui/issues/issue-23080-2.stderr @@ -1,11 +1,10 @@ error[E0380]: auto traits cannot have methods or associated items - --> $DIR/issue-23080-2.rs:5:1 + --> $DIR/issue-23080-2.rs:6:10 | -LL | / unsafe auto trait Trait { -LL | | -LL | | type Output; -LL | | } - | |_^ +LL | unsafe auto trait Trait { + | ----- auto trait cannot have items +LL | type Output; + | ^^^^^^ error[E0275]: overflow evaluating the requirement `<() as Trait>::Output` | diff --git a/src/test/ui/issues/issue-23080.rs b/src/test/ui/issues/issue-23080.rs index fdfee6981447d..fa5c35316bc28 100644 --- a/src/test/ui/issues/issue-23080.rs +++ b/src/test/ui/issues/issue-23080.rs @@ -1,8 +1,7 @@ #![feature(optin_builtin_traits)] unsafe auto trait Trait { -//~^ ERROR E0380 - fn method(&self) { + fn method(&self) { //~ ERROR E0380 println!("Hello"); } } diff --git a/src/test/ui/issues/issue-23080.stderr b/src/test/ui/issues/issue-23080.stderr index 91c2721732426..dbb9861b5784a 100644 --- a/src/test/ui/issues/issue-23080.stderr +++ b/src/test/ui/issues/issue-23080.stderr @@ -1,13 +1,10 @@ error[E0380]: auto traits cannot have methods or associated items - --> $DIR/issue-23080.rs:3:1 + --> $DIR/issue-23080.rs:4:8 | -LL | / unsafe auto trait Trait { -LL | | -LL | | fn method(&self) { -LL | | println!("Hello"); -LL | | } -LL | | } - | |_^ +LL | unsafe auto trait Trait { + | ----- auto trait cannot have items +LL | fn method(&self) { + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/parser/fn-header-semantic-fail.stderr b/src/test/ui/parser/fn-header-semantic-fail.stderr index 1142cee9851b0..d6b36fbb71450 100644 --- a/src/test/ui/parser/fn-header-semantic-fail.stderr +++ b/src/test/ui/parser/fn-header-semantic-fail.stderr @@ -2,7 +2,7 @@ error: functions cannot be both `const` and `async` --> $DIR/fn-header-semantic-fail.rs:13:5 | LL | const async unsafe extern "C" fn ff5() {} // OK. - | -----^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^-^^^^^------------------------------ | | | | | `async` because of this | `const` because of this @@ -45,7 +45,7 @@ error: functions cannot be both `const` and `async` --> $DIR/fn-header-semantic-fail.rs:21:9 | LL | const async unsafe extern "C" fn ft5(); - | -----^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^-^^^^^---------------------------- | | | | | `async` because of this | `const` because of this @@ -88,7 +88,7 @@ error: functions cannot be both `const` and `async` --> $DIR/fn-header-semantic-fail.rs:34:9 | LL | const async unsafe extern "C" fn ft5() {} - | -----^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^-^^^^^------------------------------ | | | | | `async` because of this | `const` because of this @@ -97,7 +97,7 @@ error: functions cannot be both `const` and `async` --> $DIR/fn-header-semantic-fail.rs:46:9 | LL | const async unsafe extern "C" fn fi5() {} - | -----^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^-^^^^^------------------------------ | | | | | `async` because of this | `const` because of this @@ -160,7 +160,7 @@ error: functions cannot be both `const` and `async` --> $DIR/fn-header-semantic-fail.rs:55:9 | LL | const async unsafe extern "C" fn fe5(); - | -----^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^-^^^^^---------------------------- | | | | | `async` because of this | `const` because of this diff --git a/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr b/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr index 63182a6bd9581..03d05f4c928a6 100644 --- a/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr +++ b/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr @@ -1,8 +1,10 @@ error[E0568]: auto traits cannot have super traits - --> $DIR/traits-inductive-overflow-supertrait-oibit.rs:7:1 + --> $DIR/traits-inductive-overflow-supertrait-oibit.rs:7:19 | LL | auto trait Magic: Copy {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | ----- ^^^^ cannot have this super trait + | | + | auto trait cannot have super traits error[E0277]: the trait bound `NoClone: std::marker::Copy` is not satisfied --> $DIR/traits-inductive-overflow-supertrait-oibit.rs:15:23 diff --git a/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr b/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr index 8755bcded9d2f..45c95b191bd59 100644 --- a/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr +++ b/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr @@ -1,8 +1,10 @@ error[E0568]: auto traits cannot have super traits - --> $DIR/typeck-auto-trait-no-supertraits-2.rs:3:1 + --> $DIR/typeck-auto-trait-no-supertraits-2.rs:3:20 | LL | auto trait Magic : Sized where Option : Magic {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ----- ^^^^^ cannot have this super trait + | | + | auto trait cannot have super traits error: aborting due to previous error diff --git a/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr b/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr index 5a38883490959..094f5d7dd24f2 100644 --- a/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr +++ b/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr @@ -1,8 +1,10 @@ error[E0568]: auto traits cannot have super traits - --> $DIR/typeck-auto-trait-no-supertraits.rs:27:1 + --> $DIR/typeck-auto-trait-no-supertraits.rs:27:19 | LL | auto trait Magic: Copy {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | ----- ^^^^ cannot have this super trait + | | + | auto trait cannot have super traits error: aborting due to previous error From 005fc6eaccacd129e96e049a3342d1d539316433 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 6 Mar 2020 11:45:00 -0800 Subject: [PATCH 4/7] Suggest removal of auto trait super traits and type params --- src/librustc_ast_passes/ast_validation.rs | 14 +++++++++++++- src/test/ui/auto-trait-validation.stderr | 10 ++++++++++ ...aits-inductive-overflow-supertrait-oibit.stderr | 5 +++++ .../typeck-auto-trait-no-supertraits-2.stderr | 5 +++++ .../typeck/typeck-auto-trait-no-supertraits.stderr | 5 +++++ 5 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index eff58ad4d9aab..df229e57cf0b7 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -910,6 +910,12 @@ impl<'a> Visitor<'a> for AstValidator<'a> { item.ident.span, "auto trait cannot have generic parameters", ); + err.span_suggestion_verbose( + generics.span, + "remove the parameters for the auto trait to be valid", + String::new(), + Applicability::MachineApplicable, + ); err.emit(); } if !bounds.is_empty() { @@ -922,6 +928,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { E0568, "auto traits cannot have super traits" ); + err.span_label(item.ident.span, "auto trait cannot have super traits"); if let Some(span) = last { err.span_label( span, @@ -931,8 +938,13 @@ impl<'a> Visitor<'a> for AstValidator<'a> { s = pluralize!(len) ), ); + err.span_suggestion_verbose( + generics.span.shrink_to_hi().to(span), + "remove the super traits for the auto trait to be valid", + String::new(), + Applicability::MachineApplicable, + ); } - err.span_label(item.ident.span, "auto trait cannot have super traits"); err.emit(); } if !trait_items.is_empty() { diff --git a/src/test/ui/auto-trait-validation.stderr b/src/test/ui/auto-trait-validation.stderr index 75919a1f9913f..17d9ca71d149a 100644 --- a/src/test/ui/auto-trait-validation.stderr +++ b/src/test/ui/auto-trait-validation.stderr @@ -5,6 +5,11 @@ LL | auto trait Generic {} | ------- ^ cannot have this generic parameter | | | auto trait cannot have generic parameters + | +help: remove the parameters for the auto trait to be valid + | +LL | auto trait Generic {} + | -- error[E0568]: auto traits cannot have super traits --> $DIR/auto-trait-validation.rs:5:20 @@ -13,6 +18,11 @@ LL | auto trait Bound : Copy {} | ----- ^^^^ cannot have this super trait | | | auto trait cannot have super traits + | +help: remove the super traits for the auto trait to be valid + | +LL | auto trait Bound {} + | -- error[E0380]: auto traits cannot have methods or associated items --> $DIR/auto-trait-validation.rs:7:25 diff --git a/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr b/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr index 03d05f4c928a6..8714cfbedbbc1 100644 --- a/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr +++ b/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr @@ -5,6 +5,11 @@ LL | auto trait Magic: Copy {} | ----- ^^^^ cannot have this super trait | | | auto trait cannot have super traits + | +help: remove the super traits for the auto trait to be valid + | +LL | auto trait Magic {} + | -- error[E0277]: the trait bound `NoClone: std::marker::Copy` is not satisfied --> $DIR/traits-inductive-overflow-supertrait-oibit.rs:15:23 diff --git a/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr b/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr index 45c95b191bd59..c652a0c6df03d 100644 --- a/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr +++ b/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr @@ -5,6 +5,11 @@ LL | auto trait Magic : Sized where Option : Magic {} | ----- ^^^^^ cannot have this super trait | | | auto trait cannot have super traits + | +help: remove the super traits for the auto trait to be valid + | +LL | auto trait Magic where Option : Magic {} + | -- error: aborting due to previous error diff --git a/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr b/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr index 094f5d7dd24f2..fe6468bc71621 100644 --- a/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr +++ b/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr @@ -5,6 +5,11 @@ LL | auto trait Magic: Copy {} | ----- ^^^^ cannot have this super trait | | | auto trait cannot have super traits + | +help: remove the super traits for the auto trait to be valid + | +LL | auto trait Magic {} + | -- error: aborting due to previous error From f483032e97ba7c89f803fc6f8078f0acdd9d9b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 6 Mar 2020 11:58:52 -0800 Subject: [PATCH 5/7] review comment --- src/librustc_ast_passes/ast_validation.rs | 25 +++++++++-------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index df229e57cf0b7..24cf4556dad2f 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -820,23 +820,14 @@ impl<'a> Visitor<'a> for AstValidator<'a> { ref self_ty, items: _, } => { - let error = |annotation_span, annotation, note, code| { + 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"); - if note { - err.note(&format!( - "only trait implementations may be annotated with {}", - annotation - )); - } - if code { - err.code(error_code!(E0197)); - } - err.emit(); + err }; self.invalid_visibility( @@ -844,16 +835,20 @@ impl<'a> Visitor<'a> for AstValidator<'a> { Some("place qualifiers on individual impl items instead"), ); if let Unsafe::Yes(span) = unsafety { - error(span, "unsafe", false, true) + error(span, "unsafe").code(error_code!(E0197)).emit(); } if let ImplPolarity::Negative(span) = polarity { - error(span, "negative", false, false); + error(span, "negative").emit(); } if let Defaultness::Default(def_span) = defaultness { - error(def_span, "`default`", true, false); + error(def_span, "`default`") + .note("only trait implementations may be annotated with `default`") + .emit(); } if let Const::Yes(span) = constness { - error(span, "`const`", true, false); + error(span, "`const`") + .note("only trait implementations may be annotated with `const`") + .emit(); } } ItemKind::Fn(def, ref sig, ref generics, ref body) => { From 29be741c9c15a69487cb2eae9bb895399806731f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 10 Mar 2020 17:59:32 -0700 Subject: [PATCH 6/7] review comments --- src/librustc_ast_passes/ast_validation.rs | 53 ++++++------------- src/test/ui/auto-trait-validation.stderr | 16 ++---- ...inductive-overflow-supertrait-oibit.stderr | 7 +-- .../typeck-auto-trait-no-supertraits-2.stderr | 7 +-- .../typeck-auto-trait-no-supertraits.stderr | 7 +-- 5 files changed, 22 insertions(+), 68 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 24cf4556dad2f..668ac8047d17b 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -13,7 +13,7 @@ use rustc_ast::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor}; use rustc_ast::walk_list; use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashMap; -use rustc_errors::{error_code, pluralize, struct_span_err, Applicability}; +use rustc_errors::{error_code, struct_span_err, Applicability}; use rustc_parse::validate_attr; use rustc_session::lint::builtin::PATTERNS_IN_FNS_WITHOUT_BODY; use rustc_session::lint::LintBuffer; @@ -882,64 +882,43 @@ impl<'a> Visitor<'a> for AstValidator<'a> { if is_auto == IsAuto::Yes { // Auto traits cannot have generics, super traits nor contain items. if !generics.params.is_empty() { - let spans: Vec<_> = generics.params.iter().map(|i| i.ident.span).collect(); - let last = spans.iter().last().map(|s| *s); - let len = spans.len(); let mut err = struct_span_err!( self.session, - spans, + generics.span, E0567, "auto traits cannot have generic parameters" ); - if let Some(span) = last { - err.span_label( - span, - &format!( - "cannot have {these} generic parameter{s}", - these = if len == 1 { "this" } else { "these" }, - s = pluralize!(len) - ), - ); - } err.span_label( item.ident.span, "auto trait cannot have generic parameters", ); - err.span_suggestion_verbose( + err.span_suggestion( generics.span, - "remove the parameters for the auto trait to be valid", + "remove the parameters", String::new(), Applicability::MachineApplicable, ); err.emit(); } if !bounds.is_empty() { - let spans: Vec<_> = bounds.iter().map(|b| b.span()).collect(); - let last = spans.iter().last().map(|s| *s); - let len = spans.len(); + let span = match &bounds[..] { + [] => unreachable!(), + [single] => single.span(), + [first, .., last] => first.span().to(last.span()), + }; let mut err = struct_span_err!( self.session, - spans, + span, E0568, "auto traits cannot have super traits" ); err.span_label(item.ident.span, "auto trait cannot have super traits"); - if let Some(span) = last { - err.span_label( - span, - &format!( - "cannot have {these} super trait{s}", - these = if len == 1 { "this" } else { "these" }, - s = pluralize!(len) - ), - ); - err.span_suggestion_verbose( - generics.span.shrink_to_hi().to(span), - "remove the super traits for the auto trait to be valid", - String::new(), - Applicability::MachineApplicable, - ); - } + err.span_suggestion( + span, + "remove the super traits", + String::new(), + Applicability::MachineApplicable, + ); err.emit(); } if !trait_items.is_empty() { diff --git a/src/test/ui/auto-trait-validation.stderr b/src/test/ui/auto-trait-validation.stderr index 17d9ca71d149a..4040e66c6af77 100644 --- a/src/test/ui/auto-trait-validation.stderr +++ b/src/test/ui/auto-trait-validation.stderr @@ -1,28 +1,18 @@ error[E0567]: auto traits cannot have generic parameters - --> $DIR/auto-trait-validation.rs:3:20 + --> $DIR/auto-trait-validation.rs:3:19 | LL | auto trait Generic {} - | ------- ^ cannot have this generic parameter + | -------^^^ help: remove the parameters | | | auto trait cannot have generic parameters - | -help: remove the parameters for the auto trait to be valid - | -LL | auto trait Generic {} - | -- error[E0568]: auto traits cannot have super traits --> $DIR/auto-trait-validation.rs:5:20 | LL | auto trait Bound : Copy {} - | ----- ^^^^ cannot have this super trait + | ----- ^^^^ help: remove the super traits | | | auto trait cannot have super traits - | -help: remove the super traits for the auto trait to be valid - | -LL | auto trait Bound {} - | -- error[E0380]: auto traits cannot have methods or associated items --> $DIR/auto-trait-validation.rs:7:25 diff --git a/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr b/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr index 8714cfbedbbc1..a83ff3701511d 100644 --- a/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr +++ b/src/test/ui/traits/traits-inductive-overflow-supertrait-oibit.stderr @@ -2,14 +2,9 @@ error[E0568]: auto traits cannot have super traits --> $DIR/traits-inductive-overflow-supertrait-oibit.rs:7:19 | LL | auto trait Magic: Copy {} - | ----- ^^^^ cannot have this super trait + | ----- ^^^^ help: remove the super traits | | | auto trait cannot have super traits - | -help: remove the super traits for the auto trait to be valid - | -LL | auto trait Magic {} - | -- error[E0277]: the trait bound `NoClone: std::marker::Copy` is not satisfied --> $DIR/traits-inductive-overflow-supertrait-oibit.rs:15:23 diff --git a/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr b/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr index c652a0c6df03d..e397629327754 100644 --- a/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr +++ b/src/test/ui/typeck/typeck-auto-trait-no-supertraits-2.stderr @@ -2,14 +2,9 @@ error[E0568]: auto traits cannot have super traits --> $DIR/typeck-auto-trait-no-supertraits-2.rs:3:20 | LL | auto trait Magic : Sized where Option : Magic {} - | ----- ^^^^^ cannot have this super trait + | ----- ^^^^^ help: remove the super traits | | | auto trait cannot have super traits - | -help: remove the super traits for the auto trait to be valid - | -LL | auto trait Magic where Option : Magic {} - | -- error: aborting due to previous error diff --git a/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr b/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr index fe6468bc71621..b1602e3642ecb 100644 --- a/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr +++ b/src/test/ui/typeck/typeck-auto-trait-no-supertraits.stderr @@ -2,14 +2,9 @@ error[E0568]: auto traits cannot have super traits --> $DIR/typeck-auto-trait-no-supertraits.rs:27:19 | LL | auto trait Magic: Copy {} - | ----- ^^^^ cannot have this super trait + | ----- ^^^^ help: remove the super traits | | | auto trait cannot have super traits - | -help: remove the super traits for the auto trait to be valid - | -LL | auto trait Magic {} - | -- error: aborting due to previous error From 7ee1b470920d2ff4d6ffb100a5bbcf594b9031a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 11 Mar 2020 09:17:55 -0700 Subject: [PATCH 7/7] review comments --- src/librustc_ast_passes/ast_validation.rs | 103 +++++++++++----------- src/librustc_ast_passes/lib.rs | 1 + 2 files changed, 53 insertions(+), 51 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 668ac8047d17b..2a39dc6b011e7 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/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], 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>( @@ -881,57 +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() { - let mut err = struct_span_err!( - self.session, - generics.span, - E0567, - "auto traits cannot have generic parameters" - ); - err.span_label( - item.ident.span, - "auto trait cannot have generic parameters", - ); - err.span_suggestion( - generics.span, - "remove the parameters", - String::new(), - Applicability::MachineApplicable, - ); - err.emit(); - } - if !bounds.is_empty() { - let span = match &bounds[..] { - [] => unreachable!(), - [single] => single.span(), - [first, .., last] => first.span().to(last.span()), - }; - let mut err = struct_span_err!( - self.session, - span, - E0568, - "auto traits cannot have super traits" - ); - err.span_label(item.ident.span, "auto trait cannot have super traits"); - err.span_suggestion( - span, - "remove the super traits", - String::new(), - Applicability::MachineApplicable, - ); - err.emit(); - } - 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(item.ident.span, "auto trait cannot have 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); diff --git a/src/librustc_ast_passes/lib.rs b/src/librustc_ast_passes/lib.rs index 520a7ac3e5665..10081d36754ba 100644 --- a/src/librustc_ast_passes/lib.rs +++ b/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`.