Auto merge of #56897 - euclio:parse-fatal, r=estebank

make `panictry!` private to libsyntax

This commit completely removes usage of the `panictry!` macro from
outside libsyntax. The macro causes parse errors to be fatal, so using
it in libsyntax_ext caused parse failures *within* a syntax extension to
be fatal, which is probably not intended.

Furthermore, this commit adds spans to diagnostics emitted by empty
extensions if they were missing, à la #56491.

5 files changed, 213 insertions, 111 deletions

diff --git a/src/libsyntax_ext/asm.rs b/src/libsyntax_ext/asm.rs
index a8f3c40db60..41ee6e91b3d 100644
--- a/src/libsyntax_ext/asm.rs
+++ b/src/libsyntax_ext/asm.rs
@@ -4,6 +4,7 @@ use self::State::*;
 
 use rustc_data_structures::thin_vec::ThinVec;
 
+use errors::DiagnosticBuilder;
 use syntax::ast;
 use syntax::ext::base;
 use syntax::ext::base::*;
@@ -51,6 +52,34 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt,
                                        feature_gate::EXPLAIN_ASM);
     }
 
+    let mut inline_asm = match parse_inline_asm(cx, sp, tts) {
+        Ok(Some(inline_asm)) => inline_asm,
+        Ok(None) => return DummyResult::expr(sp),
+        Err(mut err) => {
+            err.emit();
+            return DummyResult::expr(sp);
+        }
+    };
+
+    // If there are no outputs, the inline assembly is executed just for its side effects,
+    // so ensure that it is volatile
+    if inline_asm.outputs.is_empty() {
+        inline_asm.volatile = true;
+    }
+
+    MacEager::expr(P(ast::Expr {
+        id: ast::DUMMY_NODE_ID,
+        node: ast::ExprKind::InlineAsm(P(inline_asm)),
+        span: sp,
+        attrs: ThinVec::new(),
+    }))
+}
+
+fn parse_inline_asm<'a>(
+    cx: &mut ExtCtxt<'a>,
+    sp: Span,
+    tts: &[tokenstream::TokenTree],
+) -> Result<Option<ast::InlineAsm>, DiagnosticBuilder<'a>> {
     // Split the tts before the first colon, to avoid `asm!("x": y)`  being
     // parsed as `asm!(z)` with `z = "x": y` which is type ascription.
     let first_colon = tts.iter()
@@ -80,22 +109,33 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt,
                 if asm_str_style.is_some() {
                     // If we already have a string with instructions,
                     // ending up in Asm state again is an error.
-                    span_err!(cx, sp, E0660, "malformed inline assembly");
-                    return DummyResult::expr(sp);
+                    return Err(struct_span_err!(
+                        cx.parse_sess.span_diagnostic,
+                        sp,
+                        E0660,
+                        "malformed inline assembly"
+                    ));
                 }
                 // Nested parser, stop before the first colon (see above).
                 let mut p2 = cx.new_parser_from_tts(&tts[..first_colon]);
-                let (s, style) = match expr_to_string(cx,
-                                                      panictry!(p2.parse_expr()),
-                                                      "inline assembly must be a string literal") {
-                    Some((s, st)) => (s, st),
-                    // let compilation continue
-                    None => return DummyResult::expr(sp),
-                };
+
+                if p2.token == token::Eof {
+                    let mut err =
+                        cx.struct_span_err(sp, "macro requires a string literal as an argument");
+                    err.span_label(sp, "string literal required");
+                    return Err(err);
+                }
+
+                let expr = p2.parse_expr()?;
+                let (s, style) =
+                    match expr_to_string(cx, expr, "inline assembly must be a string literal") {
+                        Some((s, st)) => (s, st),
+                        None => return Ok(None),
+                    };
 
                 // This is most likely malformed.
                 if p2.token != token::Eof {
-                    let mut extra_tts = panictry!(p2.parse_all_token_trees());
+                    let mut extra_tts = p2.parse_all_token_trees()?;
                     extra_tts.extend(tts[first_colon..].iter().cloned());
                     p = parse::stream_to_parser(cx.parse_sess, extra_tts.into_iter().collect());
                 }
