Skip to content

Commit

Permalink
Auto merge of #56897 - euclio:parse-fatal, r=estebank
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Jan 4, 2019
2 parents d6d32ac + 0a6fb84 commit f381a96
Show file tree
Hide file tree
Showing 22 changed files with 451 additions and 134 deletions.
10 changes: 8 additions & 2 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use syntax::json::JsonEmitter;
use syntax::ptr::P;
use syntax::symbol::keywords;
use syntax_pos::DUMMY_SP;
use errors;
use errors::{self, FatalError};
use errors::emitter::{Emitter, EmitterWriter};
use parking_lot::ReentrantMutex;

Expand Down Expand Up @@ -429,7 +429,13 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt

let control = &driver::CompileController::basic();

let krate = panictry!(driver::phase_1_parse_input(control, &sess, &input));
let krate = match driver::phase_1_parse_input(control, &sess, &input) {
Ok(krate) => krate,
Err(mut e) => {
e.emit();
FatalError.raise();
}
};

let name = match crate_name {
Some(ref crate_name) => crate_name.clone(),
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ extern crate rustc_metadata;
extern crate rustc_target;
extern crate rustc_typeck;
extern crate serialize;
#[macro_use] extern crate syntax;
extern crate syntax;
extern crate syntax_pos;
extern crate test as testing;
#[macro_use] extern crate log;
Expand Down
13 changes: 9 additions & 4 deletions src/librustdoc/test.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use errors;
use errors::{self, FatalError};
use errors::emitter::ColorConfig;
use rustc_data_structures::sync::Lrc;
use rustc_lint;
Expand Down Expand Up @@ -84,9 +84,14 @@ pub fn run(mut options: Options) -> isize {
target_features::add_configuration(&mut cfg, &sess, &*codegen_backend);
sess.parse_sess.config = cfg;

let krate = panictry!(driver::phase_1_parse_input(&driver::CompileController::basic(),
&sess,
&input));
let krate =
match driver::phase_1_parse_input(&driver::CompileController::basic(), &sess, &input) {
Ok(krate) => krate,
Err(mut e) => {
e.emit();
FatalError.raise();
}
};
let driver::ExpansionResult { defs, mut hir_forest, .. } = {
phase_2_configure_and_expand(
&sess,
Expand Down
2 changes: 0 additions & 2 deletions src/libsyntax/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ use ast::AttrId;
// way towards a non-panic!-prone parser. It should be used for fatal parsing
// errors; eventually we plan to convert all code using panictry to just use
// normal try.
// Exported for syntax_ext, not meant for general use.
#[macro_export]
macro_rules! panictry {
($e:expr) => ({
use std::result::Result::{Ok, Err};
Expand Down
116 changes: 71 additions & 45 deletions src/libsyntax_ext/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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());
}
Expand All @@ -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.
//
Expand All @@ -143,20 +182,19 @@ 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,
});
}
}
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,
Expand All @@ -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");
Expand All @@ -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
Expand Down Expand Up @@ -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(),
}))
}
68 changes: 44 additions & 24 deletions src/libsyntax_ext/assert.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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(
Expand All @@ -49,8 +36,8 @@ pub fn expand_assert<'cx>(
))),
None,
),
)).into()
},
))
}).into(),
delim: MacDelimiter::Parenthesis,
};
let if_expr = cx.expr_if(
Expand All @@ -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
},
})
}
Loading

0 comments on commit f381a96

Please sign in to comment.