Auto merge of #69506 - Centril:stmt-semi-none, r=<try>

encode `;` stmt without expr as `StmtKind::Semi(None)` Instead of encoding `;` statements without a an expression as a tuple in AST, we modify `ast::StmtKind::Semi` to accept an `Option<_>` and then encode the `;` with `StmtKind::Semi(None)`. r? @petrochenkov
rust-lang · Feb 27, 2020 · 1d82a25 · 1d82a25
2 parents 0c15adc + fcea184
commit 1d82a25
Show file tree

Hide file tree

Showing 13 changed files with 59 additions and 88 deletions.
diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs
@@ -1267,7 +1267,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         )
     }
 
-    fn expr_unit(&mut self, sp: Span) -> &'hir hir::Expr<'hir> {
+    pub(super) fn expr_unit(&mut self, sp: Span) -> &'hir hir::Expr<'hir> {
         self.arena.alloc(self.expr(sp, hir::ExprKind::Tup(&[]), ThinVec::new()))
     }
 

diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs
@@ -2280,7 +2280,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     .collect();
             }
             StmtKind::Expr(ref e) => hir::StmtKind::Expr(self.lower_expr(e)),
-            StmtKind::Semi(ref e) => hir::StmtKind::Semi(self.lower_expr(e)),
+            StmtKind::Semi(Some(ref e)) => hir::StmtKind::Semi(self.lower_expr(e)),
+            StmtKind::Semi(None) => hir::StmtKind::Semi(self.expr_unit(s.span)),
             StmtKind::Mac(..) => panic!("shouldn't exist here"),
         };
         smallvec![hir::Stmt { hir_id: self.lower_node_id(s.id), kind, span: s.span }]

diff --git a/src/librustc_ast_pretty/pprust.rs b/src/librustc_ast_pretty/pprust.rs
@@ -1446,19 +1446,11 @@ 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();
+                if let Some(expr) = expr {
+                    self.print_expr_outer_attr_style(expr, false);
                 }
+                self.s.word(";");
             }
             ast::StmtKind::Mac(ref mac) => {
                 let (ref mac, style, ref attrs) = **mac;

diff --git a/src/librustc_lint/redundant_semicolon.rs b/src/librustc_lint/redundant_semicolon.rs
@@ -1,6 +1,7 @@
 use crate::{EarlyContext, EarlyLintPass, LintContext};
 use rustc_errors::Applicability;
-use syntax::ast::{ExprKind, Stmt, StmtKind};
+use rustc_span::Span;
+use syntax::ast::{Block, StmtKind};
 
 declare_lint! {
     pub REDUNDANT_SEMICOLON,
@@ -11,40 +12,30 @@ declare_lint! {
 declare_lint_pass!(RedundantSemicolon => [REDUNDANT_SEMICOLON]);
 
 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();
-                            });
-                        }
-                    }
-                }
+    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::Semi(None), None) => seq = Some((stmt.span, false)),
+                (StmtKind::Semi(None), 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_SEMICOLON, 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();
+        });
     }
 }
diff --git a/src/librustc_parse/parser/stmt.rs b/src/librustc_parse/parser/stmt.rs
@@ -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::Semi(None))
         } 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()))?;
@@ -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_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();
             }
         }
     }
@@ -401,6 +387,7 @@ impl<'a> Parser<'a> {
                 self.expect_semi()?;
                 eat_semi = false;
             }
+            StmtKind::Semi(None) => eat_semi = false,
             _ => {}
         }
 

diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
@@ -879,7 +879,7 @@ pub struct Stmt {
 impl Stmt {
     pub fn add_trailing_semicolon(mut self) -> Self {
         self.kind = match self.kind {
-            StmtKind::Expr(expr) => StmtKind::Semi(expr),
+            StmtKind::Expr(expr) => StmtKind::Semi(Some(expr)),
             StmtKind::Mac(mac) => {
                 StmtKind::Mac(mac.map(|(mac, _style, attrs)| (mac, MacStmtStyle::Semicolon, attrs)))
             }
@@ -911,8 +911,8 @@ pub enum StmtKind {
     Item(P<Item>),
     /// Expr without trailing semi-colon.
     Expr(P<Expr>),
-    /// Expr with a trailing semi-colon.
-    Semi(P<Expr>),
+    /// Expr with a trailing semi-colon or just a trailing semi-colon.
+    Semi(Option<P<Expr>>),
     /// Macro.
     Mac(P<(Mac, MacStmtStyle, AttrVec)>),
 }
@@ -1022,7 +1022,7 @@ impl Expr {
             match block.stmts.last().map(|last_stmt| &last_stmt.kind) {
                 // Implicit return
                 Some(&StmtKind::Expr(_)) => true,
-                Some(&StmtKind::Semi(ref expr)) => {
+                Some(&StmtKind::Semi(Some(ref expr))) => {
                     if let ExprKind::Ret(_) = expr.kind {
                         // Last statement is explicit return.
                         true

diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs
@@ -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::Expr(ref expr) | StmtKind::Semi(Some(ref expr)) => expr.attrs(),
+            StmtKind::Semi(None) | StmtKind::Item(..) => &[],
             StmtKind::Mac(ref mac) => {
                 let (_, _, ref attrs) = **mac;
                 attrs.attrs()
@@ -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(Some(expr)) => expr.visit_attrs(f),
+            StmtKind::Semi(None) | StmtKind::Item(..) => {}
             StmtKind::Mac(mac) => {
                 let (_mac, _style, attrs) = mac.deref_mut();
                 attrs.visit_attrs(f);

diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs
@@ -1264,7 +1264,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::Semi(e) => smallvec![StmtKind::Semi(e.and_then(|e| vis.filter_map_expr(e)))],
         StmtKind::Mac(mut mac) => {
             let (mac_, _semi, attrs) = mac.deref_mut();
             vis.visit_mac(mac_);

diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs
@@ -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(Some(ref expr)) => visitor.visit_expr(expr),
+        StmtKind::Semi(None) => {}
         StmtKind::Mac(ref mac) => {
             let (ref mac, _, ref attrs) = **mac;
             visitor.visit_mac(mac);

diff --git a/src/test/ui/consts/const_let_eq.rs b/src/test/ui/consts/const_let_eq.rs
@@ -5,6 +5,7 @@ struct Bar<T> { x: T }
 struct W(u32);
 struct A { a: u32 }
 
+#[allow(redundant_semicolon)]
 const fn basics((a,): (u32,)) -> u32 {
     // Deferred assignment:
     let b: u32;

diff --git a/src/test/ui/consts/const_let_eq_float.rs b/src/test/ui/consts/const_let_eq_float.rs
@@ -7,6 +7,7 @@ struct Bar<T> { x: T }
 struct W(f32);
 struct A { a: f32 }
 
+#[allow(redundant_semicolon)]
 const fn basics((a,): (f32,)) -> f32 {
     // Deferred assignment:
     let b: f32;

diff --git a/src/test/ui/parser/attr-dangling-in-fn.stderr b/src/test/ui/parser/attr-dangling-in-fn.stderr
@@ -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
 
diff --git a/src/test/ui/parser/attr-stmt-expr-attr-bad.stderr b/src/test/ui/parser/attr-stmt-expr-attr-bad.stderr
@@ -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