Make sure that macros that didn't pass LHS checking are not expanded. #33713
Conversation
|
r? @nrc (rust_highfive has picked a reviewer for you, use r? to override) |
|
Can you add a compile-fail test for this? |
|
See #29231. |
LeoTestard
force-pushed
the
macro-rules-invalid-lhs
branch
from
86fc91e to
b00a85d
Compare
|
Added two tests, also made sure that the help message indicating which fragment specifiers are valid is still emitted. This required to make |
LeoTestard
force-pushed
the
macro-rules-invalid-lhs
branch
2 times, most recently
from
49fcbcb to
ab351a1
Compare
|
r? @pnkfelix |
| @@ -291,17 +291,16 @@ pub fn compile<'cx>(cx: &'cx mut ExtCtxt, | |||
| let lhses = match **argument_map.get(&lhs_nm.name).unwrap() { | |||
| MatchedSeq(ref s, _) => { | |||
| s.iter().map(|m| match **m { | |||
| MatchedNonterminal(NtTT(ref tt)) => (**tt).clone(), | |||
| MatchedNonterminal(NtTT(ref tt)) => { | |||
| valid &= check_lhs_nt_follows(cx, lhs); | |||
There was a problem hiding this comment.
Right, looks like I copy-pasted too fast...
LeoTestard
force-pushed
the
macro-rules-invalid-lhs
branch
2 times, most recently
from
ccabeab to
40ddcce
Compare
| &TokenTree::Delimited(_, ref tts) => { | ||
| check_matcher(cx, &tts.tts); | ||
| }, | ||
| tt @ &TokenTree::Sequence(..) => { |
There was a problem hiding this comment.
I don't know if we have code formatting conventions for macro_rules but I think it looked better before (more uniform in terms of identifying the left and right hand sides of each rule )
There was a problem hiding this comment.
(To be clear I am not saying you must change if; I don't even care enough to look up whether we have established convention here)
|
r=me once above comments are addressed |
LeoTestard
force-pushed
the
macro-rules-invalid-lhs
branch
from
40ddcce to
0169dca
Compare
|
@pnkfelix Fixed the test to include the help message. |
|
@LeoTestard Travis says you've put the expected error on the wrong line in your test (that, or you need to adjust the span you use in the error reporting) |
|
Actually the span is fine, the error was in the test. |
This avoids duplicate errors for things like invalid fragment specifiers, or parsing errors for ambiguous macros. Fixes rust-lang#29231.
LeoTestard
force-pushed
the
macro-rules-invalid-lhs
branch
from
0169dca to
7d52144
Compare
|
r? @pnkfelix |
|
@bors r+ |
|
📌 Commit 7d52144 has been approved by |
|
⌛ Testing commit 7d52144 with merge da66f2f... |
Make sure that macros that didn't pass LHS checking are not expanded. This avoid duplicate errors for things like invalid fragment specifiers, or parsing errors for ambiguous macros.
This avoid duplicate errors for things like invalid fragment specifiers, or
parsing errors for ambiguous macros.