Skip to content
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

RFC: Alternatives in patterns #1500

Closed
wants to merge 1 commit into from
Closed

Conversation

White-Oak
Copy link

match new_type {
NewType(Ok(e) | Err(e), num) => println!("ok with {}: {}", num, e)
}
```

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works if for Result<T, T> , when Ok and Err both has same value type. I don't think this is good use case.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's an example, but for generalicity's sake can be replaced with Ok(_) | Err(_)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it can't. Because e is used in arm. If you don't want to use, then it may be written as NewType(_, num)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. But I've used Result not to introduce a new enum.

@main--
Copy link

main-- commented Feb 14, 2016

This RFC proposes following statements to be allowed:

if let Ok(e) | Err(e) = result {}
let Ok(e) | Err(e) = result;

Can you elaborate on this? I expect the second form to only be allowed for irrefutable patterns, just like right now. Consequently, the first example should however be disallowed as the specified pattern would have to be irrefutable (or else one the if would be redundant).

@White-Oak White-Oak force-pushed the master branch 2 times, most recently from efe9214 to 93fe27d Compare February 15, 2016 04:24
@White-Oak
Copy link
Author

@main-- I agree with both points, though irrefutable patterns may be allowed for if let statements (with a warning about unreachable code of an else-branch).

Updated a proposed RFC considering irrefutable patterns in if let and let statements.

@durka
Copy link
Contributor

durka commented Feb 15, 2016

When | was added to FOLLOW(pat), it was noted that we won't be able to extend pattern syntax to cover a | b as a single pattern, but (a | b) (required parentheses) is possible. cc @pnkfelix

@White-Oak
Copy link
Author

@durka does this affect let statements?

if let (A(e) | B(e)) = thing {}
let (A(e) | B(e) | C(e)) = thing;

Edit: Can you elaborate: does, and if yes, why this affects non-macro code?

@jonas-schievink
Copy link
Contributor

@White-Oak The macro follow sets are meant to provide a compatibility guarantee for macros. Currently A(e) | B(e) is not a valid pattern, but you can write a $p:pat | $q:pat matcher in a macro to match it. If A(e) | B(e) becomes a valid pattern, then the matcher would fail, because A(e) | B(e) is assigned to $p and the macro then expects a |.

So the issue is that this will break macros, even if it doesn't affect non-macro code.

@White-Oak
Copy link
Author

@jonas-schievink I see.
But A(e) | B(e) is used in match statements already, however. Is it not a pattern there?

@jonas-schievink
Copy link
Contributor

Nope, that's comparable to $($p:pat)|+, and the implementation would have to be changed when this RFC is accepted.

@durka
Copy link
Contributor

durka commented Feb 16, 2016

If we're careful we won't break macros, because we'll just require that top level of parentheses for it to be considered a single pattern. Alternatively we could extend the syntax of while let/if let to support multiple patterns separated by |, without changing the pattern syntax, but that's less general.

Without top-level parentheses, it would lead to ambiguities anyway, because arguments to closures are patterns; consider this code (where do the closure arguments end and the body begin?):

enum Foo { A(i32), B(i32) }

let closure = | Foo::A(i) | Foo::B(i) | { i + 1 };

@White-Oak
Copy link
Author

@durka is there a way to handle (A(i) | B(i)) as a pattern and single A(i) as a pattern (w/o parenthesis) simultaneously?

@durka
Copy link
Contributor

durka commented Feb 18, 2016

I don't see why not. But don't take my word for it.
On Feb 18, 2016 05:30, "Oak" notifications@github.com wrote:

@durka https://github.com/durka is there a way to handle (A(i) | B(i))
as a pattern and single A(i) as a pattern (w/o parenthesis)
simultaneously?


Reply to this email directly or view it on GitHub
#1500 (comment).

@nrc nrc added the T-lang Relevant to the language team, which will review and decide on the RFC. label Feb 18, 2016
@White-Oak White-Oak force-pushed the master branch 2 times, most recently from fe4fc9c to 33c96a3 Compare February 19, 2016 14:21
@White-Oak
Copy link
Author

Updated RFC adressing "requirement of surrounding parens" for multiple variants.
I'm not sure:

  • If it is possible to treat single variant w/o parens as a single pattern (as it is treated today) simultaneously with treating multiple variants with parens as a single pattern as well. (@durka supposed it may be possible above).
  • Should deeper levels of patterns be enclosed in parens like this:
match {
  NewType((Ok(e) | Err(e)), num) => () 
}

or is it possible to omit parens:

match {
  NewType(Ok(e) | Err(e), num) => () 
}

@White-Oak White-Oak force-pushed the master branch 2 times, most recently from 3996484 to e5dd60f Compare February 19, 2016 14:30
@aturon
Copy link
Member

aturon commented Feb 26, 2016

@White-Oak Thanks for the RFC!

The lang team discussed the RFC while attempting to find a shepherd for it. Our feeling is that, while we may eventually want to support such a syntax, there is not strong enough motivation at present to make it a priority, especially with so much else in flight in the compiler with the transition to MIR. As such, we're going to close as postponed.

In a future incarnation of this RFC, we'd like to see more detailed motivation with real-world use-cases where the feature makes a significant impact.

@aturon aturon closed this Feb 26, 2016
@aturon aturon added the postponed RFCs that have been postponed and may be revisited at a later time. label Feb 26, 2016
@mbrubeck
Copy link
Contributor

New RFC for this feature: #2535.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
postponed RFCs that have been postponed and may be revisited at a later time. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants