Hi Bob

> I'd like to note that the class-access is very ugly.
> ...
> But what's not fine is using an inconsistent syntax for variable bindings 
> across different contexts. In arrays binding is just a bare variable. In 
> objects it suddenly needs a colon? What.
> ...
> This also works for future ADTs. Move::Forward&{ $amount }. Then, if there's 
> a desire to actually *positionally* match an object. Then it's logical to use 
> a parenthesized expression, for a tuple. I.e.:

We'd argue that using `()` and `{}` to differentiate between positional and 
named parameters is no more intuitive than a `:` prefix. To look at preexisting 
concepts in the language: Named vs. positional arguments aren't determined by 
the type of braces, but by whether they are preceded by the argument name. 
`(:$param)` can be viewed as an extension of `(param: $param)` where the 
redundant argument name is dropped, while `{ $param }` looks like something 
completely different.  (Technnically, adding this shorthand to named argument 
calls would be possible, though we are not proposing it here.)

We also recently discovered that this is exactly the approach Dart takes:
https://dart.dev/language/patterns#destructuring-class-instances

So there is prior art.  Also, this is only a question for the shorthand syntax. 
 If using the full version, there is no room for confusion.

Additionally, as noted in the RFC, we ran into issues for `{}` both when using 
and omitting `&`.  Without a `&`, `{}` is confused for a hook when in the 
default property value position.  With a `&`, runtime disambiguation is 
required for ADTs and a class constants, which is inconsistent with the rest of 
the language.  (Unless you can suggest a way to resolve that ambiguity, in 
which case we can consider it.)  More on this topic below.

> Positional binding is quite intuitively using parenthesis - you construct the 
> enum with Foo::Bar($var) and you read it back on the right hand side with 
> Foo::Bar($var).

Yes, this is precisely the syntax we had in mind for ADTs. So we're in 
agreement here.

> It naturally allows destructuring without class name.

This is an upside of the split syntax, yes. However, as noted, that led to 
parsing issues, which is why we moved away from it.

While a bit less visually pleasing, it would be possible to allow `_` or `*`, 
or even `object` as a pseudo-class name to indicate "any class."  That would be 
a much simpler solution to the "I don't care about the object type" question.  
Eg: `$p is _(x: 5)`.

> I've also heard a consideration about "Foo::Bar & { $var }" being ambiguous 
> with respect to "is Foo::Bar now a const or an ADT class". This may be 
> resolved in the VM. I don't consider this a major issue, and is simply 
> something which can be disambiguated at optimizer-time or run-time, depending 
> on what type of symbol it is.

PHP is very explicit when it comes to distinguishing member types 
syntactically. For example, in many languages, `foo.bar` could access a field, 
static field, constant, function reference, static function reference, subtype, 
etc. PHP doesn't do that, it uses `$foo->bar`, `foo::$bar`, `foo::bar`, 
`$foo->bar(...)`, `foo::bar(...)`, `foo\bar`, etc. It goes out of its way to 
make it obvious what kind of member is being accessed. This makes a lot of 
sense for PHP, because each PHP file is compiled in a fully isolated context 
where we frequently don't know what the class `Foo` looks like. This avoids a 
lot of guesswork for the engine.

There are very few exceptions to this rule, one being `Foo::bar()`, which is 
normally a static call but _can_ also be an instance call when `Foo` is an 
ancestor of the class of the current instance. This has already caused some 
issues in the past. We tried to automatically make closures static that don't 
use `$this`, which is unsound because of these hidden instance call. We feel 
it's wise to avoid adding more such cases.

An alternative would be to disallow matching against an ADT's case name 
entirely, allowing `$p is Point & {$x}` but not `$x is Option::Some & {$val}`.  
The latter would have to be positional only, `$x is Option::Some($y)`.  That 
seems like a rather arbitrary and unexpected restriction, however.

> I'm deeply unsatisfied by the handling of object properties:
>
> "Note that matching against a property's value implies reading that 
> property's value", "If the property is uninitialized, an error will be 
> thrown." and "If the property is undefined and none of the above apply, it 
> will evaluate to null and a Warning will be issued."
>
> This is wildly inconsistent with arrays:

The rationale for treating objects and arrays differently is that objects are 
almost always structured (meaning we know and should be able to rely on which 
properties it defines), while arrays are frequently not. The obvious exception 
you already mentioned is `stdClass`, although how useful or common that is in 
practice at this point is debatable.  (I cannot recall the last time I saw 
`json_decode()` called without the flag to use an associative array instead.)  
`stdClass` should not be fundamental in shaping how this pattern works. You can 
also efficiently cast `stdClass` to `array` (because that's how `stdClass` is 
implemented anyway) to use the array pattern.

That said, this isn't a hill either of us wants to die on, so if the consensus 
is to swallow such cases and just return false, we will go with that.  
(Meaning, other people, please weigh in here.)

> It also means that uninitialized properties forcibly throw.

With fairly few exceptions, an uninitialized property on an object that is 
passed back to a caller is a sign of a design flaw.  Even if the construction 
process is multi-step -- as it is in Crell/AttributeUtils or deserializers, 
factories, ORMs, etc. -- everything is initialized by the time the object is 
returned to the caller.  Leaving properties uninitialized, or unsetting them 
manually implicitly adds an undocumented "undefined" type to your type union. 
There's absolutely no indication that accessing a property may be unsafe, but 
it is. There are niche cases where unsetting properties is useful (e.g. 
breaking cycles, as you've mentioned privately), but such objects shouldn't 
escape to users with a half-initialized state.  So there's a 99% chance that if 
the developer pattern matches against an uninitialized property, it's a 
developer error and should be corrected, not silently suppressed.

The other issue is that `$p is Point(:$x)` looks like an infallible pattern 
(assuming `$p` is an instance of `Point`) where we just want to extract `$x`, 
but if `$point->x` is uninitialized this pattern would fail. Given this is 
almost certainly a mistake, it seems better to inform the user about it rather 
than silently returning `false`.

> E.g. (assuming something like "class ResponseOrError { string $type; 
> Exception $e; string $response; }"):

This is a classic example of when ADTs would be useful.

> Enforcing positional arrays however will be quite surprising if e.g. an entry 
> was removed:
> $a = [1, 2, 3];
> unset($a[1]);
> if ($a is [1, 3]) {
>     // huh? It's [1, 2 => 3], not [1, 3].
> }

I think you misunderstood the question here. `[1, 2]` and `[0 => 1, 1 => 2]` 
should always be equivalent in terms of key behavior. What we weren't in 
agreement about is whether `[1, 2]` should have additional guarantees about 
order, i.e. about whether `[1, 2]` need to appear in that order, or whether `[1 
=> 2, 0 => 1]` is also acceptable. Normally, values are accessed by key which 
makes order mostly irrelevant, but there are cases where the user might expect 
to see values in a specific order when iterating over the array. Implementing 
this order-consistency check in a performant way is also somewhat tricky, and 
it would always add some overhead no matter what.

--Larry Garfield

Reply via email to