Skip to content

macros: Clean up code with non-optional NonterminalKind #142657

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tgross35
Copy link
Contributor

Since 1, the fragment specifier is unconditionally required in all
editions. This means NonTerminalKind no longer needs to be optional,
as we can reject this code during the expansion of macro_rules! rather
than handling it throughout the code. Do this cleanup here.

r? @petrochenkov

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 18, 2025
@tgross35 tgross35 force-pushed the nonoptional-fragment-specifiers-cleanup branch from 91218e6 to 093240b Compare June 18, 2025 04:36
@tgross35
Copy link
Contributor Author

Can be reviewed by-commit, the first commit is just refactoring a heavily nested function.

Comment on lines 7 to +11
fn main() {
m!();
m!();
m!();
m!();
m!(); //~ ERROR unexpected end
m!(); //~ ERROR unexpected end
m!(); //~ ERROR unexpected end
m!(); //~ ERROR unexpected end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why these new "unexpected end" errors are showing up at the invocation rather than at macro_rules! or not at all. Any idea how to get rid of this?

Copy link
Contributor Author

@tgross35 tgross35 Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably need to emulate this behavior somewhere

let res = self.parse_tt_inner(
matcher,
&parser.token,
parser.approx_token_stream_pos(),
track,
);
if let Some(res) = res {
return res;
}
// `parse_tt_inner` handled all of `cur_mps`, so it's empty.
assert!(self.cur_mps.is_empty());
// Error messages here could be improved with links to original rules.
match (self.next_mps.len(), self.bb_mps.len()) {
(0, 0) => {
// There are no possible next positions AND we aren't waiting for the black-box
// parser: syntax error.
return Failure(T::build_failure(
parser.token,
parser.approx_token_stream_pos(),
"no rules expected this token in macro call",
));
}
(_, 0) => {
// Dump all possible `next_mps` into `cur_mps` for the next iteration. Then
// process the next token.
self.cur_mps.append(&mut self.next_mps);
parser.to_mut().bump();
}
(0, 1) => {
// We need to call the black-box parser to get some nonterminal.
let mut mp = self.bb_mps.pop().unwrap();
let loc = &matcher[mp.idx];
if let &MatcherLoc::MetaVarDecl {
span,
kind: Some(kind),
next_metavar,
seq_depth,
..
} = loc
{
// We use the span of the metavariable declaration to determine any
// edition-specific matching behavior for non-terminals.
let nt = match parser.to_mut().parse_nonterminal(kind) {
Err(err) => {
let guarantee = err.with_span_label(
span,
format!(
"while parsing argument for this `{kind}` macro fragment"
),
)
.emit();
return ErrorReported(guarantee);
}
Ok(nt) => nt,
};
mp.push_match(next_metavar, seq_depth, MatchedSingle(nt));
mp.idx += 1;
} else {
unreachable!()
}
self.cur_mps.push(mp);
}
(_, _) => {
// Too many possibilities!
return self.ambiguity_error(matcher, parser.token.span);
}
}
assert!(!self.cur_mps.is_empty());
since parse_tt_inner used to return an Err if there was no fragment specifier.

@tgross35 tgross35 force-pushed the nonoptional-fragment-specifiers-cleanup branch from 093240b to 2cff3c2 Compare June 18, 2025 04:46
Comment on lines +72 to +79
sess.dcx().emit_err(errors::MissingFragmentSpecifier {
span,
add_span: span.shrink_to_hi(),
valid: VALID_FRAGMENT_NAMES_MSG,
});

// Fall back to a `TokenTree` since that will match anything if we continue expanding.
result.push(TokenTree::MetaVarDecl { span, name: ident, kind: NonterminalKind::TT });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we emit a InvalidFragmentSpecifier (below) the fallback is Ident rather than TT. Any reason to prefer one or the other?

tgross35 added 2 commits June 18, 2025 07:18
Non-functional change to simplify control flow.
Since [1], the fragment specifier is unconditionally required in all
editions. This means `NonTerminalKind` no longer needs to be optional,
as we can reject this code during the expansion of `macro_rules!` rather
than handling it throughout the code. Do this cleanup here.

[1]: rust-lang#128425
@tgross35 tgross35 force-pushed the nonoptional-fragment-specifiers-cleanup branch from 2cff3c2 to 1ddb1c4 Compare June 18, 2025 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants