Skip to content

/rfcs forked from rust-lang/rfcs

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

[DRAFT] RFC: or patterns #12

Closed
wants to merge 19 commits into from

Conversation

Reviewers

@Nemo157 Nemo157

@joshtriplett joshtriplett

Assignees
No one assigned
Labels
draft rfc
Projects
None yet
Milestone
No milestone
3 participants
@Centril @Nemo157 @joshtriplett
@Centril
Copy link
Owner

Centril commented Aug 29, 2018

This is a draft version of an RFC for you to review, before a formal proposal is made for consideration.

@Centril Centril added rfc draft labels Aug 29, 2018

@Nemo157

Nemo157 reviewed Aug 29, 2018
View changes

text/0000-or-patterns.md Outdated
r: Option<i32>) -> ParseResult<Option<i32>> {
match (y, q, r) {
(y, None, None) => Ok(y),
(Some(y), q, r @ (Some(0...99) None)) => { ... },

This comment has been minimized.

Sign in to view
Copy link
@Nemo157

Nemo157 Aug 29, 2018

Missing | between the Some and None (I assume, Sourcegraph appears to not be Safari compatible so I can't view the original)
@Nemo157

Nemo157 reviewed Aug 29, 2018
View changes

text/0000-or-patterns.md Outdated

```rust
match (op, &mmp.clone()) {
(&Lt, &M(0) | &MM(0, 0) | &MMP(0, 0, 0)) => debcargo_bail!(

This comment has been minimized.

Sign in to view
Copy link
@Nemo157

Nemo157 Aug 29, 2018

Could be simplified slightly more to (&Lt, &(M(0) | MM(0, 0) | MMP(0, 0, 0))) (and similar for the other branches).
@Nemo157

Nemo157 reviewed Aug 29, 2018
View changes

text/0000-or-patterns.md Outdated

+ the type inferred for `p` does not unify with the type inferred for `q`, or
+ the same set of bindings are not introduced in `p` and `q`, or
+ the type two bindings with the same name in `p` and `q` do not unify.

This comment has been minimized.

Sign in to view
Copy link
@Nemo157

Nemo157 Aug 29, 2018

missing something between "type two", maybe "the type of any two bindings"?
@Nemo157

Nemo157 reviewed Aug 29, 2018
View changes

text/0000-or-patterns.md Outdated
Thus, we conjecture that it's more common for humans to not distribute and to
instead use something akin to *conjunctive normal form* ([CNF]) when communicating.
A likely consequence of this is that a common way to understand snippet (1)
formulated in [DNF] is to first mentally reconstruct it into CNF and then

This comment has been minimized.

Sign in to view
Copy link
@Nemo157

Nemo157 Aug 29, 2018

Missing the expansion for DNF
@joshtriplett

joshtriplett reviewed Aug 29, 2018
View changes

text/0000-or-patterns.md Outdated

Here `|` has the lowest precedence.
In particular, the operator `@` binds more tightly than `|` does.
Thus, `i @ p | q` associates as `i @ (p | q)` as opposed to `(i @ p) | q`.

This comment has been minimized.

Sign in to view
Copy link
@joshtriplett

joshtriplett Aug 29, 2018

This seems inconsistent with the precedence shown elsewhere in the RFC.
text/0000-or-patterns.md Outdated

Allow `|` to be arbitrarily nested within a pattern such
that `Some(A(0) | B(1 | 2))` becomes a valid pattern.
The operator `|` has low precedence such that `i @ p | q` means `(i @ p) | q`.

This comment has been minimized.

Sign in to view
Copy link
@joshtriplett

joshtriplett Aug 29, 2018

Can you cross-reference the explanation of this precedence? My first instinct was to prefer to see @ as the lowest-precedence thing in patterns, unless there's some specific compatibility reason to do otherwise, but at the same time I could readily be convinced otherwise if you have a rationale for this.

Also, I don't think you need to put the precedence in the summary; you can put it later in the document.
text/0000-or-patterns.md Outdated
+ Farsi (3)
+ Finnish (1)
+ Esperanto (1)
+ Japanese (1)

This comment has been minimized.

Sign in to view
Copy link
@joshtriplett

joshtriplett Aug 29, 2018

Nit: This is a fun detail, but I don't think it needs to be a bulleted list; just inline it as a flat comma-separated list in the sentence.
text/0000-or-patterns.md Outdated
[RFC 2175]: https://github.com/rust-lang/rfcs/pull/2175

In concrete terms, where before we only allowed a pattern of the form
`'|'? pat | pat` at the top level of `match` and [similar constructs][RFC 2175],

This comment has been minimized.

Sign in to view
Copy link
@joshtriplett

joshtriplett Aug 29, 2018

I think including the optional leading vert here obfuscates the point you're trying to make. Just say pat | pat in both places.
text/0000-or-patterns.md Outdated

6. Some further examples found with sourcegraph include:

From [cc-rs](https://sourcegraph.com/github.com/alexcrichton/cc-rs/-/blob/src/lib.rs#L1307:18):

This comment has been minimized.

Sign in to view
Copy link
@joshtriplett

joshtriplett Aug 29, 2018

For all of these citation links, please link to a specific commit hash, so that the line number references remain working in the future.
text/0000-or-patterns.md Outdated
```

Note that the operator `|` has a low precedence. This means that you
have to write `foo @ (1 | 2 | 3)` instead of writing `foo @ 1 | 2 | 3`.

This comment has been minimized.

Sign in to view
Copy link
@joshtriplett

joshtriplett Aug 29, 2018

This phrasing seems confusing; "you have to write" suggests that this is what you'd want in the common case, in which case let's set the precedence to make that case easier to write.
text/0000-or-patterns.md Outdated

+ the type inferred for `p` does not unify with the type inferred for `q`, or
+ the same set of bindings are not introduced in `p` and `q`, or
+ the type of any two bindings with the same name in `p` and `q` do not unify.

This comment has been minimized.

Sign in to view
Copy link
@joshtriplett

joshtriplett Aug 29, 2018

Or the binding mode; for instance, if you bind the same name as mut in p and non-mut in q.
text/0000-or-patterns.md Outdated
in CNF to its equivalent form in DNF, i.e. `c(p) | c(q)`.

However, implementing `c(p | q)` in terms of a pure desugaring to `c(p) | c(q)`
may not be optimal as the desugaring can result in exponential blow-up of patterns.

This comment has been minimized.

Sign in to view
Copy link
@joshtriplett

joshtriplett Aug 29, 2018

Please give a concrete example here. (Also, s/exponential/multiplicative/.)

For an example, (0|1, 0|1, 0|1, 0|1, ...) seems like the simplest one to understand.
text/0000-or-patterns.md Outdated
this change should be done rather than *if*.

With respect to the precedence of `|`, we already cannot interpret `i @ p | q`
as `i @ (p | q)` because it is already legal to write `i @ p | j @ q`.

This comment has been minimized.

Sign in to view
Copy link
@joshtriplett

joshtriplett Aug 29, 2018

Can you add an "at the top level of a pattern" here, to make it clear that this kind of alternation currently only works at the top level?
text/0000-or-patterns.md Outdated
as `i @ (p | q)` because it is already legal to write `i @ p | j @ q`.
Therefore, if we say that `|` binds more tightly,
then `i @ p | j @ q` will associate as `i @ (p | j @ q)` which as a different
meaning than what we currently have, thus causing a breaking change.

This comment has been minimized.

Sign in to view
Copy link
@joshtriplett

joshtriplett Aug 29, 2018

OK, fair enough, this makes sense.

However, could you please specify a lint and suggestion for this? Specifically, if you write i @ p | q, you will already get an error because the two patterns (i@p and q) don't both bind i, so with a bit of extra error-reporting logic we could add a suggestion for i @ (p | q).
@joshtriplett

joshtriplett reviewed Aug 30, 2018
View changes

text/0000-or-patterns.md
we should try to keep the language more uniform rather than less.
This is an important means through which it becomes possible to give users
more expressiveness but at the same time limit the cost each feature takes
from our complexity budget.

This comment has been minimized.

Sign in to view
Copy link
@joshtriplett

joshtriplett Aug 30, 2018

I love this whole paragraph. I'd love to see it added to some top-level documentation of the RFC repository, or some kind of language design philosophy document.
@joshtriplett

joshtriplett reviewed Aug 30, 2018
View changes

text/0000-or-patterns.md Outdated
| |
| variable not in all patterns
|
| hint: if you wanted `i` to cover both cases, try adding parenthesis around:

This comment has been minimized.

Sign in to view
Copy link
@joshtriplett

joshtriplett Aug 30, 2018

"parentheses", plural.
@Centril

This comment has been minimized.

Sign in to view
Copy link
Owner Author

Centril commented Aug 30, 2018

Thanks all! Time to :shipit:

@Centril Centril closed this Aug 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.