Skip to content

Commit

Permalink
Rollup merge of rust-lang#69722 - estebank:negative-impl-span-ast, r=…
Browse files Browse the repository at this point in the history
…Centril

Tweak output for invalid negative impl AST errors

Use more accurate spans for negative `impl` errors.

r? @Centril
  • Loading branch information
Centril authored Mar 11, 2020
2 parents 9381b8b + 7ee1b47 commit 3681df6
Show file tree
Hide file tree
Showing 29 changed files with 184 additions and 141 deletions.
4 changes: 2 additions & 2 deletions src/librustc_ast/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
}
Expand Down
126 changes: 77 additions & 49 deletions src/librustc_ast_passes/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>(
Expand Down Expand Up @@ -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: _,
} => {
Expand All @@ -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();
}
Expand All @@ -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();
}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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();
}

Expand Down
8 changes: 4 additions & 4 deletions src/librustc_ast_passes/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}

Expand Down
1 change: 1 addition & 0 deletions src/librustc_ast_passes/lib.rs
Original file line number Diff line number Diff line change
@@ -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`.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_ast_pretty/pprust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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("!");
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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("!");
}

Expand Down
18 changes: 11 additions & 7 deletions src/librustc_parse/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
/// ```
Expand Down Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_save_analysis/sig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_typeck/coherence/unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, _) => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/async-await/no-const-async.stderr
Original file line number Diff line number Diff line change
@@ -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
Expand Down
18 changes: 12 additions & 6 deletions src/test/ui/auto-trait-validation.stderr
Original file line number Diff line number Diff line change
@@ -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

Expand Down
7 changes: 4 additions & 3 deletions src/test/ui/coherence/coherence-negative-impls-safe.stderr
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/error-codes/E0197.stderr
Original file line number Diff line number Diff line change
@@ -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

Expand Down
Loading

0 comments on commit 3681df6

Please sign in to comment.