Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encode ; stmt without expr as StmtKind::Empty #69506

Merged
merged 1 commit into from
Mar 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/librustc_ast/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,8 @@ pub enum StmtKind {
Expr(P<Expr>),
/// Expr with a trailing semi-colon.
Semi(P<Expr>),
/// Just a trailing semi-colon.
Empty,
/// Macro.
Mac(P<(Mac, MacStmtStyle, AttrVec)>),
}
Expand Down
7 changes: 3 additions & 4 deletions src/librustc_ast/attr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -674,8 +674,8 @@ impl HasAttrs for StmtKind {
fn attrs(&self) -> &[Attribute] {
match *self {
StmtKind::Local(ref local) => local.attrs(),
StmtKind::Item(..) => &[],
StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => expr.attrs(),
StmtKind::Empty | StmtKind::Item(..) => &[],
StmtKind::Mac(ref mac) => {
let (_, _, ref attrs) = **mac;
attrs.attrs()
Expand All @@ -686,9 +686,8 @@ impl HasAttrs for StmtKind {
fn visit_attrs(&mut self, f: impl FnOnce(&mut Vec<Attribute>)) {
match self {
StmtKind::Local(local) => local.visit_attrs(f),
StmtKind::Item(..) => {}
StmtKind::Expr(expr) => expr.visit_attrs(f),
StmtKind::Semi(expr) => expr.visit_attrs(f),
StmtKind::Expr(expr) | StmtKind::Semi(expr) => expr.visit_attrs(f),
StmtKind::Empty | StmtKind::Item(..) => {}
StmtKind::Mac(mac) => {
let (_mac, _style, attrs) = mac.deref_mut();
attrs.visit_attrs(f);
Expand Down
1 change: 1 addition & 0 deletions src/librustc_ast/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1265,6 +1265,7 @@ pub fn noop_flat_map_stmt_kind<T: MutVisitor>(
StmtKind::Item(item) => vis.flat_map_item(item).into_iter().map(StmtKind::Item).collect(),
StmtKind::Expr(expr) => vis.filter_map_expr(expr).into_iter().map(StmtKind::Expr).collect(),
StmtKind::Semi(expr) => vis.filter_map_expr(expr).into_iter().map(StmtKind::Semi).collect(),
StmtKind::Empty => smallvec![StmtKind::Empty],
StmtKind::Mac(mut mac) => {
let (mac_, _semi, attrs) = mac.deref_mut();
vis.visit_mac(mac_);
Expand Down
5 changes: 2 additions & 3 deletions src/librustc_ast/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,9 +669,8 @@ pub fn walk_stmt<'a, V: Visitor<'a>>(visitor: &mut V, statement: &'a Stmt) {
match statement.kind {
StmtKind::Local(ref local) => visitor.visit_local(local),
StmtKind::Item(ref item) => visitor.visit_item(item),
StmtKind::Expr(ref expression) | StmtKind::Semi(ref expression) => {
visitor.visit_expr(expression)
}
StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => visitor.visit_expr(expr),
StmtKind::Empty => {}
StmtKind::Mac(ref mac) => {
let (ref mac, _, ref attrs) = **mac;
visitor.visit_mac(mac);
Expand Down
1 change: 1 addition & 0 deletions src/librustc_ast_lowering/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2281,6 +2281,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}
StmtKind::Expr(ref e) => hir::StmtKind::Expr(self.lower_expr(e)),
StmtKind::Semi(ref e) => hir::StmtKind::Semi(self.lower_expr(e)),
StmtKind::Empty => return smallvec![],
StmtKind::Mac(..) => panic!("shouldn't exist here"),
};
smallvec![hir::Stmt { hir_id: self.lower_node_id(s.id), kind, span: s.span }]
Expand Down
20 changes: 7 additions & 13 deletions src/librustc_ast_pretty/pprust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1446,19 +1446,13 @@ impl<'a> State<'a> {
}
}
ast::StmtKind::Semi(ref expr) => {
match expr.kind {
// Filter out empty `Tup` exprs created for the `redundant_semicolon`
// lint, as they shouldn't be visible and interact poorly
// with proc macros.
ast::ExprKind::Tup(ref exprs) if exprs.is_empty() && expr.attrs.is_empty() => {
()
}
_ => {
self.space_if_not_bol();
self.print_expr_outer_attr_style(expr, false);
self.s.word(";");
}
}
self.space_if_not_bol();
self.print_expr_outer_attr_style(expr, false);
self.s.word(";");
}
ast::StmtKind::Empty => {
self.space_if_not_bol();
self.s.word(";");
}
ast::StmtKind::Mac(ref mac) => {
let (ref mac, style, ref attrs) = **mac;
Expand Down
5 changes: 4 additions & 1 deletion src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,10 @@ impl EarlyLintPass for UnusedDocComment {
ast::StmtKind::Local(..) => "statements",
ast::StmtKind::Item(..) => "inner items",
// expressions will be reported by `check_expr`.
ast::StmtKind::Semi(..) | ast::StmtKind::Expr(..) | ast::StmtKind::Mac(..) => return,
ast::StmtKind::Empty
| ast::StmtKind::Semi(_)
| ast::StmtKind::Expr(_)
| ast::StmtKind::Mac(_) => return,
};

warn_if_doc(cx, stmt.span, kind, stmt.kind.attrs());
Expand Down
6 changes: 4 additions & 2 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ macro_rules! early_lint_passes {
WhileTrue: WhileTrue,
NonAsciiIdents: NonAsciiIdents,
IncompleteFeatures: IncompleteFeatures,
RedundantSemicolon: RedundantSemicolon,
RedundantSemicolons: RedundantSemicolons,
UnusedDocComment: UnusedDocComment,
]
);
Expand Down Expand Up @@ -274,7 +274,8 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
UNUSED_EXTERN_CRATES,
UNUSED_FEATURES,
UNUSED_LABELS,
UNUSED_PARENS
UNUSED_PARENS,
REDUNDANT_SEMICOLONS
);

add_lint_group!(
Expand Down Expand Up @@ -307,6 +308,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
store.register_renamed("unused_doc_comment", "unused_doc_comments");
store.register_renamed("async_idents", "keyword_idents");
store.register_renamed("exceeding_bitshifts", "arithmetic_overflow");
store.register_renamed("redundant_semicolon", "redundant_semicolons");
store.register_removed("unknown_features", "replaced by an error");
store.register_removed("unsigned_negation", "replaced by negate_unsigned feature gate");
store.register_removed("negate_unsigned", "cast a signed value instead");
Expand Down
65 changes: 28 additions & 37 deletions src/librustc_lint/redundant_semicolon.rs
Original file line number Diff line number Diff line change
@@ -1,50 +1,41 @@
use crate::{EarlyContext, EarlyLintPass, LintContext};
use rustc_ast::ast::{ExprKind, Stmt, StmtKind};
use rustc_ast::ast::{Block, StmtKind};
use rustc_errors::Applicability;
use rustc_span::Span;

declare_lint! {
pub REDUNDANT_SEMICOLON,
pub REDUNDANT_SEMICOLONS,
Warn,
"detects unnecessary trailing semicolons"
}

declare_lint_pass!(RedundantSemicolon => [REDUNDANT_SEMICOLON]);
declare_lint_pass!(RedundantSemicolons => [REDUNDANT_SEMICOLONS]);

impl EarlyLintPass for RedundantSemicolon {
fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &Stmt) {
if let StmtKind::Semi(expr) = &stmt.kind {
if let ExprKind::Tup(ref v) = &expr.kind {
if v.is_empty() {
// Strings of excess semicolons are encoded as empty tuple expressions
// during the parsing stage, so we check for empty tuple expressions
// which span only semicolons
if let Ok(source_str) = cx.sess().source_map().span_to_snippet(stmt.span) {
if source_str.chars().all(|c| c == ';') {
let multiple = (stmt.span.hi() - stmt.span.lo()).0 > 1;
let msg = if multiple {
"unnecessary trailing semicolons"
} else {
"unnecessary trailing semicolon"
};
cx.struct_span_lint(REDUNDANT_SEMICOLON, stmt.span, |lint| {
let mut err = lint.build(&msg);
let suggest_msg = if multiple {
"remove these semicolons"
} else {
"remove this semicolon"
};
err.span_suggestion(
stmt.span,
&suggest_msg,
String::new(),
Applicability::MaybeIncorrect,
);
err.emit();
});
}
}
}
impl EarlyLintPass for RedundantSemicolons {
fn check_block(&mut self, cx: &EarlyContext<'_>, block: &Block) {
let mut seq = None;
for stmt in block.stmts.iter() {
match (&stmt.kind, &mut seq) {
(StmtKind::Empty, None) => seq = Some((stmt.span, false)),
(StmtKind::Empty, Some(seq)) => *seq = (seq.0.to(stmt.span), true),
(_, seq) => maybe_lint_redundant_semis(cx, seq),
}
}
maybe_lint_redundant_semis(cx, &mut seq);
}
}

fn maybe_lint_redundant_semis(cx: &EarlyContext<'_>, seq: &mut Option<(Span, bool)>) {
if let Some((span, multiple)) = seq.take() {
cx.struct_span_lint(REDUNDANT_SEMICOLONS, span, |lint| {
let (msg, rem) = if multiple {
("unnecessary trailing semicolons", "remove these semicolons")
} else {
("unnecessary trailing semicolon", "remove this semicolon")
};
lint.build(msg)
.span_suggestion(span, rem, String::new(), Applicability::MaybeIncorrect)
.emit();
});
}
}
27 changes: 7 additions & 20 deletions src/librustc_parse/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,23 +59,10 @@ impl<'a> Parser<'a> {
} else if let Some(item) = self.parse_stmt_item(attrs.clone())? {
// FIXME: Bad copy of attrs
self.mk_stmt(lo.to(item.span), StmtKind::Item(P(item)))
} else if self.token == token::Semi {
} else if self.eat(&token::Semi) {
// Do not attempt to parse an expression if we're done here.
self.error_outer_attrs(&attrs);
self.bump();
let mut last_semi = lo;
while self.token == token::Semi {
last_semi = self.token.span;
self.bump();
}
// We are encoding a string of semicolons as an an empty tuple that spans
// the excess semicolons to preserve this info until the lint stage.
let kind = StmtKind::Semi(self.mk_expr(
lo.to(last_semi),
ExprKind::Tup(Vec::new()),
AttrVec::new(),
));
self.mk_stmt(lo.to(last_semi), kind)
self.mk_stmt(lo, StmtKind::Empty)
} else if self.token != token::CloseDelim(token::Brace) {
// Remainder are line-expr stmts.
let e = self.parse_expr_res(Restrictions::STMT_EXPR, Some(attrs.into()))?;
Expand Down Expand Up @@ -144,12 +131,11 @@ impl<'a> Parser<'a> {
/// Error on outer attributes in this context.
/// Also error if the previous token was a doc comment.
fn error_outer_attrs(&self, attrs: &[Attribute]) {
if !attrs.is_empty() {
if matches!(self.prev_token.kind, TokenKind::DocComment(..)) {
self.span_fatal_err(self.prev_token.span, Error::UselessDocComment).emit();
if let [.., last] = attrs {
if last.is_doc_comment() {
self.span_fatal_err(last.span, Error::UselessDocComment).emit();
} else if attrs.iter().any(|a| a.style == AttrStyle::Outer) {
self.struct_span_err(self.token.span, "expected statement after outer attribute")
.emit();
self.struct_span_err(last.span, "expected statement after outer attribute").emit();
}
}
}
Expand Down Expand Up @@ -401,6 +387,7 @@ impl<'a> Parser<'a> {
self.expect_semi()?;
eat_semi = false;
}
StmtKind::Empty => eat_semi = false,
_ => {}
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/block-expr-precedence.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ warning: unnecessary trailing semicolons
LL | if (true) { 12; };;; -num;
| ^^ help: remove these semicolons
|
= note: `#[warn(redundant_semicolon)]` on by default
= note: `#[warn(redundant_semicolons)]` on by default

1 change: 1 addition & 0 deletions src/test/ui/consts/const_let_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ struct Bar<T> { x: T }
struct W(u32);
struct A { a: u32 }

#[allow(redundant_semicolons)]
Centril marked this conversation as resolved.
Show resolved Hide resolved
const fn basics((a,): (u32,)) -> u32 {
// Deferred assignment:
let b: u32;
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/consts/const_let_eq_float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ struct Bar<T> { x: T }
struct W(f32);
struct A { a: f32 }

#[allow(redundant_semicolons)]
const fn basics((a,): (f32,)) -> f32 {
// Deferred assignment:
let b: f32;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// aux-build:redundant-semi-proc-macro-def.rs

#![deny(redundant_semicolon)]
#![deny(redundant_semicolons)]
extern crate redundant_semi_proc_macro;
use redundant_semi_proc_macro::should_preserve_spans;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
TokenStream [Ident { ident: "fn", span: #0 bytes(197..199) }, Ident { ident: "span_preservation", span: #0 bytes(200..217) }, Group { delimiter: Parenthesis, stream: TokenStream [], span: #0 bytes(217..219) }, Group { delimiter: Brace, stream: TokenStream [Ident { ident: "let", span: #0 bytes(227..230) }, Ident { ident: "tst", span: #0 bytes(231..234) }, Punct { ch: '=', spacing: Alone, span: #0 bytes(235..236) }, Literal { lit: Lit { kind: Integer, symbol: "123", suffix: None }, span: Span { lo: BytePos(237), hi: BytePos(240), ctxt: #0 } }, Punct { ch: ';', spacing: Joint, span: #0 bytes(240..241) }, Punct { ch: ';', spacing: Alone, span: #0 bytes(241..242) }, Ident { ident: "match", span: #0 bytes(288..293) }, Ident { ident: "tst", span: #0 bytes(294..297) }, Group { delimiter: Brace, stream: TokenStream [Literal { lit: Lit { kind: Integer, symbol: "123", suffix: None }, span: Span { lo: BytePos(482), hi: BytePos(485), ctxt: #0 } }, Punct { ch: '=', spacing: Joint, span: #0 bytes(486..488) }, Punct { ch: '>', spacing: Alone, span: #0 bytes(486..488) }, Group { delimiter: Parenthesis, stream: TokenStream [], span: #0 bytes(489..491) }, Punct { ch: ',', spacing: Alone, span: #0 bytes(491..492) }, Ident { ident: "_", span: #0 bytes(501..502) }, Punct { ch: '=', spacing: Joint, span: #0 bytes(503..505) }, Punct { ch: '>', spacing: Alone, span: #0 bytes(503..505) }, Group { delimiter: Parenthesis, stream: TokenStream [], span: #0 bytes(506..508) }], span: #0 bytes(298..514) }, Punct { ch: ';', spacing: Joint, span: #0 bytes(514..515) }, Punct { ch: ';', spacing: Joint, span: #0 bytes(515..516) }, Punct { ch: ';', spacing: Alone, span: #0 bytes(516..517) }], span: #0 bytes(221..561) }]
TokenStream [Ident { ident: "fn", span: #0 bytes(198..200) }, Ident { ident: "span_preservation", span: #0 bytes(201..218) }, Group { delimiter: Parenthesis, stream: TokenStream [], span: #0 bytes(218..220) }, Group { delimiter: Brace, stream: TokenStream [Ident { ident: "let", span: #0 bytes(228..231) }, Ident { ident: "tst", span: #0 bytes(232..235) }, Punct { ch: '=', spacing: Alone, span: #0 bytes(236..237) }, Literal { lit: Lit { kind: Integer, symbol: "123", suffix: None }, span: Span { lo: BytePos(238), hi: BytePos(241), ctxt: #0 } }, Punct { ch: ';', spacing: Joint, span: #0 bytes(241..242) }, Punct { ch: ';', spacing: Alone, span: #0 bytes(242..243) }, Ident { ident: "match", span: #0 bytes(289..294) }, Ident { ident: "tst", span: #0 bytes(295..298) }, Group { delimiter: Brace, stream: TokenStream [Literal { lit: Lit { kind: Integer, symbol: "123", suffix: None }, span: Span { lo: BytePos(483), hi: BytePos(486), ctxt: #0 } }, Punct { ch: '=', spacing: Joint, span: #0 bytes(487..489) }, Punct { ch: '>', spacing: Alone, span: #0 bytes(487..489) }, Group { delimiter: Parenthesis, stream: TokenStream [], span: #0 bytes(490..492) }, Punct { ch: ',', spacing: Alone, span: #0 bytes(492..493) }, Ident { ident: "_", span: #0 bytes(502..503) }, Punct { ch: '=', spacing: Joint, span: #0 bytes(504..506) }, Punct { ch: '>', spacing: Alone, span: #0 bytes(504..506) }, Group { delimiter: Parenthesis, stream: TokenStream [], span: #0 bytes(507..509) }], span: #0 bytes(299..515) }, Punct { ch: ';', spacing: Joint, span: #0 bytes(515..516) }, Punct { ch: ';', spacing: Joint, span: #0 bytes(516..517) }, Punct { ch: ';', spacing: Alone, span: #0 bytes(517..518) }], span: #0 bytes(222..562) }]
error: unnecessary trailing semicolon
--> $DIR/redundant-semi-proc-macro.rs:9:19
|
Expand All @@ -8,8 +8,8 @@ LL | let tst = 123;;
note: the lint level is defined here
--> $DIR/redundant-semi-proc-macro.rs:3:9
|
LL | #![deny(redundant_semicolon)]
| ^^^^^^^^^^^^^^^^^^^
LL | #![deny(redundant_semicolons)]
| ^^^^^^^^^^^^^^^^^^^^

error: unnecessary trailing semicolons
--> $DIR/redundant-semi-proc-macro.rs:16:7
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/parser/attr-dangling-in-fn.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: expected statement after outer attribute
--> $DIR/attr-dangling-in-fn.rs:5:1
--> $DIR/attr-dangling-in-fn.rs:4:3
|
LL | }
| ^
LL | #[foo = "bar"]
| ^^^^^^^^^^^^^^

error: aborting due to previous error

8 changes: 4 additions & 4 deletions src/test/ui/parser/attr-stmt-expr-attr-bad.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -411,16 +411,16 @@ LL | #[cfg(FALSE)] fn e() { let _ = x.#[attr]foo(); }
| ^ expected one of `.`, `;`, `?`, or an operator

error: expected statement after outer attribute
--> $DIR/attr-stmt-expr-attr-bad.rs:114:44
--> $DIR/attr-stmt-expr-attr-bad.rs:114:37
|
LL | #[cfg(FALSE)] fn e() { { fn foo() { #[attr]; } } }
| ^
| ^^^^^^^

error: expected statement after outer attribute
--> $DIR/attr-stmt-expr-attr-bad.rs:116:45
--> $DIR/attr-stmt-expr-attr-bad.rs:116:37
|
LL | #[cfg(FALSE)] fn e() { { fn foo() { #[attr] } } }
| ^
| ^^^^^^^

error: aborting due to 57 previous errors

Expand Down
2 changes: 0 additions & 2 deletions src/test/ui/parser/doc-before-semi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,4 @@ fn main() {
//~^ ERROR found a documentation comment that doesn't document anything
//~| HELP maybe a comment was intended
;
//~^ WARNING unnecessary trailing semicolon
//~| HELP remove this semicolon
}
8 changes: 0 additions & 8 deletions src/test/ui/parser/doc-before-semi.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,6 @@ LL | /// hi
|
= help: doc comments must come before what they document, maybe a comment was intended with `//`?

warning: unnecessary trailing semicolon
--> $DIR/doc-before-semi.rs:5:5
|
LL | ;
| ^ help: remove this semicolon
|
= note: `#[warn(redundant_semicolon)]` on by default

error: aborting due to previous error

For more information about this error, try `rustc --explain E0585`.