From 8823634db8c4ee65b4a3d47ccb1d81891e33d0de Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Mon, 8 Aug 2022 00:05:20 +0200 Subject: [PATCH 1/2] Manually cleanup token stream when macro expansion aborts. --- .../rustc_parse/src/parser/attr_wrapper.rs | 45 ++++++++++++------- src/test/ui/macros/syntax-error-recovery.rs | 17 +++++++ .../ui/macros/syntax-error-recovery.stderr | 24 ++++++++++ 3 files changed, 71 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/macros/syntax-error-recovery.rs create mode 100644 src/test/ui/macros/syntax-error-recovery.stderr diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 5fdafd187c660..34021c40ba967 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -442,23 +442,38 @@ fn make_token_stream( } token_and_spacing = iter.next(); } - let mut final_buf = stack.pop().expect("Missing final buf!"); - if break_last_token { - let last_token = final_buf.inner.pop().unwrap(); - if let AttrTokenTree::Token(last_token, spacing) = last_token { - let unglued_first = last_token.kind.break_two_token_op().unwrap().0; - - // An 'unglued' token is always two ASCII characters - let mut first_span = last_token.span.shrink_to_lo(); - first_span = first_span.with_hi(first_span.lo() + rustc_span::BytePos(1)); - - final_buf + while let Some(FrameData { open_delim_sp, mut inner }) = stack.pop() { + // A former macro expansion could give us malformed tokens. + // In that case, manually close all open delimitors so downstream users + // don't ICE on them. + if let Some((delim, open_sp)) = open_delim_sp { + let dspan = DelimSpan::from_pair(open_sp, rustc_span::DUMMY_SP); + let stream = AttrTokenStream::new(inner); + let delimited = AttrTokenTree::Delimited(dspan, delim, stream); + stack + .last_mut() + .unwrap_or_else(|| panic!("Bottom token frame is missing for recovered token")) .inner - .push(AttrTokenTree::Token(Token::new(unglued_first, first_span), spacing)); + .push(delimited); } else { - panic!("Unexpected last token {:?}", last_token) + if break_last_token { + let last_token = inner.pop().unwrap(); + if let AttrTokenTree::Token(last_token, spacing) = last_token { + let unglued_first = last_token.kind.break_two_token_op().unwrap().0; + + // An 'unglued' token is always two ASCII characters + let mut first_span = last_token.span.shrink_to_lo(); + first_span = first_span.with_hi(first_span.lo() + rustc_span::BytePos(1)); + + inner + .push(AttrTokenTree::Token(Token::new(unglued_first, first_span), spacing)); + } else { + panic!("Unexpected last token {:?}", last_token) + } + } + assert!(stack.is_empty(), "Stack should be empty: stack={:?}", stack); + return AttrTokenStream::new(inner); } } - assert!(stack.is_empty(), "Stack should be empty: final_buf={:?} stack={:?}", final_buf, stack); - AttrTokenStream::new(final_buf.inner) + panic!("Missing final buf!") } diff --git a/src/test/ui/macros/syntax-error-recovery.rs b/src/test/ui/macros/syntax-error-recovery.rs new file mode 100644 index 0000000000000..2b2eec047058f --- /dev/null +++ b/src/test/ui/macros/syntax-error-recovery.rs @@ -0,0 +1,17 @@ +macro_rules! values { + ($($token:ident($value:literal) $(as $inner:ty)? => $attr:meta,)*) => { + #[derive(Debug)] + pub enum TokenKind { + $( + #[$attr] + $token $($inner)? = $value, + )* + } + }; +} +//~^^^^^ ERROR expected one of `(`, `,`, `=`, `{`, or `}`, found `(String)` +//~| ERROR macro expansion ignores token `(String)` and any following + +values!(STRING(1) as (String) => cfg(test),); + +fn main() {} diff --git a/src/test/ui/macros/syntax-error-recovery.stderr b/src/test/ui/macros/syntax-error-recovery.stderr new file mode 100644 index 0000000000000..1d9ce110d7b6e --- /dev/null +++ b/src/test/ui/macros/syntax-error-recovery.stderr @@ -0,0 +1,24 @@ +error: expected one of `(`, `,`, `=`, `{`, or `}`, found `(String)` + --> $DIR/syntax-error-recovery.rs:7:26 + | +LL | $token $($inner)? = $value, + | ^^^^^^ expected one of `(`, `,`, `=`, `{`, or `}` +... +LL | values!(STRING(1) as (String) => cfg(test),); + | -------------------------------------------- in this macro invocation + | + = note: this error originates in the macro `values` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: macro expansion ignores token `(String)` and any following + --> $DIR/syntax-error-recovery.rs:7:26 + | +LL | $token $($inner)? = $value, + | ^^^^^^ +... +LL | values!(STRING(1) as (String) => cfg(test),); + | -------------------------------------------- caused by the macro expansion here + | + = note: the usage of `values!` is likely invalid in item context + +error: aborting due to 2 previous errors + From cb5ea8d0b67e4a46f9f30aa93107035d9a1dadf0 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Mon, 8 Aug 2022 19:17:37 +0200 Subject: [PATCH 2/2] Emit an error instead of reconstructing token stream. --- compiler/rustc_builtin_macros/src/cfg_eval.rs | 64 +++++++++++-------- .../rustc_parse/src/parser/attr_wrapper.rs | 44 ++++--------- src/test/ui/macros/syntax-error-recovery.rs | 1 + .../ui/macros/syntax-error-recovery.stderr | 8 ++- 4 files changed, 58 insertions(+), 59 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/cfg_eval.rs b/compiler/rustc_builtin_macros/src/cfg_eval.rs index e673dff0dea8e..009f3c783d4c8 100644 --- a/compiler/rustc_builtin_macros/src/cfg_eval.rs +++ b/compiler/rustc_builtin_macros/src/cfg_eval.rs @@ -7,6 +7,7 @@ use rustc_ast::visit::Visitor; use rustc_ast::NodeId; use rustc_ast::{mut_visit, visit}; use rustc_ast::{Attribute, HasAttrs, HasTokens}; +use rustc_errors::PResult; use rustc_expand::base::{Annotatable, ExtCtxt}; use rustc_expand::config::StripUnconfigured; use rustc_expand::configure; @@ -144,33 +145,34 @@ impl CfgEval<'_, '_> { // the location of `#[cfg]` and `#[cfg_attr]` in the token stream. The tokenization // process is lossless, so this process is invisible to proc-macros. - let parse_annotatable_with: fn(&mut Parser<'_>) -> _ = match annotatable { - Annotatable::Item(_) => { - |parser| Annotatable::Item(parser.parse_item(ForceCollect::Yes).unwrap().unwrap()) - } - Annotatable::TraitItem(_) => |parser| { - Annotatable::TraitItem( - parser.parse_trait_item(ForceCollect::Yes).unwrap().unwrap().unwrap(), - ) - }, - Annotatable::ImplItem(_) => |parser| { - Annotatable::ImplItem( - parser.parse_impl_item(ForceCollect::Yes).unwrap().unwrap().unwrap(), - ) - }, - Annotatable::ForeignItem(_) => |parser| { - Annotatable::ForeignItem( - parser.parse_foreign_item(ForceCollect::Yes).unwrap().unwrap().unwrap(), - ) - }, - Annotatable::Stmt(_) => |parser| { - Annotatable::Stmt(P(parser.parse_stmt(ForceCollect::Yes).unwrap().unwrap())) - }, - Annotatable::Expr(_) => { - |parser| Annotatable::Expr(parser.parse_expr_force_collect().unwrap()) - } - _ => unreachable!(), - }; + let parse_annotatable_with: for<'a> fn(&mut Parser<'a>) -> PResult<'a, _> = + match annotatable { + Annotatable::Item(_) => { + |parser| Ok(Annotatable::Item(parser.parse_item(ForceCollect::Yes)?.unwrap())) + } + Annotatable::TraitItem(_) => |parser| { + Ok(Annotatable::TraitItem( + parser.parse_trait_item(ForceCollect::Yes)?.unwrap().unwrap(), + )) + }, + Annotatable::ImplItem(_) => |parser| { + Ok(Annotatable::ImplItem( + parser.parse_impl_item(ForceCollect::Yes)?.unwrap().unwrap(), + )) + }, + Annotatable::ForeignItem(_) => |parser| { + Ok(Annotatable::ForeignItem( + parser.parse_foreign_item(ForceCollect::Yes)?.unwrap().unwrap(), + )) + }, + Annotatable::Stmt(_) => |parser| { + Ok(Annotatable::Stmt(P(parser.parse_stmt(ForceCollect::Yes)?.unwrap()))) + }, + Annotatable::Expr(_) => { + |parser| Ok(Annotatable::Expr(parser.parse_expr_force_collect()?)) + } + _ => unreachable!(), + }; // 'Flatten' all nonterminals (i.e. `TokenKind::Interpolated`) // to `None`-delimited groups containing the corresponding tokens. This @@ -193,7 +195,13 @@ impl CfgEval<'_, '_> { let mut parser = rustc_parse::stream_to_parser(&self.cfg.sess.parse_sess, orig_tokens, None); parser.capture_cfg = true; - annotatable = parse_annotatable_with(&mut parser); + match parse_annotatable_with(&mut parser) { + Ok(a) => annotatable = a, + Err(mut err) => { + err.emit(); + return Some(annotatable); + } + } // Now that we have our re-parsed `AttrTokenStream`, recursively configuring // our attribute target will correctly the tokens as well. diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 34021c40ba967..86c386b94c834 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -442,38 +442,22 @@ fn make_token_stream( } token_and_spacing = iter.next(); } - while let Some(FrameData { open_delim_sp, mut inner }) = stack.pop() { - // A former macro expansion could give us malformed tokens. - // In that case, manually close all open delimitors so downstream users - // don't ICE on them. - if let Some((delim, open_sp)) = open_delim_sp { - let dspan = DelimSpan::from_pair(open_sp, rustc_span::DUMMY_SP); - let stream = AttrTokenStream::new(inner); - let delimited = AttrTokenTree::Delimited(dspan, delim, stream); - stack - .last_mut() - .unwrap_or_else(|| panic!("Bottom token frame is missing for recovered token")) + let mut final_buf = stack.pop().expect("Missing final buf!"); + if break_last_token { + let last_token = final_buf.inner.pop().unwrap(); + if let AttrTokenTree::Token(last_token, spacing) = last_token { + let unglued_first = last_token.kind.break_two_token_op().unwrap().0; + + // An 'unglued' token is always two ASCII characters + let mut first_span = last_token.span.shrink_to_lo(); + first_span = first_span.with_hi(first_span.lo() + rustc_span::BytePos(1)); + + final_buf .inner - .push(delimited); + .push(AttrTokenTree::Token(Token::new(unglued_first, first_span), spacing)); } else { - if break_last_token { - let last_token = inner.pop().unwrap(); - if let AttrTokenTree::Token(last_token, spacing) = last_token { - let unglued_first = last_token.kind.break_two_token_op().unwrap().0; - - // An 'unglued' token is always two ASCII characters - let mut first_span = last_token.span.shrink_to_lo(); - first_span = first_span.with_hi(first_span.lo() + rustc_span::BytePos(1)); - - inner - .push(AttrTokenTree::Token(Token::new(unglued_first, first_span), spacing)); - } else { - panic!("Unexpected last token {:?}", last_token) - } - } - assert!(stack.is_empty(), "Stack should be empty: stack={:?}", stack); - return AttrTokenStream::new(inner); + panic!("Unexpected last token {:?}", last_token) } } - panic!("Missing final buf!") + AttrTokenStream::new(final_buf.inner) } diff --git a/src/test/ui/macros/syntax-error-recovery.rs b/src/test/ui/macros/syntax-error-recovery.rs index 2b2eec047058f..ae6de3c5046cd 100644 --- a/src/test/ui/macros/syntax-error-recovery.rs +++ b/src/test/ui/macros/syntax-error-recovery.rs @@ -13,5 +13,6 @@ macro_rules! values { //~| ERROR macro expansion ignores token `(String)` and any following values!(STRING(1) as (String) => cfg(test),); +//~^ ERROR expected one of `!` or `::`, found `` fn main() {} diff --git a/src/test/ui/macros/syntax-error-recovery.stderr b/src/test/ui/macros/syntax-error-recovery.stderr index 1d9ce110d7b6e..c153b3b910bbe 100644 --- a/src/test/ui/macros/syntax-error-recovery.stderr +++ b/src/test/ui/macros/syntax-error-recovery.stderr @@ -20,5 +20,11 @@ LL | values!(STRING(1) as (String) => cfg(test),); | = note: the usage of `values!` is likely invalid in item context -error: aborting due to 2 previous errors +error: expected one of `!` or `::`, found `` + --> $DIR/syntax-error-recovery.rs:15:9 + | +LL | values!(STRING(1) as (String) => cfg(test),); + | ^^^^^^ expected one of `!` or `::` + +error: aborting due to 3 previous errors