Skip to content

Transition macro_legacy_warnings into a hard error #69129

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

Merged
merged 1 commit into from
Feb 17, 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
11 changes: 4 additions & 7 deletions src/librustc_expand/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
span: Span,
) -> AstFragment {
let mut parser = self.cx.new_parser_from_tts(toks);
match parse_ast_fragment(&mut parser, kind, false) {
match parse_ast_fragment(&mut parser, kind) {
Ok(fragment) => {
ensure_complete_parse(&mut parser, path, kind.name(), span);
fragment
Expand All @@ -840,7 +840,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
pub fn parse_ast_fragment<'a>(
this: &mut Parser<'a>,
kind: AstFragmentKind,
macro_legacy_warnings: bool,
) -> PResult<'a, AstFragment> {
Ok(match kind {
AstFragmentKind::Items => {
Expand Down Expand Up @@ -873,11 +872,9 @@ pub fn parse_ast_fragment<'a>(
}
AstFragmentKind::Stmts => {
let mut stmts = SmallVec::new();
while this.token != token::Eof &&
// won't make progress on a `}`
this.token != token::CloseDelim(token::Brace)
{
if let Some(stmt) = this.parse_full_stmt(macro_legacy_warnings)? {
// Won't make progress on a `}`.
while this.token != token::Eof && this.token != token::CloseDelim(token::Brace) {
if let Some(stmt) = this.parse_full_stmt()? {
stmts.push(stmt);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_expand/mbe/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ fn suggest_slice_pat(e: &mut DiagnosticBuilder<'_>, site_span: Span, parser: &Pa
impl<'a> ParserAnyMacro<'a> {
crate fn make(mut self: Box<ParserAnyMacro<'a>>, kind: AstFragmentKind) -> AstFragment {
let ParserAnyMacro { site_span, macro_ident, ref mut parser, arm_span } = *self;
let fragment = panictry!(parse_ast_fragment(parser, kind, true).map_err(|mut e| {
let fragment = panictry!(parse_ast_fragment(parser, kind).map_err(|mut e| {
if parser.token == token::Eof && e.message().ends_with(", found `<eof>`") {
if !e.span.is_dummy() {
// early end of macro arm (#52866)
Expand Down
62 changes: 9 additions & 53 deletions src/librustc_parse/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,14 @@ impl<'a> Parser<'a> {
/// Parses 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.
pub fn parse_stmt(&mut self) -> PResult<'a, Option<Stmt>> {
Ok(self.parse_stmt_without_recovery(true).unwrap_or_else(|mut e| {
Ok(self.parse_stmt_without_recovery().unwrap_or_else(|mut e| {
e.emit();
self.recover_stmt_(SemiColonMode::Break, BlockMode::Ignore);
None
}))
}

fn parse_stmt_without_recovery(
&mut self,
macro_legacy_warnings: bool,
) -> PResult<'a, Option<Stmt>> {
fn parse_stmt_without_recovery(&mut self) -> PResult<'a, Option<Stmt>> {
maybe_whole!(self, NtStmt, |x| Some(x));

let attrs = self.parse_outer_attributes()?;
Expand Down Expand Up @@ -74,7 +71,7 @@ impl<'a> Parser<'a> {
let path = self.parse_path(PathStyle::Expr)?;

if self.eat(&token::Not) {
return self.parse_stmt_mac(lo, attrs.into(), path, macro_legacy_warnings);
return self.parse_stmt_mac(lo, attrs.into(), path);
}

let expr = if self.check(&token::OpenDelim(token::Brace)) {
Expand Down Expand Up @@ -137,7 +134,6 @@ impl<'a> Parser<'a> {
lo: Span,
attrs: AttrVec,
path: ast::Path,
legacy_warnings: bool,
) -> PResult<'a, Option<Stmt>> {
let args = self.parse_mac_args()?;
let delim = args.delim();
Expand All @@ -150,30 +146,6 @@ impl<'a> Parser<'a> {

let kind = if delim == token::Brace || self.token == token::Semi || self.token == token::Eof
{
StmtKind::Mac(P((mac, style, attrs.into())))
}
// 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.
else if legacy_warnings
&& self.token.can_begin_expr()
&& match self.token.kind {
// These 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
| token::DotDotEq => false,
_ => true,
}
{
self.warn_missing_semicolon();
StmtKind::Mac(P((mac, style, attrs)))
} else {
// Since none of the above applied, this is an expression statement macro.
Expand Down Expand Up @@ -334,7 +306,7 @@ impl<'a> Parser<'a> {
// bar;
//
// which is valid in other languages, but not Rust.
match self.parse_stmt_without_recovery(false) {
match self.parse_stmt_without_recovery() {
Ok(Some(stmt)) => {
if self.look_ahead(1, |t| t == &token::OpenDelim(token::Brace))
|| do_not_suggest_help
Expand Down Expand Up @@ -393,7 +365,7 @@ impl<'a> Parser<'a> {
if self.token == token::Eof {
break;
}
let stmt = match self.parse_full_stmt(false) {
let stmt = match self.parse_full_stmt() {
Err(mut err) => {
self.maybe_annotate_with_ascription(&mut err, false);
err.emit();
Expand All @@ -413,11 +385,11 @@ impl<'a> Parser<'a> {
}

/// Parses a statement, including the trailing semicolon.
pub fn parse_full_stmt(&mut self, macro_legacy_warnings: bool) -> PResult<'a, Option<Stmt>> {
pub fn parse_full_stmt(&mut self) -> PResult<'a, Option<Stmt>> {
// Skip looking for a trailing semicolon when we have an interpolated statement.
maybe_whole!(self, NtStmt, |x| Some(x));

let mut stmt = match self.parse_stmt_without_recovery(macro_legacy_warnings)? {
let mut stmt = match self.parse_stmt_without_recovery()? {
Some(stmt) => stmt,
None => return Ok(None),
};
Expand Down Expand Up @@ -457,13 +429,8 @@ impl<'a> Parser<'a> {
}
}
StmtKind::Local(..) => {
// We used to incorrectly allow a macro-expanded let statement to lack a semicolon.
if macro_legacy_warnings && self.token != token::Semi {
self.warn_missing_semicolon();
} else {
self.expect_semi()?;
eat_semi = false;
}
self.expect_semi()?;
eat_semi = false;
}
_ => {}
}
Expand All @@ -475,17 +442,6 @@ impl<'a> Parser<'a> {
Ok(Some(stmt))
}

fn warn_missing_semicolon(&self) {
self.diagnostic()
.struct_span_warn(self.token.span, {
&format!("expected `;`, found {}", super::token_descr(&self.token))
})
.note({
"this was erroneously allowed and will become a hard error in a future release"
})
.emit();
}

pub(super) fn mk_block(&self, stmts: Vec<Stmt>, rules: BlockCheckMode, span: Span) -> P<Block> {
P(Block { stmts, id: DUMMY_NODE_ID, rules, span })
}
Expand Down
12 changes: 0 additions & 12 deletions src/test/ui/missing/missing-semicolon-warning.rs

This file was deleted.

24 changes: 0 additions & 24 deletions src/test/ui/missing/missing-semicolon-warning.stderr

This file was deleted.

8 changes: 8 additions & 0 deletions src/test/ui/parser/missing-semicolon.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
macro_rules! m {
($($e1:expr),*; $($e2:expr),*) => {
$( let x = $e1 )*; //~ ERROR expected one of `.`, `;`, `?`, or
$( println!("{}", $e2) )*;
}
}

fn main() { m!(0, 0; 0, 0); }
13 changes: 13 additions & 0 deletions src/test/ui/parser/missing-semicolon.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error: expected one of `.`, `;`, `?`, or an operator, found keyword `let`
--> $DIR/missing-semicolon.rs:3:12
|
LL | $( let x = $e1 )*;
| ^^^ expected one of `.`, `;`, `?`, or an operator
...
LL | fn main() { m!(0, 0; 0, 0); }
| --------------- in this macro invocation
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error