diff --git a/src/libsyntax/ext/tt/macro_parser.rs b/src/libsyntax/ext/tt/macro_parser.rs index 89ecf02ee4c..ca5eb8f8003 100644 --- a/src/libsyntax/ext/tt/macro_parser.rs +++ b/src/libsyntax/ext/tt/macro_parser.rs @@ -549,13 +549,8 @@ pub fn parse_nt<'a>(p: &mut Parser<'a>, sp: Span, name: &str) -> Nonterminal { token::NtPath(Box::new(panictry!(p.parse_path(PathStyle::Type)))) }, "meta" => token::NtMeta(panictry!(p.parse_meta_item())), - _ => { - p.span_fatal_help(sp, - &format!("invalid fragment specifier `{}`", name), - "valid fragment specifiers are `ident`, `block`, \ - `stmt`, `expr`, `pat`, `ty`, `path`, `meta`, `tt` \ - and `item`").emit(); - panic!(FatalError); - } + // this is not supposed to happen, since it has been checked + // when compiling the macro. + _ => p.span_bug(sp, "invalid fragment specifier") } } diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index 41d3991aee8..36d705f3596 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -299,7 +299,7 @@ pub fn compile<'cx>(cx: &'cx mut ExtCtxt, }; for lhs in &lhses { - check_lhs_nt_follows(cx, lhs, def.span); + valid &= check_lhs_nt_follows(cx, lhs); } let rhses = match **argument_map.get(&rhs_nm.name).unwrap() { @@ -330,19 +330,19 @@ pub fn compile<'cx>(cx: &'cx mut ExtCtxt, // why is this here? because of https://github.com/rust-lang/rust/issues/27774 fn ref_slice(s: &A) -> &[A] { use std::slice::from_raw_parts; unsafe { from_raw_parts(s, 1) } } -fn check_lhs_nt_follows(cx: &mut ExtCtxt, lhs: &TokenTree, sp: Span) { +fn check_lhs_nt_follows(cx: &mut ExtCtxt, lhs: &TokenTree) -> bool { // lhs is going to be like TokenTree::Delimited(...), where the // entire lhs is those tts. Or, it can be a "bare sequence", not wrapped in parens. match lhs { - &TokenTree::Delimited(_, ref tts) => { - check_matcher(cx, &tts.tts); - }, - tt @ &TokenTree::Sequence(..) => { - check_matcher(cx, ref_slice(tt)); - }, - _ => cx.span_err(sp, "invalid macro matcher; matchers must be contained \ - in balanced delimiters or a repetition indicator") - }; + &TokenTree::Delimited(_, ref tts) => check_matcher(cx, &tts.tts), + tt @ &TokenTree::Sequence(..) => check_matcher(cx, ref_slice(tt)), + _ => { + cx.span_err(lhs.get_span(), + "invalid macro matcher; matchers must be contained \ + in balanced delimiters or a repetition indicator"); + false + } + } // we don't abort on errors on rejection, the driver will do that for us // after parsing/expansion. we can report every error in every macro this way. } @@ -364,20 +364,25 @@ struct OnFail { action: OnFailAction, } -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq)] enum OnFailAction { Warn, Error, DoNothing } impl OnFail { fn warn() -> OnFail { OnFail { saw_failure: false, action: OnFailAction::Warn } } fn error() -> OnFail { OnFail { saw_failure: false, action: OnFailAction::Error } } fn do_nothing() -> OnFail { OnFail { saw_failure: false, action: OnFailAction::DoNothing } } - fn react(&mut self, cx: &mut ExtCtxt, sp: Span, msg: &str) { + fn react(&mut self, cx: &mut ExtCtxt, sp: Span, msg: &str, help: Option<&str>) { match self.action { OnFailAction::DoNothing => {} - OnFailAction::Error => cx.span_err(sp, msg), + OnFailAction::Error => { + let mut err = cx.struct_span_err(sp, msg); + if let Some(msg) = help { err.span_help(sp, msg); } + err.emit(); + } OnFailAction::Warn => { - cx.struct_span_warn(sp, msg) - .span_note(sp, "The above warning will be a hard error in the next release.") + let mut warn = cx.struct_span_warn(sp, msg); + if let Some(msg) = help { warn.span_help(sp, msg); } + warn.span_note(sp, "The above warning will be a hard error in the next release.") .emit(); } }; @@ -385,7 +390,7 @@ impl OnFail { } } -fn check_matcher(cx: &mut ExtCtxt, matcher: &[TokenTree]) { +fn check_matcher(cx: &mut ExtCtxt, matcher: &[TokenTree]) -> bool { // Issue 30450: when we are through a warning cycle, we can just // error on all failure conditions (and remove check_matcher_old). @@ -400,6 +405,9 @@ fn check_matcher(cx: &mut ExtCtxt, matcher: &[TokenTree]) { OnFail::warn() }; check_matcher_new(cx, matcher, &mut on_fail); + // matcher is valid if the new pass didn't see any error, + // or if errors were considered warnings + on_fail.action != OnFailAction::Error || !on_fail.saw_failure } // returns the last token that was checked, for TokenTree::Sequence. @@ -435,11 +443,11 @@ fn check_matcher_old<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token, on_fai // sequence, which may itself be a sequence, // and so on). on_fail.react(cx, sp, - &format!("`${0}:{1}` is followed by a \ - sequence repetition, which is not \ - allowed for `{1}` fragments", - name, frag_spec) - ); + &format!("`${0}:{1}` is followed by a \ + sequence repetition, which is not \ + allowed for `{1}` fragments", + name, frag_spec), + None); Eof }, // die next iteration @@ -456,8 +464,10 @@ fn check_matcher_old<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token, on_fai // If T' is in the set FOLLOW(NT), continue. Else, reject. match (&next_token, is_in_follow(cx, &next_token, &frag_spec.name.as_str())) { - (_, Err(msg)) => { - on_fail.react(cx, sp, &msg); + (_, Err((msg, _))) => { + // no need for help message, those messages + // are never emitted anyway... + on_fail.react(cx, sp, &msg, None); continue } (&Eof, _) => return Some((sp, tok.clone())), @@ -466,7 +476,7 @@ fn check_matcher_old<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token, on_fai on_fail.react(cx, sp, &format!("`${0}:{1}` is followed by `{2}`, which \ is not allowed for `{1}` fragments", name, frag_spec, - token_to_string(next))); + token_to_string(next)), None); continue }, } @@ -494,7 +504,8 @@ fn check_matcher_old<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token, on_fai delim.close_token(), Some(_) => { on_fail.react(cx, sp, "sequence repetition followed by \ - another sequence repetition, which is not allowed"); + another sequence repetition, which is not allowed", + None); Eof }, None => Eof @@ -514,7 +525,7 @@ fn check_matcher_old<'a, I>(cx: &mut ExtCtxt, matcher: I, follow: &Token, on_fai Some(&&TokenTree::Delimited(_, ref delim)) => delim.close_token(), Some(_) => { on_fail.react(cx, sp, "sequence repetition followed by another \ - sequence repetition, which is not allowed"); + sequence repetition, which is not allowed", None); Eof }, None => Eof @@ -810,7 +821,11 @@ fn check_matcher_core(cx: &mut ExtCtxt, TokenTree::Token(sp, ref tok) => { let can_be_followed_by_any; if let Err(bad_frag) = has_legal_fragment_specifier(tok) { - on_fail.react(cx, sp, &format!("invalid fragment specifier `{}`", bad_frag)); + on_fail.react(cx, sp, + &format!("invalid fragment specifier `{}`", bad_frag), + Some("valid fragment specifiers are `ident`, `block`, \ + `stmt`, `expr`, `pat`, `ty`, `path`, `meta`, `tt` \ + and `item`")); // (This eliminates false positives and duplicates // from error messages.) can_be_followed_by_any = true; @@ -884,8 +899,8 @@ fn check_matcher_core(cx: &mut ExtCtxt, if let MatchNt(ref name, ref frag_spec) = *t { for &(sp, ref next_token) in &suffix_first.tokens { match is_in_follow(cx, next_token, &frag_spec.name.as_str()) { - Err(msg) => { - on_fail.react(cx, sp, &msg); + Err((msg, help)) => { + on_fail.react(cx, sp, &msg, Some(help)); // don't bother reporting every source of // conflict for a particular element of `last`. continue 'each_last; @@ -907,7 +922,9 @@ fn check_matcher_core(cx: &mut ExtCtxt, name=name, frag=frag_spec, next=token_to_string(next_token), - may_be=may_be)); + may_be=may_be), + None + ); } } } @@ -978,7 +995,7 @@ fn can_be_followed_by_any(frag: &str) -> bool { /// break macros that were relying on that binary operator as a /// separator. // when changing this do not forget to update doc/book/macros.md! -fn is_in_follow(_: &ExtCtxt, tok: &Token, frag: &str) -> Result { +fn is_in_follow(_: &ExtCtxt, tok: &Token, frag: &str) -> Result { if let &CloseDelim(_) = tok { // closing a token tree can never be matched by any fragment; // iow, we always require that `(` and `)` match, etc. @@ -1027,7 +1044,10 @@ fn is_in_follow(_: &ExtCtxt, tok: &Token, frag: &str) -> Result { // harmless Ok(true) }, - _ => Err(format!("invalid fragment specifier `{}`", frag)) + _ => Err((format!("invalid fragment specifier `{}`", frag), + "valid fragment specifiers are `ident`, `block`, \ + `stmt`, `expr`, `pat`, `ty`, `path`, `meta`, `tt` \ + and `item`")) } } } diff --git a/src/test/compile-fail/invalid-macro-matcher.rs b/src/test/compile-fail/invalid-macro-matcher.rs index 03bcaab4a9d..a0ac5d4c720 100644 --- a/src/test/compile-fail/invalid-macro-matcher.rs +++ b/src/test/compile-fail/invalid-macro-matcher.rs @@ -9,7 +9,7 @@ // except according to those terms. macro_rules! invalid { - _ => (); //~^ ERROR invalid macro matcher + _ => (); //~ ERROR invalid macro matcher } fn main() { diff --git a/src/test/compile-fail/macro-invalid-fragment-spec.rs b/src/test/compile-fail/macro-invalid-fragment-spec.rs new file mode 100644 index 00000000000..ca6cd664e73 --- /dev/null +++ b/src/test/compile-fail/macro-invalid-fragment-spec.rs @@ -0,0 +1,19 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +macro_rules! foo( + ($x:foo) => () + //~^ ERROR invalid fragment specifier + //~| HELP valid fragment specifiers are +); + +fn main() { + foo!(foo); +} diff --git a/src/test/compile-fail/macro-missing-delimiters.rs b/src/test/compile-fail/macro-missing-delimiters.rs new file mode 100644 index 00000000000..eed2a207e89 --- /dev/null +++ b/src/test/compile-fail/macro-missing-delimiters.rs @@ -0,0 +1,17 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +macro_rules! baz( + baz => () //~ ERROR invalid macro matcher; +); + +fn main() { + baz!(baz); +}