Change behavior of `?` as a macro separator and Kleene op in 2018 edition

[mark-i-m]

This is a request for FCP on PR #51587, according to #51587 (comment).

Tracking issue: #48075 (? macro repetition)

Current behavior:

• ? is a macro separator, not a Kleene op.

Proposed new behavior (implemented in #51587):

• Edition 2015: ? is a macro separator, not a kleene op, but using it as a separator triggers a migration lint.
• Edition 2018: ? is not a valid separator. ? is a valid Kleene op.

EDIT: To clarify: Under this proposal, $(a)?+ matches + and a+ unambiguously.

Why we chose this behavior:

• It's the cleanest choice both in terms of end-user experience and implementation. See the alternatives.

Alternatives: The main problem to address is ambiguity. Currently, ? is a valid separator, so there are a lot of ambiguous possibilities like $(a)?+. Is the ? a separator for 1 or more iterations OR is it a zero-or-one kleene op followed by a + token? The solutions fall into 2 categories: make breaking changes or add fallbacks.

1. Fallbacks

   • The original implementation (currently on nightly behind feature gate macro_at_most_once_rep):
     • The ? kleene op is allowed to take a separator which can never be used (we don't allow trailing separators and ? allows at most one repetition).
     • When parsing a macro, we disambiguate by looking after the separator: if ? is followed by +, *, or ?, we treat the ? as a separator and the second token as a kleene op.
   • On the tracking issue, we decided that it is confusing for ? to take a separator, so the rules would be that ? followed by + or * is a separator, but ? followed by ? or any other token would be a ? kleene op followed by the other token.
     • This is a viable implementation, but it seems like a lot of special cases and the code gets messy. This led us more towards the breaking changes options.
   • There are probably lots of variations of fallbacks we could do, but they all have weird corner cases.

2. Breaking changes

   • Make a breaking change in the 2015 edition (implemented in Update ? repetition disambiguation. #49719, accidentally merged without FCP, and rolled back).
     • Disallow ? as a separator in 2015 edition.
     • Disallow ? kleene op from taking a separator.
     • This means that $(a)?+ matches + and a+ unambiguously. However, it is a breaking change. This was deemed not worthy of a breaking change in the 2015 edition.
   • Make a breaking change in the 2018 edition and lint in the 2015 edition. No change to behavior in 2015. This is the current proposal implemented in 2018 edition ? Kleene operator #51587 and is the subject of this FCP.
     • The primary downside here is that it's not clear what happens if you import a macro from 2015 into 2018 that uses ? as a separator. Currently, you would just get a compile error.
     • I believe the breakage is likely to be extremely rare. A crater run showed no breakage (Update ? repetition disambiguation. #49719 (comment)). However, we did found out later that there was apparently some breakage elsewhere (Update ? repetition disambiguation. #49719 (comment)).
     • The implementation is significantly cleaner and more maintainable and the behavior is more consistent for end-users.

cc @nikomatsakis
cc @petrochenkov Reviewed the second attempt
cc @durka @alexreg @clarcharr @Centril @kennytm @ExpHP Who participated extensively in the original design discussions (EDIT: apologies if I missed anyone; there were a lot of people who came in at various points)
cc @oli-obk Who brought up clippy breakage
cc @sgrif @pietroalbini Who wanted to see an FCP

Whew... that's a lot of people :)

[alexreg]

Nice work, @mark-i-m. I think we finally have a fully appropriate solution in all cases, at least as I understand it. LTGM!

[petrochenkov]

I think this is too much hassle over nothing.
This is a very useful feature and crater found zero regressions, we should make a "breaking change" implement it on edition 2015.

The one symbol lookup checking for another Kleene op and reducing the breakage from zero to absolute zero also doesn't look like too much complexity or special casing to me.

Also, the separator should be allowed on both left-hand and right-hand sides of a macro (this is the first thing I tried to use when trying ? in 399da7b), or conservatively accepted and reported as a non-fatal error.

[emilyalbini]

I don't really see a point in making breaking changes like this in the current edition, especially now that we have editions and the next one is right around the corner.

[kennytm]

What would happen for macro definitions across editions?

// edition (4033 - A)
macro_rules! define_macro {
    ($($t:tt)*) => { 
        macro_rules! use_macro {
            ($($t)*) => {}
        }
    }
}

// edition A
define_macro!($(a)?*);

use_macro!(a?a?a);  // ???
use_macro!(a*);     // ???

[mark-i-m]

@kennytm In the current design, if you use ? as a separator in 2018, you get an error. Perhaps this is a rare enough case that that's ok? The fix is frankly trivial: use any other separator than ?...

[ExpHP]

@kennytm In the current design, if you use ? as a separator in 2018, you get an error. Perhaps this is a rare enough case that that's ok? The fix is frankly trivial: use any other separator than ?...

@mark-i-m but can a 2018 crate use a macro defined in a 2015 crate that uses ? as a separator?

(going even further down the rabbit hole, there is the possibility of a 2015 crate with a macro that generates a macro_rules! definition that uses ? as a separator. ISTM that this clearly would not be callable from 2018... however, the odds of any such macro existing seem to be a trillion to one)

[ExpHP]

Also, it is unclear to me from the summary, but:

This means that $(a)?+ matches + and a+ unambiguously.

Is this describing the new proposed behavior in 2018, or the behavior of an alternative design? It seems desirable to me.

[mark-i-m]

@ExpHP

@mark-i-m but can a 2018 crate use a macro defined in a 2015 crate that uses ? as a separator?

Sorry, perhaps I misunderstanding. What is the distinction from @kennytm's point?

Yes, in 2018 (according to the current plan) if you were to use ? as a separator (even coming from a 2015 crate), you would get an error. The expectation is that this case is unlikely enough and easy enough to fix that we are ok with the breakage across an edition boundary.

This means that $(a)?+ matches + and a+ unambiguously.

Is this describing the new proposed behavior in 2018, or the behavior of an alternative design? It seems desirable to me.

Yes, this describes the proposed behavior (implemented in #51587).

[ExpHP]

What is the distinction from @kennytm's point?

None that I know of, it just sounded to me like you and I interpreted his question differently, especially due to the quote

The fix is frankly trivial: use any other separator than ?...

which is not something a 2018 consumer of a 2015 macro crate can do.

[mark-i-m]

which is not something a 2018 consumer of a 2015 macro crate can do.

Yes, that's true. However, I believe the fact that the fix is easy mitigates this a bit, since you can often make a PR to fix the problem, and a PR with this fix ought to be easy to review.

Again, I agree that this is not a super great situation, but I expect it to be rare.

[nikomatsakis]

Ignoring the state of the current implementation: I believe the expected behavior for the cross-crate case would be that a macro defined in the 2015 Edition crate would not treat ? as "0 or 1" but rather as separator. Put another way, the behavior of macros across crates is a somewhat orthogonal issue. (We have an open issue or two on that topic, and it does need attention and resolution, don't get me wrong.)