Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:109654
MIME-Version: 1.0
References: <CAPyj-LDfdATXFD+dc5AY9EqWWxgf8LP1joD2fzvm==1gUoN35A@mail.gmail.com>
In-Reply-To: <CAPyj-LDfdATXFD+dc5AY9EqWWxgf8LP1joD2fzvm==1gUoN35A@mail.gmail.com>
Date: Wed, 15 Apr 2020 15:25:50 +0200
Message-ID: <CAF+90c9poyn5trxeCES57WTFbi9joqsJ7co4mFEYB3us75nfYw@mail.gmail.com>
To: Ilija Tovilo <tovilo.ilija@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000354d9105a3544242"
Subject: Re: [PHP-DEV] [DISCUSSION] Match expression
From: nikita.ppv@gmail.com (Nikita Popov)

--000000000000354d9105a3544242
Content-Type: text/plain; charset="UTF-8"

On Sun, Apr 12, 2020 at 2:16 AM Ilija Tovilo <tovilo.ilija@gmail.com> wrote:

> Hi internals
>
> I'd like to announce the match expression RFC that replaces the switch
> expression RFC I announced a few weeks ago.
> New: https://wiki.php.net/rfc/match_expression
> Old: https://wiki.php.net/rfc/switch_expression
>
> Due to the feedback I decided to take the RFC in a slightly different
> direction. The main objections were:
>
> 1. It didn't behave like the traditional switch so it shouldn't use
> the switch keyword
> 2. It didn't fix type coercion
> 3. The RFC was poorly structured
>
> In hindsight I understand why I had a hard time writing the RFC. I
> tried making a case against the switch statement while really not
> addressing the switch statement at all. The new RFC proposes a match
> expression that fixes all the objections raised against the switch
> statement. Additionally, match arms can now contain statement lists
> which allows you to use the match expression anywhere the switch
> statement would have been used previously.
>
> While some people have suggested statement lists aren't necessary I
> don't think it makes sense to raise objections against the switch
> statement without offering an alternative.
>
> I also experimented with pattern matching but decided against it. The
> exact reason is described in the RFC.
>
> I'm now confident this is the right approach. I hope you will be
> happier with this proposal.
>
> Happy Easter!
>

Thanks for the proposal, I like the direction.

Regarding the block syntax: I do think that block support is pretty
important, but the way it is introduced in the RFC is not very clear to me.
I think part of the problem is that it's a case of either / or. If you have
a match that returns a value, but one of the match cases becomes too
complicated and would benefit from a temporary variable, you cannot
introduce it easily. You have to go all the way from

$x = match ($y) {
    0 => foo(),
    1 => bar(),
    2 => baz(),
    3 => oof(),
};

to

match ($y) {
    0 => {
        $x = foo();
    },
    1 => {
        $x = bar();
    },
    2 => {
        $x = baz();
    },
    3 => {
        $tmp = foobar(); // <-- The only new part!
        $x = oof($tmp);
    }
}

While what you really wanted is the Rust-style:

$x = match ($y) {
    0 => foo(),
    1 => bar(),
    2 => baz(),
    3 => {
        $tmp = foobar();
        oof($tmp)
    },
};

We already have a similar issue with arrow functions, where adding an extra
line requires conversion to a normal closure. This is already not great,
but it's a very contained change. Here you may have to refactor a large
match expression to incorporate a local change.

I'm not quite sure what the way forward regarding that is. I can understand
the reluctance to propose the Rust-style block expression syntax, given its
currently fairly limited usefulness outside match expressions. I do think
that this is principally the right approach though, if we want to introduce
control flow expressions like match. The alternative is to leave match as a
statement only. (If we do not support both expressions *and* statements,
then I believe we should only be supporting statements, not the other way
around.)


Regarding pattern matching: I agree that we shouldn't tackle this topic as
part of this proposal, but we do need to already have a pretty firm idea of
how that is going to look like to avoid forward-compatibility hazards. It
is very easy to run into pattern vs expression ambiguities.

The section of your RFC discussing pattern matching tries to avoids most of
the problem by prefixing patterns with "let". However, even under that
premise there are going to be issues. For example, you have this example:

match ($value) {
    let [1, 2, $c] => ..., // Array pattern
}

The intention here is clearly that $c is supposed to capture the third
array element. But if we stay consistent with the remaining proposal, then
$c should be interpreted as a value to match against here, just like 1 and
2. The way to actually capture $c should be

match ($value) {
    let [1, 2, let $c] => ...
}

I believe, which is sub-optimal. If we allow arbitrary expressions as match
values, then every captured variable would have to be prefixed by let
individually to make things work (I think, correct me if I'm missing
something here).

Another consideration here is that "let" carries a very strong implication
of block scoping. Block scoping is something we might well want to have,
but I'm not sure we would want it to be bound to pattern matching matching
syntax.

Finally, the use of "," to specify multiple match values could be a
composition problem. Rust uses | to specify multiple match values, and also
allows its use in sub-patterns (this is the "or patterns" feature). This
allows you to write patterns like

Some(Enum::A | Enum::B | Enum::C | Enum::D) => ...

which are equivalent to

Some(Enum::A) | Some(Enum::B) | Some(Enum::C) | Some(Enum::D) => ..

Picking "," as separator makes that impossible, as it would cause
ambiguities with commas in array patterns, if nothing else. On the other
hand, we cannot use "|" if we want to allow arbitrary expressions for match
values. (Do we really want that?)

Regards,
Nikita

--000000000000354d9105a3544242--