@@ -105,18 +145,17 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt,
             }
             Outputs => {
                 while p.token != token::Eof && p.token != token::Colon && p.token != token::ModSep {
-
                     if !outputs.is_empty() {
                         p.eat(&token::Comma);
                     }
 
-                    let (constraint, _str_style) = panictry!(p.parse_str());
+                    let (constraint, _) = p.parse_str()?;
 
                     let span = p.prev_span;
 
-                    panictry!(p.expect(&token::OpenDelim(token::Paren)));
-                    let out = panictry!(p.parse_expr());
-                    panictry!(p.expect(&token::CloseDelim(token::Paren)));
+                    p.expect(&token::OpenDelim(token::Paren))?;
+                    let expr = p.parse_expr()?;
+                    p.expect(&token::CloseDelim(token::Paren))?;
 
                     // Expands a read+write operand into two operands.
                     //
@@ -143,7 +182,7 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt,
                     let is_indirect = constraint_str.contains("*");
                     outputs.push(ast::InlineAsmOutput {
                         constraint: output.unwrap_or(constraint),
-                        expr: out,
+                        expr,
                         is_rw,
                         is_indirect,
                     });
@@ -151,12 +190,11 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt,
             }
             Inputs => {
                 while p.token != token::Eof && p.token != token::Colon && p.token != token::ModSep {
-
                     if !inputs.is_empty() {
                         p.eat(&token::Comma);
                     }
 
-                    let (constraint, _str_style) = panictry!(p.parse_str());
+                    let (constraint, _) = p.parse_str()?;
 
                     if constraint.as_str().starts_with("=") {
                         span_err!(cx, p.prev_span, E0662,
@@ -166,21 +204,20 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt,
                                                 "input operand constraint contains '+'");
                     }
 
-                    panictry!(p.expect(&token::OpenDelim(token::Paren)));
-                    let input = panictry!(p.parse_expr());
-                    panictry!(p.expect(&token::CloseDelim(token::Paren)));
+                    p.expect(&token::OpenDelim(token::Paren))?;
+                    let input = p.parse_expr()?;
+                    p.expect(&token::CloseDelim(token::Paren))?;
 
                     inputs.push((constraint, input));
                 }
             }
             Clobbers => {
                 while p.token != token::Eof && p.token != token::Colon && p.token != token::ModSep {
-
                     if !clobs.is_empty() {
                         p.eat(&token::Comma);
                     }
 
-                    let (s, _str_style) = panictry!(p.parse_str());
+                    let (s, _) = p.parse_str()?;
 
                     if OPTIONS.iter().any(|&opt| s == opt) {
                         cx.span_warn(p.prev_span, "expected a clobber, found an option");
@@ -193,7 +230,7 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt,
                 }
             }
             Options => {
-                let (option, _str_style) = panictry!(p.parse_str());
+                let (option, _) = p.parse_str()?;
 
                 if option == "volatile" {
                     // Indicates that the inline assembly has side effects
@@ -234,26 +271,15 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt,
         }
     }
 
-    // If there are no outputs, the inline assembly is executed just for its side effects,
-    // so ensure that it is volatile
-    if outputs.is_empty() {
-        volatile = true;
-    }
-
-    MacEager::expr(P(ast::Expr {
-        id: ast::DUMMY_NODE_ID,
-        node: ast::ExprKind::InlineAsm(P(ast::InlineAsm {
-            asm,
-            asm_str_style: asm_str_style.unwrap(),
-            outputs,
-            inputs,
-            clobbers: clobs,
-            volatile,
-            alignstack,
-            dialect,
-            ctxt: cx.backtrace(),
-        })),
-        span: sp,
-        attrs: ThinVec::new(),
+    Ok(Some(ast::InlineAsm {
+        asm,
+        asm_str_style: asm_str_style.unwrap(),
+        outputs,
+        inputs,
+        clobbers: clobs,
+        volatile,
+        alignstack,
+        dialect,
+        ctxt: cx.backtrace(),
     }))
 }
diff --git a/src/libsyntax_ext/assert.rs b/src/libsyntax_ext/assert.rs
index 2f5743e5e9b..b27f495322a 100644
--- a/src/libsyntax_ext/assert.rs
+++ b/src/libsyntax_ext/assert.rs
@@ -1,9 +1,11 @@
-use syntax::ast::*;
+use errors::DiagnosticBuilder;
+use syntax::ast::{self, *};
 use syntax::source_map::Spanned;
 use syntax::ext::base::*;
 use syntax::ext::build::AstBuilder;
 use syntax::parse::token;
 use syntax::print::pprust;
+use syntax::ptr::P;
 use syntax::symbol::Symbol;
 use syntax::tokenstream::{TokenStream, TokenTree};
 use syntax_pos::{Span, DUMMY_SP};
@@ -13,33 +15,18 @@ pub fn expand_assert<'cx>(
     sp: Span,
     tts: &[TokenTree],
 ) -> Box<dyn MacResult + 'cx> {
-    let mut parser = cx.new_parser_from_tts(tts);
-
-    if parser.token == token::Eof {
-        cx.struct_span_err(sp, "macro requires a boolean expression as an argument")
-            .span_label(sp, "boolean expression required")
-            .emit();
-        return DummyResult::expr(sp);
-    }
-
-    let cond_expr = panictry!(parser.parse_expr());
-    let custom_msg_args = if parser.eat(&token::Comma) {
-        let ts = parser.parse_tokens();
-        if !ts.is_empty() {
-            Some(ts)
-        } else {
-            None
+    let Assert { cond_expr, custom_message } = match parse_assert(cx, sp, tts) {
+        Ok(assert) => assert,
+        Err(mut err) => {
+            err.emit();
+            return DummyResult::expr(sp);
         }
-    } else {
-        None
     };
 
     let sp = sp.apply_mark(cx.current_expansion.mark);
     let panic_call = Mac_ {
         path: Path::from_ident(Ident::new(Symbol::intern("panic"), sp)),
-        tts: if let Some(ts) = custom_msg_args {
-            ts.into()
-        } else {
+        tts: custom_message.unwrap_or_else(|| {
             TokenStream::from(TokenTree::Token(
                 DUMMY_SP,
                 token::Literal(
@@ -49,8 +36,8 @@ pub fn expand_assert<'cx>(
                     ))),
                     None,
                 ),
-            )).into()
-        },
+            ))
+        }).into(),
         delim: MacDelimiter::Parenthesis,
     };
     let if_expr = cx.expr_if(
@@ -67,3 +54,36 @@ pub fn expand_assert<'cx>(
     );
     MacEager::expr(if_expr)
 }
