Re: [VOTE] PHP\iterable\any() and all() on iterables

Hi Guilliam Xavier, > > > 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. > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > > > > > 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)... A callback function called **once** (before php 8.0) on a pair mapping 2 states (true, false) onto 3 states (-1, 0, 1) only worked coincidentally because the sort wasn't stable. The desired type had more possible values than the returned type, so it did the wrong thing. - This is different in that there's only 2 desired values. https://wiki.php.net/rfc/stable_sorting fixed that calling it twice and deprecating booleans. It still allows you to return strings (not likely in practice) and casts those to integers. "Second, if boolean false is returned, PHP will automatically call the comparison function again with arguments swapped. This allows us to distinguish whether the “false” stood for “equal” or “smaller than”. This fallback behavior should be removed in a future version of PHP." > 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.)? If an iterables RFC passed, and all amendments got approved before 8.1 stable was released, it wouldn't matter. There's months before the feature freeze. If a function was added with flags "just in case" in 8.1 stable and we never used the flags, we'd have to deprecate them and remove them in a subsequent major release. I don't think that's worth it. Cheers, Tyson Andre