Skip to content

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

Closed
@mark-i-m

Description

@mark-i-m
Member

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 :)

Activity

alexreg

alexreg commented on Jun 30, 2018

@alexreg
Contributor

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

petrochenkov commented on Jun 30, 2018

@petrochenkov
Contributor

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.

pietroalbini

pietroalbini commented on Jun 30, 2018

@pietroalbini
Member

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

kennytm commented on Jun 30, 2018

@kennytm
Member

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

mark-i-m commented on Jun 30, 2018

@mark-i-m
MemberAuthor

@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

ExpHP commented on Jun 30, 2018

@ExpHP
Contributor

@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

ExpHP commented on Jun 30, 2018

@ExpHP
Contributor

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

mark-i-m commented on Jun 30, 2018

@mark-i-m
MemberAuthor

@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

ExpHP commented on Jun 30, 2018

@ExpHP
Contributor

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

mark-i-m commented on Jun 30, 2018

@mark-i-m
MemberAuthor

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

nikomatsakis commented on Jul 2, 2018

@nikomatsakis
Contributor

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.)

40 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    T-langRelevant to the language teamdisposition-mergeThis issue / PR is in PFCP or FCP with a disposition to merge it.finished-final-comment-periodThe final comment period is finished for this PR / Issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @alexreg@kennytm@nikomatsakis@ExpHP@sgrif

        Issue actions

          Change behavior of `?` as a macro separator and Kleene op in 2018 edition · Issue #51934 · rust-lang/rust