-
Notifications
You must be signed in to change notification settings - Fork 205
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
A "guard-let"-like statement form #2537
Comments
Just to complement, I have seen an idea about an if! (json case [int x, int y]) {
throw FormatException('Invalid JSON.');
} Edit: Or maybe |
The issue I have is that, for me, It isn't immediately obvious what the scope of unless (json case [int x, int y]) {
// x and y are not in scope
throw FormatException('Invalid JSON.');
}
print('$x,$y'); // x and y are in scope
// compared to
if (json case [int x, int y]) {
// x and y are in scope
print('$x,$y');
}
// x and y are not in scope Also, I sort of expect an It is also unintuitive if if (json case [int x, int y]) else {
// early return required
throw FormatException('Invalid JSON.');
}
// x and y in scope
print('Was coordinate array $x,$y');
// compare to
if (json case [int x, int y]) {} else {
// early exit no longer required
throw FormatException('Invalid JSON.');
}
// x and y no longer in scope |
This is an issue for me too. It's useful to have a construct like this that drops the variables into the current block scope because it lets you flatten out nesting, but it's not obvious that it does that.
Agreed on all counts. That's why I haven't found a syntax I like yet. :-/ |
I wonder whether it would help to turn the syntax on its head? test(List<Object> json) {
{
throw FormatException('Invalid JSON.');
} unless (json case [int x, int y]);
print('Was coordinate array $x,$y');
} This makes the scoping of the variables read much more naturally. |
I toyed with some ideas like that too, but never found anything I liked there. It makes the scoping more obvious "if you get here, you can use these". But I think it makes the control flow read counter to importance. "Why am I throwing an exception here? Oh wait, I'm not. Because there's a condition after this block that decides whether the preceding code is executed." |
A pattern match, using Here it sounds like we want to put the action of a non-match before action on a match. That destroys the flow, because the bindings coming out of the match are now detached (spatially) from the code which uses them. One way Swift differs from our Something similar in Dart might be: case (var x, var y) = e when x < y
else break;
// rest of block using x, y The This lifts the pattern match out of being nested inside the It's basically as shorhand for if (e case (var x, var y) when x< y) {
// rest of block
} else {
break;
} which is also why I don't think it's a very important feature. It allows you to flip branches and unnest the else branch. That's nice, but hardly essential. (Could we just introduce an |
Any specific reason why we can't make some variation of variable declaration followed by else work? final [int x, int y] = json else {
throw "";
}
case [int x, int y] = json else {
throw "";
} I really would love to see some regularity in our pattern matching (both internally and externally, with other programming languages), so it would be easy to remember the syntax and also understand what the syntax means. I guess the current reason for post-fix case in conditions is to try and match Most of variable declarations go left to right (variable is equal to expression). In pattern matching we suddenly go right to left (expression decomposed into variable). But only if pattern occurs in the I think it is beneficial for readability to keep // Instead of
if (json case [int x, int y]) {
// ...
}
// Do
if (final [int x, int y] = json) {
// ...
}
// Or even
if final [int x, int y] = json {
...
}
// This naturally leads to
final [int x, int y] = json else {
// ...
} Such syntax is easy to remember, it's fairly regular. It matches normal destructuring. |
The distinction between The distinction matters because refutable pattern matching must be used in a branch condition of some kind, because the bindings only work in code guarded by the match. The allowed patterns are also different between the two. final (0, int x?) = e else break; but now the patterns allowed in the "declaration" depends on whether there is a trailing The two kinds of patterns are different enough that it's important to keep them clearly separated. I'd probably be fine with About the sequencing, declarations are really the problem. Doing For |
@mraleph The penultimate proposal essentially admitted and used that syntax. But for various reasons, we decided to split the syntax of patterns, so that binding patterns (irrefutable, used for destructuring declarations) and matching patterns (refutable, used for switch cases) have slightly different syntax. Basically, a bare identifier in the first means "bind a new variable with this name", whereas a bare identifier in the second means "refer to a constant with this name". Given this choice, the syntax you propose doesn't really work well, because you now have a refutable pattern in a declaration location. For this reason, as part of the change above, @munificent changed the |
Building on what @lrhn and @leafpetersen said, more concretely;
That syntax gets weird when you consider simple identifiers: const x = 1;
const y = 2;
{
final [x, y] = json else { ... }
...
} Here, are We tried very hard to come up with a unified grammar that uses the exact same syntax in refutable and irrefutable contexts, but it's hard to get past the fact that So instead (like Swift), the variable binding syntax is slightly different between irrefutable and refutable contexts. But that in turn means that a guard-let-like construct (which takes a refutable pattern) should look less like a variable declaration statement and more like a switch case.
I've considered something like that, but it gets kind of weird when it happens to appear as a statement inside a switch case body: switch (foo) {
case bar:
case [int x, int y] = json else {
throw "";
}
} It might not actually be ambiguous, but it's pretty hard to read, I think. |
Thanks for the responses @leafpetersen @lrhn @munificent. I am pretty confident that you likely spent hours discussing this before, so I am somewhat afraid to repeat arguments that we already raised... That being said. I assign high value to syntax consistency because it makes things easier to remember and understand. It feel counter-intuitive to me that we will have two syntactically different ways to do almost exactly the same thing, with some additional inconsistencies on top. final [x, y] = expr; // ok, x & y are variables
if (expr case [x, y]) // not okay, unless we have constants x&y in scope
if (expr case [var x, var y]) // ok, x&y are variables. This inconsistency feels strange. This is probably too late but I think maybe just thinking about all patterns as refutable patterns in the uniform way was a possibility. match [var x, var y] = expr; // okay if pattern is guaranteed to always match
match [var x, var y] = expr else { // okay if pattern is not guaranteed to always match
throw ''; // if pattern is irrefutable then warning due to unreachable else
}
if (match [var x, var y] = expr) {
// warning if pattern always matches
}
switch (expr) {
match [var x, var y]: ... // okay
match ...: // error if one of the patterns above always matches.
} Is my understanding correct that the current proposal introduces the difference between bindings in patterns purely to save on typing |
Yes, that's right. We considered it, but it's really hard to get excited about: var (var x, var y) = (1, 2); When most other languages let you simply write:
We could introduce a new keyword for both destructuring variable declarations and pattern-based cases like you have with switch (expr) {
match [var x, var y]:
match [var a, var b] = list;
} Here, the first |
I agree that Let's take a look at languages that have both pattern matching and destructuring declarations (JS and Kotlin don't). Rust
const foo:i32 = 1;
const bar:i32 = 2;
let (x, y) = (1, 2);
match (1, 2) {
(x, foo) => ..., // foo refers to constant foo above
(x, bar) => ..., // bar refers to constant bar above
(x, y) => ...
}
if let (x, foo) = (1, 2) {
// ...
}
let x = if let (x, foo) = (1, 2) { x } else { return }; Swift
// can't write let (let x, let y) = ...
let (x, y) = (1, 2)
let bar = 1
let foo = 2
switch (1, 2) {
// Bind x and y to the elements of point.
case (x, bar):
print("[1] The point is at (\(x), \(bar)).")
// case let (x, foo) declares variable foo
case (let x, foo):
print("[2] The point is at (\(x), \(foo)).")
case let (x, y): // same as (let x, let y)
print("[3] The point is at (\(x), \(y)).")
}
if case (let x, foo) = (1, 2) {
print("inside if");
}
// can't say guard (let x, bar) or guard let (let x, bar) = ...
guard case (let x, bar) = (1, 2) else {
exit(1)
} Scala
val (x,y) = (1,2)
(3, 2) match {
case (x, `y`) => ... // y refers to y above, x declare new binding
}
val (z, `y`) = (1, 3); // will throw in runtime AnalysisI think syntactically we are close to Swift with our inconsistency. That being said I honestly prefer simplicity and uniformity of Rust. It gives constants a special treatment - which we could also do, and then asks developers to use guard clauses for any comparisons which are not comparisons with constants. I do also like guarded bindings that Swift provides. So if I were to mix and transplant this to Dart, we would get the following: const foo = 1;
const bar = 2;
final (x, y) = (1, 2);
switch ((1, 2)) {
case (x, bar):
}
if var (x, bar) = (1, 2) {
}
var (x, bar) = (1, 2) else {
return;
} |
I really, really worry about introducing a new variable with no I'd be OK with allowing The argument against that is that users don't want options to choose between, they just want to know what to write. |
Note that this is what our current syntax for functions uses, and it doesn't seem to cause problems. |
Thanks for doing this analysis. I think it's fairly similar to what I wrote up here. As you note, the current design is based most heavily on Swift's approach. I like Rust's approach too:
But in practice, I think this works largely because constants are idiomatically capitalized in Rust. That's not the case in Dart. In Rust, it's pretty easy to tell what's going on here: if let (left, top, Width, Height) = rect {
// ...
} It matches a rectangle with a certain known size and extracts its position. (And, in fact, GitHub even syntax highlights the constants differently!) But with Dart's constant capitalization rules, I find this very opaque: if (rect case (left, top, width, height)) {
// ...
} Whereas with the current proposal, it's pretty clear: if (rect case (var left, var top, width, height)) {
// ...
} This is a big part of why I think a Swift-like approach is a better fit for Dart. |
I must admit it is pretty clear. What about this then: we prohibit naked references to constants and instead require relational pattern to be used. Your examples becomes: if (rect case (left, top, == width, == height)) {
// ...
} Here we ask users to type more for constants (likely uncommon case) rather than variables (common case). It also does not require any additional syntax. |
I have considered that too. It does address the ambiguity, but I think it makes the more common case (in switches) more verbose. Most switch cases today are switching on named constants. Even with destructuring, I expect it will be more common for switch cases to test values than bind variables. (I could be wrong about this. I know @leafpetersen thinks destructuring will end up more common in cases.) If I'm right, then it's somewhat punishing to force users to write One way to think about the current proposal is that it presupposes:
And to make common patterns be terse in both contexts, it chooses a different default for each. In irrefutable contexts where binding is the norm, an identifier binds. In refutable contexts where constants are common, an identifier tests a constant and binders get an extra marker. |
Is this true? I thought most existing switch cases used prefixed identifiers, which would always resolve to constants. |
Slava's proposal was:
Which I interpreted to mean that all constants in cases needed |
The current patterns specification has the advantage that an identifier expression generally means what the identifier would mean outside of a pattern. In all other cases, the identifier means what it means as an expression, we just have strong restrictions on which identifiers we allow, so that only constants are allowed in refutable patterns, only non-final variables are allowed in assignment patterns, and the I like this because it might allow us to extend the rules in the future, say to |
After I have spent more time talking to @lrhn and going through @munificent's arguments I am got kinda sold on the current syntax. I understand the idea behind the current choices and I concede that it comes together cohesively. I still have some lingering concerns around how |
For what it's worth, I have some of those lingering concerns too. I'm fairly confident that it will work out, but I wouldn't say I love it. I do think it's important to have some if-let like construct because in my testing, it comes up really frequently. |
Rust 1.65.0 just came out which adds support for let-else statements: let PATTERN: TYPE = EXPRESSION else {
DIVERGING_CODE;
}; As with Swift's guard-let, the |
We could have: unless (patternClause) {
// must escape
} and allow the variables captured in the pattern to flow into the following scope. if (patternClause) {
....
} else {
// may escape
} with less indentation. I think case pattern = value else { /* must escape */ } is somewhat reasonable. It's a little weird that the value comes after the pattern for a refutable pattern (and after a The most general solution would be to allow: (expression case patternClause) as a boolean expression which provides pattern variables along its {
if ((pair case (int x, int y) when x < y) && (computePair(x, y) case (int x2, int y2))) {
// All variables in scope
} else {
throw "WAT?";
}
// All variables still in scope.
}
// No variables in scope any more. Then it's just: if (!(json case [int x, int y])) throw "nope";
print(x + y); |
I personally think this is hard to read because it's not clear that |
I think supporting this pattern with less indentation is worthwhile. A while ago I wrote: if (element
case LibraryImportElement(:final DirectiveUriWithRelativeUri uri)
when uri.relativeUriString == expectedImport) {
// Annotation is on the correct import
} else {
throw InvalidCheckExtensions(
'must annotate an import of $expectedImport');
} I happened to not need I would prefer unless (element
case LibraryImportElement(:final DirectiveUriWithRelativeUri uri)
when uri.relativeUriString == expectedImport) {
throw InvalidCheckExtensions(
'must annotate an import of $expectedImport');
} |
Wouldn't this be resolved with the idea of negation patterns? // something like this
// variable assignment would not be allowed in negation patters
// (you cant assign a variable if you're explicitly checking that it *isnt* what want)
if (data case SomeClass(value: !ValueSubType())) {
// where SomeClass.value is of type ValueSuperType and ValueSubtype implements ValueSuperType
throw 'data.value needs to be a ValueSubType!';
} would this be a good example? |
I don't know what you mean by a "negation" pattern. What would this do: if ('string' case !(int i)) print(i / 2); |
I really like the // parameter
void foo(value) {
if (value is! double) return;
print(value.round());
}
// non-local variable
void foo() {
if (_value case! double x) return;
print(x.round());
} |
That's more cumbersome, and less general, than necessary. What if we allowed Then this is just: if (!(value case double x)) return;
// Use x |
It's feasible, but I worry that this scoping rule would be very subtle and confusing for users.
I have to say that the double parentheses don't fill me with joy. |
Alternatively (or complementarily) we could have unless (value case double x) return;
// Use x |
Requiring parenthesis might resolve the parsing ambiguity concerns that originally made us dismiss this idea. This idea was also brought up in #2181 (comment). I think it would satisfy the request in #3062 |
I think the expression grammar would be something like: <expression> ::=
<assignableExpression> <assignmentOperator> <expression>
| <conditionalExpression>
| <cascade>
| <throwExpression>
| <patternExpression> -- new!
<patternExpression> ::=
<expressionWithoutCascade> 'case' <guardedPatternWithoutCascade>
<guardedPatternWithoutCascade> ::= <pattern> ('when' <expressionWithoutCascade)? Maybe rename Then we'd have to figure out the scope of the variables bound by such a pattern. They're definitely only available on the |
That's one of the syntaxes I considered in the initial description for this issue. :) |
Sorry, it has been a year since I last read it! 😅 |
I'll repost this idea here, since #3506 was closed in favor of this thread. But given Dart already supports object deconstruction. final Foo(:bar) = Foo(1); and if-case statements ABC abc = A(1);
if(abc case A(:final a)){
// can now use "a"
} The logical extension of this is "object deconstruction or else statements" ABC abc = A(1);
A(:final a) = abc else {
// break, continue, return, or throw
}
// can now use "a" I believe this fits well into the current syntax. Currently you have to do this ABC abc = A(1);
int a;
if(abc case A(a:final innerA)){
a = innerA;
}
else {
return;
}
// use a While ideally we should be able to do this ABC abc = A(1);
A(:final a) = abc else {
return;
}
// use a Adding another keyword like |
While a cute idea, I am honestly having a hard time interpreting this syntax. You have to jump around while reading it out as: I like the idea with case! or unless more in terms of code readability, though I must admit your suggestion gives quite some flexibility as syntax sugar. |
That is the syntax that Rust uses https://doc.rust-lang.org/rust-by-example/flow_control/let_else.html , I find "deconstruct or else" the logical complement to "if can deconstruct" (if-case). Another benefit of this syntax is that if we decide to loosen the restriction that the branch has to end with a control flow, we could have default values. A defaultVal = A(2);
ABC abc = A(1);
A(:final a) = abc else defaultVal;
// use a, a is 2 if abc is not an A. Not saying defaults should be adopted but the form allows this type of syntax in the future. |
A similar "let-else" syntax was discussed earlier in this thread. The short version of @munificent's comment on that was in #2537 (comment):
and that links to the long version. I think the same points apply to this most recent version. (Like @Sominemo I find the |
I too agree Also |
The patterns proposal adds a new "if-case" statement form for matching a single refutable pattern:
That binds pattern variables inside the then branch when the pattern matches. Swift has something similar with "if-case" (which is where I got the idea). Swift also has a "guard-case" feature:
Which that, if the pattern does not match, then the guard body is executed. That body must exit the surrounding scope (by returning, throwing, breaking, etc.). Any variables bound by the pattern are now in scope in the rest of the block following the guard-case. This lets users flatten out a level of control-flow related nesting.
Before null safety, Dart had much simpler type promotion rules. It would promote inside the then branch of an if statement:
But it would not promote when the if performed a negative test and the then branch exited:
This was a frequent source of frustration because when you had multiple promotions, you were forced to add a level of nesting for each one. I suspect that if we only have if-case and not a corresponding guard-like form, we will create something equally frustrating.
Can we come up with one? If so, what should it look like? I've tried a few times, but never found a syntax I liked. Two ideas:
So using the contextual keyword
unless
instead of Swift'sguard
. Or:Like an if-case-else except there is no then branch at all.
The text was updated successfully, but these errors were encountered: