Hi, On Tue, Feb 9, 2021 at 7:33 PM Larry Garfield <la...@garfieldtech.com> wrote:
> On Mon, Feb 8, 2021, at 6:48 PM, Levi Morrison via internals wrote: > > On Mon, Feb 8, 2021 at 5:15 PM tyson andre <tysonandre...@hotmail.com> > wrote: > > > > > > Hi Larry Garfield, > > > > > > > > Hi Larry Garfield, > > > > > > > > > > > > Hi internals, > > > > > > > > > > > > > > Voting has started on > https://wiki.php.net/rfc/any_all_on_iterable and > > > > > > > ends on 2021-02-22. > > > > > > > > > > > > > > This RFC proposes to add the functions > `PHP\iterable\any(iterable > > > > > > > $input, ?callable $callback = null): bool` and > `PHP\iterable\all(...)` > > > > > > > to PHP's standard library's function set, using the namespace > preferred > > > > > > > in the previous straw poll. > > > > > > > > > > > > > > [...] <https://www.php.net/unsub.php> > > > > > > > > > > > > > > > > > > Ak! I literally just finished reading it and wanted to note a > lack of clarity on one point. :-) > > > > > > > > > > > > The signature of the callback is never specified explicitly. > The ternary is a bit confusing. I assume the signature is > > > > > > > > > > > > callable(mixed): bool > > > > > > > > > > > > But that's not made explicit. It's also not made explict that > omitting the callable collapses to "is truthy". That's a sensible thing to > do, but it's not stated explicitly anywhere, just inferred from the code > sample. > > > > > > > > > > > > [...] > > > > > > > > > > If there is a callable, it allows `callable(mixed): mixed`, > > > > > and converts the callable's return value to a boolean. > > > > > So omitting the callable is the same as passing in the callable > `fn($x) > > > > > => $x`, which is equivalent to `fn($x) => (bool)$x`. > > > > > This is exactly what the reference implementation would do. > > > > > > > > > > [...] > > > > > > > > Oof. I'm glad I asked, because I don't like that at all. If > available, the callable should be returning bool, not "anything that may be > truthy/falsy." If you have an explicit function, it should have an > explicit return type. A truthy check is a reasonable default, but not for > when you're opting in to specifying the logic. > > > > > > > > I am in favor of the RFC, but I will have to consider if that > changes my vote to No. > > > > > > > > --Larry Garfield > > > > > > This was a deliberate choice and is consistent with the weak type > comparison behavior of array_filter() and other functions that default to > using weak type checks internally. > > > > > > I'd agree that I'd prefer to see callbacks returning booleans in code > I'm reviewing, > > > but a truthiness check seems more practical and consistent with the > rest of the language > > > than throwing a TypeError or checking the predicate return value using > `!== true` > > > > > > This was made to make PHP more widely accessible and free of surprises. > > > e.g. `(bool)array_filter($arr, $predicate)` can be safely converted to > `any($arr, $predicate)` without introducing a TypeError or behavior change. > > > > > > [...] > > > > For what it is worth, in C++ it is fairly normal to use a convertible > > to bool type. For instance, having an overload on a < b for iterators > > can return whatever type it wants, as long as it is contextually > > convertible to bool. > > Yet in 8.0, a non-[ 1 | 0 | -1 ] return from a comparison function as just > converted to a warning. So the trend in the language seems to be the other > direction. > Are you referring to this frequent mistake? ``` $list = [2,4,1,3]; usort($list, fn ($a, $b) => $a < $b); /* Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero */ echo json_encode($list); // [4,3,2,1] ``` (That's probably because PHP user comparison functions were modelled from C (e.g. strcmp()), not C++ (e.g. std::less).) Of course here the "correct" thing is to return `$a <=> $b` (or `$b <=> $a` for descending order), but you can also return `$a - $b` (not necessarily in [-1,0,1]), or even a string `"foo"` still without any warning in 8.0.2 (just a certainly wrong result)... Anyway, to me it feels natural that any()/all() would "work" like array_filter(). @Tyson by the way, in the any()/all() case (vs the any_value()/all_values() and potential any_key()/all_keys() etc.), wouldn't it be preferable to add the optional `int $flags = 0` (or "$mode") parameter right from the start (even if not used yet), as adding it in a later release would apparently pose some BC concerns (ArgumentCountError, polyfills etc.)? -- Guilliam Xavier
