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

Allow extractor patterns to use full selector chains. #2433

Open
lrhn opened this issue Aug 24, 2022 · 9 comments
Open

Allow extractor patterns to use full selector chains. #2433

lrhn opened this issue Aug 24, 2022 · 9 comments
Labels
feature Proposed language feature that solves one or more problems patterns Issues related to pattern matching.

Comments

@lrhn
Copy link
Member

lrhn commented Aug 24, 2022

Proposal

I propos that the "name" in an extractor pattern is extended to allow any non-assignment cascade selector chain where arguments must be constant values.

That is, it starts with a cascade selector (either an identifier or a [constant] index operator), and can be followed by a sequence of the selectors:

  • !
  • [constant]
  • ?[constant]
  • .id
  • ?.id
  • (constantArgs)
  • <constantTypeArgs>(constantArgs)

This provides convenience when accessing other parts of an object than just those accessible using getters, but not anything you can't already do by declaring an extension getter for the operation (since the arguments must be constant, so they can be hardcoded into the getter too.)

Motivation

Currently an extractor pattern can only syntactically access getters:

  case Foo(foo: p1, bar: p2, :var baz): ....

The foo, bar and (implict) baz names must be naming getters of the Foo type.

You can access nested chains of getters, like foo.foo2.foo3 as:

 case Foo(foo: Foo2(foo2: Foo3(foo3: var x))): ...

If we allow you to write getter chains before the :, we won't be giving any new functionality, just more convenience;

case Foo(foo.foo2.foo3: var x): ...

We can allow an ? modifier and ! selector as well, getting:

case Foo(foo?.foo2!.foo3: var x): ...

(The ! throws if foo.foo2 is null.)

Further, we usually treat [] as a (parameterized) getter and []= as a setter. Notice( how []= doesn't need a return type and defaults to void, like a setter.)
We should allow [constantExpression] as a selector in the extractor pattern too:

case List(length: >= 2, [0]: p0, [1]: p1): ...

would then be equivalent to:

case [p0, p1, ...]: ....

but it can be used for other situations too.

The arguments to operator[] must be constant, so that we can properly recognize when multiple lookups in the same switch case are reading the same value.

I said that above that the extractor pattern can access things that are syntactically getters.
That means that you can access other things too, if you define an extension getter which does the work for you.

extension on Object? {
   get asString => toString();
}

...
  case Foo(asString: "arglebargle"): ...

That means that we don't actually provide any extra power by allowing you to call nullary functions, or any functions with known constant arguments, in an extractor pattern, and we might as well allow it:

  case int(toRadixString(16): "deadbeef"): ... // silly example, I know.

The user could just define extension on int { String get hexString => toRadixString(16); } and do case int(hexString: "deadbeef"): ... instead.

@lrhn lrhn added feature Proposed language feature that solves one or more problems patterns Issues related to pattern matching. labels Aug 24, 2022
@eernstg
Copy link
Member

eernstg commented Aug 24, 2022

I really like the idea that the mechanism that used to denote a getter call could just as well denote a chain of invocations. Why not?!

But this brings up another issue that I've mentioned before: Caching of returned results. In short, we'd need to handle every prefix of these getter/method invocation chains and cache the result.

But that won't even suffice: If we wish to ensure that each getter of a scrutinee is called at most once during matching in one switch statement/expression then we need to keep track of which getters have already been called. This is not so easy in the case where we're calling an extension instance getter that may in turn call whatever it wants, as often as it wants:

class Foo {
  static int _counter = 0;
  int get g => ++_counter;
}

extension E on Foo {
  int get gg {
    print(g);
    return g;
  }
}

void f(Foo foo) {
  switch (foo) {
    case Foo(gg: 3): print('Case gg'); break; // Skipped.
    case Foo(g: 3): print('Case g'); break; // Reached, so we called `g` three times!
  }

  // This could be desugared as follows:
  late final foo_gg = foo.gg;
  late final foo_g = foo.g;
  if (foo_gg == 3) {
    print('Case gg');
  } else if (foo_g == 3) {
    print('Case g');
  }
}

void main() => f(Foo());

(I used an explicit break just so we don't have to think about the details of implicit breaks, because that's a different topic.)

The point is that the motto "we only call your getters at most once in a matching process, and henceforth use the cached value" has tons of loopholes.

We could inline extension methods, but that doesn't actually help that much, because we could just have some regular instance getters that are calling other instance getters.

I do think that it's more and more obvious that practical, professional software written using pattern matching must avoid unstable getters (chains or not, that doesn't matter), and it should ensure that side effects are unobservable or at least idempotent. We may or may not wish to help developers by making stable getters a language mechanism.

But regardless of whether it's a language construct or a theoretical notion, unstable getters and side effects during pattern matching must be taken into account, because nobody is going to be happy about maintaining code where the value of a getter is dancing around like in the example above.

@lrhn
Copy link
Member Author

lrhn commented Aug 24, 2022

We do not promise that each getter/constant-selector is only called once, only that it's only called once directly by the pattern matching code. If one getter forwards to another, then the pattern matching doesn't know. We'll only call each of these once from the pattern matching code, but what the bodies do is completely free (the one time it's called).
And we don't even need to promise it, just allow it.

Basically, we allow the pattern matching to assume that getters are stable. If they aren't, well, shucks. You probably shouldn't have tried pattern matching against them then.
By assuming (and internally caching to enforce) stability, we have a chance of achieving sound exhaustiveness. Otherwise, if Foo(x: true) and Foo(x: false) is not actually exhaustive (where Foo.x has type bool and we are in a null-safe program), then exhaustiveness is basically impossible except for identity checks at the top-level (the current switch on enums).
So, to help people get sound exhaustiveness on things that actually look exhaustive, we need to assume (and simulate) stability.

If we had actual stable getters, using those would be sound even if we read the getter more than once.
I don't think we'll have that by the time we ship patterns (unless we decide to).

@munificent
Copy link
Member

I'm definitely interested in extending extractor patterns to invoke other members and potentially chains of members. However, this also takes up pattern grammar space and I want to be sure that we'd using that judiciously. I think the best approach is to keep this on the table but defer it until after the initial release of patterns. That way, we can see how users are using extractor patterns in practice and get a better feel for how useful this syntax is or if there is something more valuable we might want to use it for.

@lrhn
Copy link
Member Author

lrhn commented Apr 27, 2023

One case already came up: A JSON destructuring where some keys are allowed to be absent.

The pre-patterns code just read ['propertyName'] manually and allowed it to be null.

A pattern rewrite cannot use a map pattern (they reject if the key isn't there) and it cannot use an object pattern with Map type because it cannot use the ['propertyName'] "getter".

(We generally treat [] as a getter and []= as the corresponding setter, which is why o[i] += 2 is allowed.)

There is no way to say optionally match this map entry, if it's there, but don't reject if it's not.

We could say:

  if (map case {"foo": var foo, ?"bar": var bar}) ...

would match var bar against null if bar is not a key of the map.

Or we could allow:

 if (map case Map<String, dynamic>(["foo"]: var foo!, ["bar"]: var bar)) ...

(which could also be useful for other types than maps).

So, even if we don't allow selector chains, but only allow the [constant] selector as an object pattern "getter", we'd support that use-case.

You can always chain using nested object patterns, it's allowing other single selectors than just getters that open up opportunities.
(And avoids people doing crazy stuff like:

extension on Map<String,dynamic> {
  List<Object?> get _jsonPropertyName => this['propertyName'];
}
....
  if (map case {"id": var id) && Map(_jsonPropertyName: var property)) ...

to get everything into one pattern patch. Because I'll totally do that!)

@TimWhiting
Copy link

TimWhiting commented May 10, 2023

I've already used extension methods to work around many limitations in patterns such as this. I think this is definitely worth taking up the pattern grammar space, from my experience so far. Already the solution with extension getters works great and I'm super glad that extension getters work in patterns. However having to create a extension namespace or locate your existing extension destructuring definitions every time you need to test a different deeply nested property is terrible user experience compared to this proposal.

@rrousselGit
Copy link

Could we have some sugar regardings bools too?
It feels off to me to have to do case MyClass(property: true).

That's like writing if (obj.property == true) instead of if (obj.property)

@TimWhiting
Copy link

To invert the boolean condition it could be useful to allow a ! before the name, in addition to after the name for non-null assertions.

@rrousselGit
Copy link

rrousselGit commented May 10, 2023

Maybe we could reuse ?/!, but place them before the variable name:

case Class(?property) // equivalent of Class(property: true)
case Class(!property) // Class(property: false)

edit: @TimWhiting posted at the same time 🙈

@lrhn
Copy link
Member Author

lrhn commented May 10, 2023

I like the idea of short boolean patterns (because it does feel a little weird to write isFoo: true), but I'm a little wary about spending the leading ? on something like this. I think there could be more significant uses for such a prime syntactic real-estate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems patterns Issues related to pattern matching.
Projects
None yet
Development

No branches or pull requests

5 participants