Better error message on invalid fragment specifiers in macros #21158
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nick29581 (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see CONTRIBUTING.md for more information. |
| @@ -453,7 +453,7 @@ fn is_in_follow(cx: &ExtCtxt, tok: &Token, frag: &str) -> bool { | |||
| // harmless | |||
| true | |||
| }, | |||
| _ => cx.bug(format!("unrecognized builtin nonterminal {}", | |||
| frag).as_slice()), | |||
| _ => cx.span_fatal(sp, format!("unrecognized builtin nonterminal {}", | |||
There was a problem hiding this comment.
bleugh, I was trying to format backticks/graves there
|
r=me with the nit about backticks fixed, if you want to add a helpful message, that would be great, but its not necessary to fix this bug. Thanks for the PR! |
| _ => { | ||
| // invalid fragment specifier. Just pass it along, it will be | ||
| // caught in macro_parser::parse_nt() | ||
| true |
There was a problem hiding this comment.
I'm not sure about silently accepting invalid specifiers here.
Alternative is calling span_help() and span_fatal(), but I didn't want to duplicate error messages from parse_nt.
There was a problem hiding this comment.
@cmr - do you think this is OK? It seems OK to me, but also a little bit bad.
There was a problem hiding this comment.
This seems reasonable to me, should add in a comment in parse_nt that this bit of code depends on it rejecting invalid fragment specifiers.
|
re-r? @nick29581 |
Also, print help on valid fragment specifiers.
alkor
changed the title
|
@nick29581 Issue #21131 is already fixed in master. So after rebase this PR only modifies error messages to match terminology from the macros guide. |
|
⌛ Testing commit 8a22454 with merge 06410ef... |
Closes #21131Issue #21131 is already fixed in PR #21429.
I just edited error messages to match terminology from the macros guide ("fragment specifiers")