+
+struct Assert {
+    cond_expr: P<ast::Expr>,
+    custom_message: Option<TokenStream>,
+}
+
+fn parse_assert<'a>(
+    cx: &mut ExtCtxt<'a>,
+    sp: Span,
+    tts: &[TokenTree]
+) -> Result<Assert, DiagnosticBuilder<'a>> {
+    let mut parser = cx.new_parser_from_tts(tts);
+
+    if parser.token == token::Eof {
+        let mut err = cx.struct_span_err(sp, "macro requires a boolean expression as an argument");
+        err.span_label(sp, "boolean expression required");
+        return Err(err);
+    }
+
+    Ok(Assert {
+        cond_expr: parser.parse_expr()?,
+        custom_message: if parser.eat(&token::Comma) {
+            let ts = parser.parse_tokens();
+            if !ts.is_empty() {
+                Some(ts)
+            } else {
+                None
+            }
+        } else {
+            None
+        },
+    })
+}
diff --git a/src/libsyntax_ext/cfg.rs b/src/libsyntax_ext/cfg.rs
index 7e3c1bbddf5..3b47b03cbe8 100644
--- a/src/libsyntax_ext/cfg.rs
+++ b/src/libsyntax_ext/cfg.rs
@@ -2,6 +2,8 @@
 /// a literal `true` or `false` based on whether the given cfg matches the
 /// current compilation environment.
 
+use errors::DiagnosticBuilder;
+use syntax::ast;
 use syntax::ext::base::*;
 use syntax::ext::base;
 use syntax::ext::build::AstBuilder;
@@ -15,16 +17,39 @@ pub fn expand_cfg<'cx>(cx: &mut ExtCtxt,
                        tts: &[tokenstream::TokenTree])
                        -> Box<dyn base::MacResult + 'static> {
     let sp = sp.apply_mark(cx.current_expansion.mark);
+
+    match parse_cfg(cx, sp, tts) {
+        Ok(cfg) => {
+            let matches_cfg = attr::cfg_matches(&cfg, cx.parse_sess, cx.ecfg.features);
+            MacEager::expr(cx.expr_bool(sp, matches_cfg))
+        }
+        Err(mut err) => {
+            err.emit();
+            DummyResult::expr(sp)
+        }
+    }
+}
+
+fn parse_cfg<'a>(
+    cx: &mut ExtCtxt<'a>,
+    sp: Span,
+    tts: &[tokenstream::TokenTree],
+) -> Result<ast::MetaItem, DiagnosticBuilder<'a>> {
     let mut p = cx.new_parser_from_tts(tts);
