Auto merge of #34660 - jseyfried:fix_parse_stmt, r=nrc

Fix bugs in macro-expanded statement parsing

Fixes #34543.

This is a [breaking-change]. For example, the following would break:
```rust
macro_rules! m { () => {
    println!("") println!("")
    //^ Semicolons are now required on macro-expanded non-braced macro invocations
    //| in statement positions.
    let x = 0
    //^ Semicolons are now required on macro-expanded `let` statements
    //| that are followed by more statements, so this would break.
    let y = 0 //< (this would still be allowed to reduce breakage in the wild)
}
fn main() { m!() }
```

r? @eddyb

Author: bors

src/doc/book/macros.md

@@ -328,7 +328,7 @@ invocation site. Code such as the following will not work:
 
 ```rust,ignore
 macro_rules! foo {
-    () => (let x = 3);
+    () => (let x = 3;);
 }
 
 fn main() {
@@ -342,7 +342,7 @@ tagged with the right syntax context.
 
 ```rust
 macro_rules! foo {
-    ($v:ident) => (let $v = 3);
+    ($v:ident) => (let $v = 3;);
 }
 
 fn main() {

src/librustc_save_analysis/dump_visitor.rs

@@ -57,7 +57,7 @@ macro_rules! down_cast_data {
             data
         } else {
             span_bug!($sp, "unexpected data kind: {:?}", $id);
-        }
+        };
     };
 }
 

src/libsyntax/ast.rs

@@ -804,6 +804,19 @@ pub struct Stmt {
     pub span: Span,
 }
 
+impl Stmt {
+    pub fn add_trailing_semicolon(mut self) -> Self {
+        self.node = match self.node {
+            StmtKind::Expr(expr) => StmtKind::Semi(expr),
+            StmtKind::Mac(mac) => StmtKind::Mac(mac.map(|(mac, _style, attrs)| {
+                (mac, MacStmtStyle::Semicolon, attrs)
+            })),
+            node @ _ => node,
+        };
+        self
+    }
+}
+
 impl fmt::Debug for Stmt {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "stmt({}: {})", self.id.to_string(), pprust::stmt_to_string(self))

src/libsyntax/ext/expand.rs

