Skip to content

Allow leading | anywhere we allow or-patterns #81415

New issue

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

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

Already on GitHub? # to your account

Closed
joshtriplett opened this issue Jan 26, 2021 · 14 comments
Closed

Allow leading | anywhere we allow or-patterns #81415

joshtriplett opened this issue Jan 26, 2021 · 14 comments
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team

Comments

@joshtriplett
Copy link
Member

See #54883 (comment) for details.

Filing this issue to check lang-team consensus on this: do we want to allow leading | everywhere we allow or-patterns?

@joshtriplett joshtriplett added the T-lang Relevant to the language team label Jan 26, 2021
@joshtriplett
Copy link
Member Author

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Jan 26, 2021

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jan 26, 2021
@scottmcm
Copy link
Member

Somehow to me they feel a little weirder in something like matches!(foo, | Bar(_) | Baz(_)), but regardless I think allowing them is the right thing to do for consistency and the overall grammar.

@joshtriplett
Copy link
Member Author

@scottmcm In that use case, they'd likely look like this:

    matches!(
        some_long_expression,
        | SomeLongPattern
        | SomeOtherLongPattern
    )

@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

@joshtriplett
Copy link
Member Author

Checking box for boats per rust-lang/team#526 .

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Feb 2, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented Feb 2, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Feb 2, 2021
@mark-i-m
Copy link
Member

mark-i-m commented Feb 7, 2021

@mark-i-m
Copy link
Member

mark-i-m commented Feb 8, 2021

Implementation: #81869

@lukechu10
Copy link
Contributor

I think rustfmt should add a leading | when the pattern is on multiple lines but remove it when it's on a single line:

matches!(foo, Bar | Baz);

matches!(
    some_really_long_expression,
    | Bar
    | Baz
)

match foo {
    Bar | Baz => {},
    _ => {}
}

match foo {
    | Bar
    | Baz => {},
    _ => {}
}

Just for consistency

@jplatte
Copy link
Contributor

jplatte commented Feb 11, 2021

@lukechu10 Formatting is not something the compiler does. IIRC there is an issue about this on https://github.com/rust-lang/rustfmt somewhere.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Feb 12, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented Feb 12, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Feb 12, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 16, 2021
Simplify pattern grammar, improve or-pattern diagnostics

This implements the change under FCP in rust-lang#81415. It allows nested or-patterns to contain a leading `|`, simplifying the [grammar for patterns](https://github.com/rust-lang/reference/pull/957/files?short_path=cc629f1#diff-cc629f15712821139bc706c63b3845ab59a008e2a998e08ffad42e3aebcbcbe2).

Along the way, we also improve the diagnostics around a few specially-handled cases, such as using `||` instead of `|`, using or-patterns in fn params, including the leading `|` in the pattern span, etc.

r? `@petrochenkov`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 16, 2021
Simplify pattern grammar, improve or-pattern diagnostics

This implements the change under FCP in rust-lang#81415. It allows nested or-patterns to contain a leading `|`, simplifying the [grammar for patterns](https://github.com/rust-lang/reference/pull/957/files?short_path=cc629f1#diff-cc629f15712821139bc706c63b3845ab59a008e2a998e08ffad42e3aebcbcbe2).

Along the way, we also improve the diagnostics around a few specially-handled cases, such as using `||` instead of `|`, using or-patterns in fn params, including the leading `|` in the pattern span, etc.

r? ``@petrochenkov``
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 17, 2021
Simplify pattern grammar, improve or-pattern diagnostics

This implements the change under FCP in rust-lang#81415. It allows nested or-patterns to contain a leading `|`, simplifying the [grammar for patterns](https://github.com/rust-lang/reference/pull/957/files?short_path=cc629f1#diff-cc629f15712821139bc706c63b3845ab59a008e2a998e08ffad42e3aebcbcbe2).

Along the way, we also improve the diagnostics around a few specially-handled cases, such as using `||` instead of `|`, using or-patterns in fn params, including the leading `|` in the pattern span, etc.

r? ```@petrochenkov```
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 17, 2021
Simplify pattern grammar, improve or-pattern diagnostics

This implements the change under FCP in rust-lang#81415. It allows nested or-patterns to contain a leading `|`, simplifying the [grammar for patterns](https://github.com/rust-lang/reference/pull/957/files?short_path=cc629f1#diff-cc629f15712821139bc706c63b3845ab59a008e2a998e08ffad42e3aebcbcbe2).

Along the way, we also improve the diagnostics around a few specially-handled cases, such as using `||` instead of `|`, using or-patterns in fn params, including the leading `|` in the pattern span, etc.

r? `@petrochenkov`
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Feb 18, 2021
@WiSaGaN
Copy link
Contributor

WiSaGaN commented May 3, 2021

Should this be closed now that the implementation is merged?

@nikomatsakis
Copy link
Contributor

Sounds good.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team
Projects
None yet
Development

No branches or pull requests

9 participants