-    let cfg = panictry!(p.parse_meta_item());
+
+    if p.token == token::Eof {
+        let mut err = cx.struct_span_err(sp, "macro requires a cfg-pattern as an argument");
+        err.span_label(sp, "cfg-pattern required");
+        return Err(err);
+    }
+
+    let cfg = p.parse_meta_item()?;
 
     let _ = p.eat(&token::Comma);
 
     if !p.eat(&token::Eof) {
-        cx.span_err(sp, "expected 1 cfg-pattern");
-        return DummyResult::expr(sp);
+        return Err(cx.struct_span_err(sp, "expected 1 cfg-pattern"));
     }
 
-    let matches_cfg = attr::cfg_matches(&cfg, cx.parse_sess, cx.ecfg.features);
-    MacEager::expr(cx.expr_bool(sp, matches_cfg))
+    Ok(cfg)
 }
diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs
index c11f27f3ed5..61722ba5516 100644
--- a/src/libsyntax_ext/format.rs
+++ b/src/libsyntax_ext/format.rs
@@ -3,6 +3,7 @@ use self::Position::*;
 
 use fmt_macros as parse;
 
+use errors::DiagnosticBuilder;
 use syntax::ast;
 use syntax::ext::base::{self, *};
 use syntax::ext::build::AstBuilder;
@@ -112,7 +113,7 @@ struct Context<'a, 'b: 'a> {
     is_literal: bool,
 }
 
-/// Parses the arguments from the given list of tokens, returning None
+/// Parses the arguments from the given list of tokens, returning the diagnostic
 /// if there's a parse error so we can continue parsing other format!
 /// expressions.
 ///
@@ -121,27 +122,26 @@ struct Context<'a, 'b: 'a> {
 /// ```text
 /// Some((fmtstr, parsed arguments, index map for named arguments))
 /// ```
-fn parse_args(ecx: &mut ExtCtxt,
-              sp: Span,
-              tts: &[tokenstream::TokenTree])
-              -> Option<(P<ast::Expr>, Vec<P<ast::Expr>>, FxHashMap<String, usize>)> {
+fn parse_args<'a>(
+    ecx: &mut ExtCtxt<'a>,
+    sp: Span,
+    tts: &[tokenstream::TokenTree]
+) -> Result<(P<ast::Expr>, Vec<P<ast::Expr>>, FxHashMap<String, usize>), DiagnosticBuilder<'a>> {
     let mut args = Vec::<P<ast::Expr>>::new();
     let mut names = FxHashMap::<String, usize>::default();
 
     let mut p = ecx.new_parser_from_tts(tts);
 
     if p.token == token::Eof {
-        ecx.span_err(sp, "requires at least a format string argument");
-        return None;
+        return Err(ecx.struct_span_err(sp, "requires at least a format string argument"));
     }
 
-    let fmtstr = panictry!(p.parse_expr());
+    let fmtstr = p.parse_expr()?;
     let mut named = false;
 
     while p.token != token::Eof {
         if !p.eat(&token::Comma) {
-            ecx.span_err(p.span, "expected token: `,`");
-            return None;
+            return Err(ecx.struct_span_err(p.span, "expected token: `,`"));
         }
         if p.token == token::Eof {
             break;
@@ -152,16 +152,15 @@ fn parse_args(ecx: &mut ExtCtxt,
                 p.bump();
                 i
             } else {
-                ecx.span_err(
+                return Err(ecx.struct_span_err(
                     p.span,
                     "expected ident, positional arguments cannot follow named arguments",
-                );
-                return None;
+                ));
             };
             let name: &str = &ident.as_str();
 
-            panictry!(p.expect(&token::Eq));
-            let e = panictry!(p.parse_expr());
+            p.expect(&token::Eq).unwrap();
+            let e = p.parse_expr()?;
             if let Some(prev) = names.get(name) {
                 ecx.struct_span_err(e.span, &format!("duplicate argument named `{}`", name))
                     .span_note(args[*prev].span, "previously here")
@@ -177,10 +176,11 @@ fn parse_args(ecx: &mut ExtCtxt,
             names.insert(name.to_string(), slot);
             args.push(e);
         } else {
-            args.push(panictry!(p.parse_expr()));
+            let e = p.parse_expr()?;
+            args.push(e);
         }
     }
-    Some((fmtstr, args, names))
+    Ok((fmtstr, args, names))
 }
 
 impl<'a, 'b> Context<'a, 'b> {
@@ -689,10 +689,13 @@ pub fn expand_format_args<'cx>(ecx: &'cx mut ExtCtxt,
                                -> Box<dyn base::MacResult + 'cx> {
     sp = sp.apply_mark(ecx.current_expansion.mark);
     match parse_args(ecx, sp, tts) {
-        Some((efmt, args, names)) => {
+        Ok((efmt, args, names)) => {
             MacEager::expr(expand_preparsed_format_args(ecx, sp, efmt, args, names, false))
         }
-        None => DummyResult::expr(sp),
+        Err(mut err) => {
+            err.emit();
+            DummyResult::expr(sp)
+        }
     }
 }
 
@@ -716,10 +719,13 @@ pub fn expand_format_args_nl<'cx>(
     }
     sp = sp.apply_mark(ecx.current_expansion.mark);
     match parse_args(ecx, sp, tts) {
-        Some((efmt, args, names)) => {
+        Ok((efmt, args, names)) => {
             MacEager::expr(expand_preparsed_format_args(ecx, sp, efmt, args, names, true))
         }
-        None => DummyResult::expr(sp),
+        Err(mut err) => {
+            err.emit();
+            DummyResult::expr(sp)
+        }
     }
 }
 
diff --git a/src/libsyntax_ext/global_asm.rs b/src/libsyntax_ext/global_asm.rs
index a58c267ab4f..0a12e27c4fc 100644
--- a/src/libsyntax_ext/global_asm.rs
+++ b/src/libsyntax_ext/global_asm.rs
@@ -8,11 +8,13 @@
 /// LLVM's `module asm "some assembly here"`. All of LLVM's caveats
 /// therefore apply.
 
+use errors::DiagnosticBuilder;
 use syntax::ast;
 use syntax::source_map::respan;
 use syntax::ext::base;
 use syntax::ext::base::*;
 use syntax::feature_gate;
+use syntax::parse::token;
 use syntax::ptr::P;
 use syntax::symbol::Symbol;
 use syntax_pos::Span;
@@ -31,24 +33,47 @@ pub fn expand_global_asm<'cx>(cx: &'cx mut ExtCtxt,
                                        feature_gate::EXPLAIN_GLOBAL_ASM);
     }
 
+    match parse_global_asm(cx, sp, tts) {
+        Ok(Some(global_asm)) => {
+            MacEager::items(smallvec![P(ast::Item {
+                ident: ast::Ident::with_empty_ctxt(Symbol::intern("")),
+                attrs: Vec::new(),
+                id: ast::DUMMY_NODE_ID,
+                node: ast::ItemKind::GlobalAsm(P(global_asm)),
+                vis: respan(sp.shrink_to_lo(), ast::VisibilityKind::Inherited),
+                span: sp,
+                tokens: None,
+            })])
+        }
+        Ok(None) => DummyResult::any(sp),
+        Err(mut err) => {
+            err.emit();
+            DummyResult::any(sp)
+        }
+    }
+}
+
+fn parse_global_asm<'a>(
+    cx: &mut ExtCtxt<'a>,
+    sp: Span,
+    tts: &[tokenstream::TokenTree]
+) -> Result<Option<ast::GlobalAsm>, DiagnosticBuilder<'a>> {
     let mut p = cx.new_parser_from_tts(tts);
-    let (asm, _) = match expr_to_string(cx,
-                                        panictry!(p.parse_expr()),
-                                        "inline assembly must be a string literal") {
+
+    if p.token == token::Eof {
+        let mut err = cx.struct_span_err(sp, "macro requires a string literal as an argument");
+        err.span_label(sp, "string literal required");
+        return Err(err);
+    }
+
+    let expr = p.parse_expr()?;
+    let (asm, _) = match expr_to_string(cx, expr, "inline assembly must be a string literal") {
         Some((s, st)) => (s, st),
-        None => return DummyResult::any(sp),
+        None => return Ok(None),
     };
 
-    MacEager::items(smallvec![P(ast::Item {
-        ident: ast::Ident::with_empty_ctxt(Symbol::intern("")),
-        attrs: Vec::new(),
-        id: ast::DUMMY_NODE_ID,
-        node: ast::ItemKind::GlobalAsm(P(ast::GlobalAsm {
-            asm,
-            ctxt: cx.backtrace(),
-        })),
-        vis: respan(sp.shrink_to_lo(), ast::VisibilityKind::Inherited),
-        span: sp,
-        tokens: None,
-    })])
+    Ok(Some(ast::GlobalAsm {
+        asm,
+        ctxt: cx.backtrace(),
+    }))
 }