Re: [RFC] Pattern Matching

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