Make sure that macros that didn't pass LHS checking are not expanded.

This avoids duplicate errors for things like invalid fragment specifiers, or
parsing errors for ambiguous macros. Fixes #29231.
This commit is contained in:
Leo Testard 2016-05-18 15:08:19 +02:00
parent 310d8996f4
commit eb364e9c29
5 changed files with 93 additions and 42 deletions

View File

@ -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")
}
}

View File

@ -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<A>(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<bool, String> {
fn is_in_follow(_: &ExtCtxt, tok: &Token, frag: &str) -> Result<bool, (String, &'static str)> {
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<bool, String> {
// 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`"))
}
}
}

View File

@ -9,7 +9,7 @@
// except according to those terms.
macro_rules! invalid {
_ => (); //~^ ERROR invalid macro matcher
_ => (); //~ ERROR invalid macro matcher
}
fn main() {

View File

@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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);
}

View File

@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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);
}