Remove TrailingToken.

[nnethercote]

It's used in Parser::collect_tokens_trailing_token to decide whether to capture a trailing token. But the callers actually know whether to capture a trailing token, so it's simpler for them to just pass in a bool.

Also, the TrailingToken::Gt case was weird, because it didn't result in a trailing token being captured. It could have been subsumed by the TrailingToken::MaybeComma case, and it effectively is in the new code.

r? @petrochenkov

[petrochenkov]

I would expect a next_token == token::Comma here, because match arms are separated by commas.

I suspect this may cause issues with configured out arms inside derive/cfg_eval.

#[cfg_eval]
#[proc_macro_that_tries_to_parse_it_again] // will fail with a syntax error because the comma
                                           // is not removed from the token representation?
fn foo() {
  match 10 {
    #[cfg(FALSE)]
    10 => true,
     _ => false,
  }
}

[petrochenkov]

Including commas into the node's tokens in node1, node2, node3 doesn't feel right, but maybe it's ok if proc macro attributes are not supported on this kind of nodes, only cfgs.
That's assuming a proc macro should only see node's tokens without comma, and cfg(FALSE) should remove tokens including tokens.

On the other hand, proc macros probably should see the trailing comma as a part of their input, so they could correctly expand into nothing if necessary, like cfg(FALSE).
That would be a change in behavior, but it should only affect unstable proc macros (?).
That would eliminate the need in TrailingToken::MaybeComma entirely, because the commas would always be included and would never be trailing.

[petrochenkov]

Okay, maybe I'm a bit confused about the way in which the current machinery works, because it's not very consistent.
Maybe it depends on the parser code specifics whether capture_trailing should be set to true, not on node kinds.

If we parse like this

collect_tokens(parse_expr);
eat_comma();

then we should pass capture_trailing = true to collect_tokens so it picks up the comma even if it's outside of collect_tokens.

If we parse like this

collect_tokens(parse_expr_and_comma);

then we should pass capture_trailing = false to collect_tokens because the comma is already inside collect_tokens and is collected.

Either way the comma will be collected, just through slightly different means, and will be included into the token stream for the expression.

I'm actually not sure anymore, will the commas appear in the token streams that my_proc_macro sees in

[#[my_proc_macro] a, #[my_proc_macro] b, #[my_proc_macro] c];

in the first two invocations?

[nnethercote]

I'm not sure the answers to your questions. This is meant to be a refactoring that just relies on local code reasoning.

E.g. for this code:

            TrailingToken::Semi => {
                assert_eq!(self.token.kind, token::Semi);
                true
            }

It's a bit silly to assert that self.token.kind equals token::Semi, because all the call sites that pass TrailingToken::Semi do so after checking that self.token.kind == token::Semi. That's the basic observation that is the starting point for this PR.

[petrochenkov]

As a refactoring this looks good to me, even if the pre-existing behavior is somewhat questionable.
r=me after fixing #127842 (comment).

[bors]

☔ The latest upstream changes (presumably #127892) made this pull request unmergeable. Please resolve the merge conflicts.

[nnethercote]

I fixed the mistaken case and rebased.

@bors r=petrochenkov rollup

[bors]

📌 Commit 487802d has been approved by petrochenkov

It is now in the queue for this repository.