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

Why does pattern matching have such strict restrictions on non-constant values? #3064

Open
nex3 opened this issue May 11, 2023 · 6 comments
Open
Labels
request Requests to resolve a particular developer problem

Comments

@nex3
Copy link
Member

nex3 commented May 11, 2023

Both guard clauses and constant clauses in patterns require the use of contants where, semantically, it seems like any expression value could be used with a clear meaning. Similarly, a guard clause can only be used at the end of a case expression when it seems like the semantics would be fairly clear if it were allowed after any pattern as a way to "escape" into some more complex boolean logic. I'm guessing there's a good reason for this design, but it's not really explained in the docs so it's hard to integrate it with my mental model. Consider this a hybrid feature request / request for more documentation.

As a motivating example, I'm trying to convert this conditional code to a pattern-matching style:

var left = this.left;
var leftNeedsParens = (left is BinaryOperationExpression &&
        left.operator.precedence < operator.precedence) ||
    (left is ListExpression &&
        !left.hasBrackets &&
        left.contents.length > 1);

The tricky bit is left.operator.precedence < operator.precedence. I'd like to be able to write something like

case BinaryOperationExpression(
  operator: BinaryOperator(precedence: < operator.precedence)
) ||
ListExpression(hasBrackets: false, contents: [_, _, ...]) =>
  true

but instead I think my best option is something like

BinaryOperationExpression(operator: BinaryOperator(precedence: var p))
    when p < operator.precedence =>
  true,
ListExpression(hasBrackets: false, contents: [_, _, ...]) =>
  true

This works well enough in my case where the pattern isn't too deeply nested and both the right-hand sides are true, but it would get very nasty otherwise. And it feels like I'm essentially doing a manual transformation that the compiler could do for me. Why not just allow me to write precedence: < operator.precedence and interpret that as a when clause at the end? Or even allow the pattern to eagerly fail to match if it's not evaluated?

@nex3 nex3 added the request Requests to resolve a particular developer problem label May 11, 2023
@munificent
Copy link
Member

Guards can refer to non-constant values (which is one of the things that makes them useful over constant patterns), but you're right that we don't allow them to be nested inside patterns.

In many cases, that's not much of a restriction because you can use variable binding to capture intermediate values and then refer to those variables in the guard, like:

var leftNeedsParens = switch (left) {
  BinaryOperationExpression(operator: var leftOperator)
      when leftOperator.precedence < operator.precedence => true,
  ListExpression(hasBrackets: false, :var contents)
      when contents.length > 1 => true,
  _ => false,
}

This works particularly well in switch statements because you can have multiple cases share a body and each case can have its own guard:

var leftNeedsParens = false;
switch (left) {
  case BinaryOperationExpression(operator: var leftOperator)
      when leftOperator.precedence < operator.precedence:
  ListExpression(hasBrackets: false, :var contents)
      when contents.length > 1:
    leftNeedsParens = true;
}

Switch expressions, unfortunately, require each pattern to have its own body. You can effectively have more patterns share a body using an || pattern:

var leftNeedsParens = switch (left) {
  BinaryOperationExpression() ||
  ListExpression() => true,
  _ => false,
}

But then you lose the ability to have a separate guard for each subpattern.

There's no principled reason behind this. As far as letting guards be deeply nested inside expressions, I'm not aware of other languages that do this and I don't know if I even considered it. In general, I think it would probably lead to pretty hard to read code and it's usually cleaner to just bind a variable from a subpattern and then use that in the guard.

For having multiple cases share a body in switch expressions (which would then let each case have its own guard), I could just never come up with a syntax that I felt hung together. The current syntax is annoyingly limited when you try to use it for very complex cases. But in return for that, it's quite compact in simple cases, which is what I felt made sense to optimize for in an expression form.

Retrofitting complex pattern matching in a language designed to look like Java was really hard and we couldn't always make things fit in seamlessly like we wanted.

@nex3
Copy link
Member Author

nex3 commented May 12, 2023

That all makes sense, but I'm still left wondering: why not let patterns refer to non-constant values, especially since there's a straightforward transform from that to the current form?

@munificent
Copy link
Member

It might be doable, but it's possible that it would interact poorly with exhaustiveness checking. I'll have to think about it more.

@lrhn
Copy link
Member

lrhn commented Jun 15, 2023

The way I think about patterns is not so much as general value checking, but mainly as destructuring for statically known structures.

With that view, it makes sense that the structure has to be known. Checking specific values against constants is itself extra, but it's something that interacts well with exhaustiveness, especially for enums.

It is annoying that you can't do

extension <K,V> on Map<K,V> {
  V get(K key) {
    if (this case {key: var v}) return v;
    throw Badnes();
  }
}

to get the benefit of lookup plus containsKey check in one syntax, but any non-constant/final key expression would not be cacheable, and would make exhaustion harder. Probably not impossible, we'd just have to make sure the behavior is predictable.

If the rewrite is to capture the value and check it in a guard, which turned exhaustiveness off completely for that case, then the exhaustiveness would not be worse by allowing the check inline.
But it might not be as visible that it doesn't count towards exhaustiveness.

@nex3
Copy link
Member Author

nex3 commented Jun 15, 2023

The way I think about patterns is not so much as general value checking, but mainly as destructuring for statically known structures.

I think it's relatively likely that users will want to use it for general value checking regardless, because it's generally terser, often clearer, and provides access to internal variable bindings. You've created a good feature, and people want to use it 😃.

@rrousselGit
Copy link

rrousselGit commented Jun 21, 2023

One issue with not allowing non-constant values is, sometimes objects expose static utilities similar to a MyObject.empty but that is not const.

For example today I was using VersionConstraint from pub_semver, and wanted to do:

switch (Dependency()) {
  case HostedDependency(version: VersionConstraint.any):
    print('doSomething');
  case HostedDependency(version: final version):
    /* handle non-any versions */
}

This looks like it should work. But VersionContraint.any is not a constant, and therefore it doesn't work.

This forces us to use when when it would be totally natural to use a pattern check here.
It causes the syntax to be a bit inconsistent.


It also decreases readability quite a bit.

In a similar fashion, I wanted a pattern match to compare the "hosted" of two HoestedDependency. The class doesn't implement ==, so I have to do the equal check on my own.

I would've wanted to write:

HostedDependency constraint;
Dependency dependency;
switch (dependency) {
  case HostedDependency(
    hosted: constraint.hosted || HostedDetails(
      declaredName: constraint.hosted?.name,
      url: constraint.hosted?.url,
    ),
  ): 
}

But I had to write:

HostedDependency constraint;
Dependency dependency;
switch (dependency) {
    case HostedDependency()
        when constraint.hosted?.declaredName ==
                dependency.hosted?.declaredName &&
            constraint.hosted?.url == dependency.hosted?.url:
}

The way the "when" condition formats is way worse. At a glance, it's much harder to find where the "case" starts and to visually see all the compared properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Requests to resolve a particular developer problem
Projects
None yet
Development

No branches or pull requests

4 participants