@@ -444,14 +444,7 @@ fn expand_stmt(stmt: Stmt, fld: &mut MacroExpander) -> SmallVector<Stmt> {
     // semicolon to the final statement produced by expansion.
     if style == MacStmtStyle::Semicolon {
         if let Some(stmt) = fully_expanded.pop() {
-            fully_expanded.push(Stmt {
-                id: stmt.id,
-                node: match stmt.node {
-                    StmtKind::Expr(expr) => StmtKind::Semi(expr),
-                    _ => stmt.node /* might already have a semi */
-                },
-                span: stmt.span,
-            });
+            fully_expanded.push(stmt.add_trailing_semicolon());
         }
     }
 

src/libsyntax/ext/tt/macro_rules.rs

@@ -123,7 +123,7 @@ impl<'a> MacResult for ParserAnyMacro<'a> {
             let mut parser = self.parser.borrow_mut();
             match parser.token {
                 token::Eof => break,
-                _ => match parser.parse_stmt() {
+                _ => match parser.parse_full_stmt(true) {
                     Ok(maybe_stmt) => match maybe_stmt {
                         Some(stmt) => ret.push(stmt),
                         None => (),

src/libsyntax/parse/parser.rs

@@ -3789,7 +3789,13 @@ impl<'a> Parser<'a> {
         self.span_err(self.last_span, message);
     }
 
-    /// Parse a statement. may include decl.
+    /// Parse a statement. This stops just before trailing semicolons on everything but items.
+    /// e.g. a `StmtKind::Semi` parses to a `StmtKind::Expr`, leaving the trailing `;` unconsumed.
+    ///
+    /// Also, if a macro begins an expression statement, this only parses the macro. For example,
+    /// ```rust
+    /// vec![1].into_iter(); //< `parse_stmt` only parses the "vec![1]"
+    /// ```
     pub fn parse_stmt(&mut self) -> PResult<'a, Option<Stmt>> {
         Ok(self.parse_stmt_())
     }
@@ -4038,36 +4044,14 @@ impl<'a> Parser<'a> {
         let mut stmts = vec![];
 
         while !self.eat(&token::CloseDelim(token::Brace)) {
-            let Stmt {node, span, ..} = if let Some(s) = self.parse_stmt_() {
-                s
+            if let Some(stmt) = self.parse_full_stmt(false)? {
+                stmts.push(stmt);
             } else if self.token == token::Eof {
                 break;
             } else {
                 // Found only `;` or `}`.
                 continue;
             };
-
-            match node {
-                StmtKind::Expr(e) => {
-                    self.handle_expression_like_statement(e, span, &mut stmts)?;
-                }
-                StmtKind::Mac(mac) => {
-                    self.handle_macro_in_block(mac.unwrap(), span, &mut stmts)?;
-                }
-                _ => { // all other kinds of statements:
-                    let mut hi = span.hi;
-                    if classify::stmt_ends_with_semi(&node) {
-                        self.expect(&token::Semi)?;
-                        hi = self.last_span.hi;
-                    }
-
-                    stmts.push(Stmt {
-                        id: ast::DUMMY_NODE_ID,
-                        node: node,
-                        span: mk_sp(span.lo, hi)
-                    });
-                }
-            }
         }
 
         Ok(P(ast::Block {
@@ -4078,93 +4062,88 @@ impl<'a> Parser<'a> {
         }))
     }
 
-    fn handle_macro_in_block(&mut self,
-                             (mac, style, attrs): (ast::Mac, MacStmtStyle, ThinVec<Attribute>),
-                             span: Span,
-                             stmts: &mut Vec<Stmt>)
-                             -> PResult<'a, ()> {
-        if style == MacStmtStyle::NoBraces {
-            // statement macro without braces; might be an
-            // expr depending on whether a semicolon follows
-            match self.token {
-                token::Semi => {
-                    stmts.push(Stmt {
-                        id: ast::DUMMY_NODE_ID,
-                        node: StmtKind::Mac(P((mac, MacStmtStyle::Semicolon, attrs))),
-                        span: mk_sp(span.lo, self.span.hi),
-                    });
-                    self.bump();
-                }
-                _ => {
-                    let e = self.mk_mac_expr(span.lo, span.hi, mac.node, ThinVec::new());
-                    let lo = e.span.lo;
-                    let e = self.parse_dot_or_call_expr_with(e, lo, attrs)?;
-                    let e = self.parse_assoc_expr_with(0, LhsExpr::AlreadyParsed(e))?;
-                    self.handle_expression_like_statement(e, span, stmts)?;
-                }
-            }
-        } else {
-            // statement macro; might be an expr
-            match self.token {
-                token::Semi => {
-                    stmts.push(Stmt {
-                        id: ast::DUMMY_NODE_ID,
-                        node: StmtKind::Mac(P((mac, MacStmtStyle::Semicolon, attrs))),
-                        span: mk_sp(span.lo, self.span.hi),
-                    });
-                    self.bump();
-                }
-                _ => {
-                    stmts.push(Stmt {
-                        id: ast::DUMMY_NODE_ID,
-                        node: StmtKind::Mac(P((mac, style, attrs))),
-                        span: span
-                    });
+    /// Parse a statement, including the trailing semicolon.
+    /// This parses expression statements that begin with macros correctly (c.f. `parse_stmt`).
+    pub fn parse_full_stmt(&mut self, macro_expanded: bool) -> PResult<'a, Option<Stmt>> {
+        let mut stmt = match self.parse_stmt_() {
+            Some(stmt) => stmt,
+            None => return Ok(None),
+        };
+
+        if let StmtKind::Mac(mac) = stmt.node {
+            if mac.1 != MacStmtStyle::NoBraces ||
+               self.token == token::Semi || self.token == token::Eof {
+                stmt.node = StmtKind::Mac(mac);
+            } else {
+                // We used to incorrectly stop parsing macro-expanded statements here.
+                // If the next token will be an error anyway but could have parsed with the
+                // earlier behavior, stop parsing here and emit a warning to avoid breakage.
+                if macro_expanded && self.token.can_begin_expr() && match self.token {
+                    // These tokens can continue an expression, so we can't stop parsing and warn.
+                    token::OpenDelim(token::Paren) | token::OpenDelim(token::Bracket) |
+                    token::BinOp(token::Minus) | token::BinOp(token::Star) |
+                    token::BinOp(token::And) | token::BinOp(token::Or) |
+                    token::AndAnd | token::OrOr |
+                    token::DotDot | token::DotDotDot => false,
+                    _ => true,
+                } {
+                    self.warn_missing_semicolon();
+                    stmt.node = StmtKind::Mac(mac);
+                    return Ok(Some(stmt));
                 }
+
+                let (mac, _style, attrs) = mac.unwrap();
+                let e = self.mk_mac_expr(stmt.span.lo, stmt.span.hi, mac.node, ThinVec::new());
+                let e = self.parse_dot_or_call_expr_with(e, stmt.span.lo, attrs)?;
+                let e = self.parse_assoc_expr_with(0, LhsExpr::AlreadyParsed(e))?;
+                stmt.node = StmtKind::Expr(e);
             }
         }
-        Ok(())
+
+        stmt = self.handle_trailing_semicolon(stmt, macro_expanded)?;
+        Ok(Some(stmt))
     }
 
-    fn handle_expression_like_statement(&mut self,
-                                        e: P<Expr>,
-                                        span: Span,
-                                        stmts: &mut Vec<Stmt>)
-                                        -> PResult<'a, ()> {
-        // expression without semicolon
-        if classify::expr_requires_semi_to_be_stmt(&e) {
-            // Just check for errors and recover; do not eat semicolon yet.
-            if let Err(mut e) =
-                self.expect_one_of(&[], &[token::Semi, token::CloseDelim(token::Brace)])
-            {
-                e.emit();
-                self.recover_stmt();
+    fn handle_trailing_semicolon(&mut self, mut stmt: Stmt, macro_expanded: bool)
+                                 -> PResult<'a, Stmt> {
+        match stmt.node {
+            StmtKind::Expr(ref expr) if self.token != token::Eof => {
+                // expression without semicolon
+                if classify::expr_requires_semi_to_be_stmt(expr) {
+                    // Just check for errors and recover; do not eat semicolon yet.
+                    if let Err(mut e) =
+                        self.expect_one_of(&[], &[token::Semi, token::CloseDelim(token::Brace)])
+                    {
+                        e.emit();
+                        self.recover_stmt();
+                    }
+                }
+            }
+            StmtKind::Local(..) => {
+                // We used to incorrectly allow a macro-expanded let statement to lack a semicolon.
+                if macro_expanded && self.token != token::Semi {
+                    self.warn_missing_semicolon();
+                } else {
+                    self.expect_one_of(&[token::Semi], &[])?;
+                }
             }
+            _ => {}
         }
 
-        match self.token {
-            token::Semi => {
-                self.bump();
-                let span_with_semi = Span {
-                    lo: span.lo,
-                    hi: self.last_span.hi,
-                    expn_id: span.expn_id,
-                };
-                stmts.push(Stmt {
-                    id: ast::DUMMY_NODE_ID,
-                    node: StmtKind::Semi(e),
-                    span: span_with_semi,
-                });
-            }
-            _ => {
-                stmts.push(Stmt {
-                    id: ast::DUMMY_NODE_ID,
-                    node: StmtKind::Expr(e),
-                    span: span
-                });
-            }
+        if self.eat(&token::Semi) {
+            stmt = stmt.add_trailing_semicolon();
         }
-        Ok(())
+
+        stmt.span.hi = self.last_span.hi;
+        Ok(stmt)
+    }
+
+    fn warn_missing_semicolon(&self) {
+        self.diagnostic().struct_span_warn(self.span, {
+            &format!("expected `;`, found `{}`", self.this_token_to_string())
+        }).note({
+            "This was erroneously allowed and will become a hard error in a future release"
+        }).emit();
     }
 
     // Parses a sequence of bounds if a `:` is found,

src/libsyntax/print/pprust.rs

@@ -1638,9 +1638,8 @@ impl<'a> State<'a> {
                     _ => token::Paren
                 };
                 try!(self.print_mac(&mac, delim));
-                match style {
-                    ast::MacStmtStyle::Braces => {}
-                    _ => try!(word(&mut self.s, ";")),
+                if style == ast::MacStmtStyle::Semicolon {
+                    try!(word(&mut self.s, ";"));
                 }
             }
         }

src/test/compile-fail/macro-incomplete-parse.rs

@@ -19,7 +19,7 @@ macro_rules! ignored_item {
 }
 
 macro_rules! ignored_expr {
-    () => ( 1,  //~ ERROR expected expression, found `,`
+    () => ( 1,  //~ ERROR expected one of `.`, `;`, `?`, `}`, or an operator, found `,`
             2 )
 }
 

src/test/compile-fail/missing-semicolon-warning.rs

@@ -0,0 +1,22 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(rustc_attrs)]
+#![allow(unused)]
+
+macro_rules! m {
+    ($($e1:expr),*; $($e2:expr),*) => {
+        $( let x = $e1 )*; //~ WARN expected `;`
+        $( println!("{}", $e2) )*; //~ WARN expected `;`
+    }
+}
+
+#[rustc_error]
+fn main() { m!(0, 0; 0, 0); } //~ ERROR compilation successful