Iterable pseudo-type #1941

Merged
merged 13 commits into from Jul 4, 2016

10 participants

@trowski
trowski commented Jun 10, 2016 edited

It is common for a function to accept or return either an array or an object implementing Traversable to be used with foreach. However, because array is a primitive type and Traversable is an interface, there currently is no way to use a type declaration on a parameter or return type to indicate that the value is iterable.

This PR proposes a new iterable pseduo-type. This type is analogous to callable, accepting multiple types instead of one single type.

iterable accepts any array or object implementing Traversable. Both of these types are iterable using foreach and can be used with yield from within a generator.

iterable can be used as a parameter type to indicate that a function requires a set of values, but does not care about the form of the value set (array, Iterator, Generator, etc.) since it will be used with foreach. If a value is not an array or instance of Traversable, a TypeError will be thrown.

function foo(iterable $iterable) {
    foreach ($iterable as $value) {
        // ...
    }
}

iterable can also be used as a return type to indicate a function will return an iterable value. If the returned value is not an array or instance of Traversable, a TypeError will be thrown.

function bar(): iterable {
    return [1, 2, 3];
}

Parameters declared as iterable may use null or an array as a default value.

function foo(iterable $iterable = []) {
    // ...
}

Functions declaring iterable as a return type may also be generators.

function gen(): iterable {
    yield 1;
    yield 2;
    yield 3;
}

This proposal also adds a function is_iterable() that returns a boolean: true if a value is iterable and will be accepted by the iterable pseudo-type, false for other values.

var_dump(is_iterable([1, 2, 3])); // bool(true)
var_dump(is_iterable(new ArrayIterator([1, 2, 3]))); // bool(true)
var_dump(is_iterable((function () { yield 1; })())); // bool(true)
var_dump(is_iterable(1)); // bool(false)
var_dump(is_iterable(new stdClass())); // bool(false)

RFC: https://wiki.php.net/rfc/iterable

@nikic

This should do a string comparison -- otherwise might trigger autoloading.

trowski replied Jun 5, 2016 edited

Autoloading may be desired to get the class definition. We need to know if the class implements Traversable. Otherwise we'd be limited to just a few predefined types.

Hm, good point, I didn't think about subclasses. Problem is that we do not allow subclass co/contravariance elsewhere either, so not sure what to do here.

trowski replied Jun 5, 2016 edited

I pushed another commit (277d89c) limiting co/contravariance to array and Traversable only. Allowing only the two component types is probably the most sensible and least surprising option.

@nikic

This abort too early. The nullability and reference checks below are still relevant.

Thanks, fixed.

@mokeev1995

so... what about strings? are they iterable? if no, why?
@trowski @nikic

@codedokode

@mokeev1995 PHP cannot iterate strings by characters anyway (only by bytes which makes little sence with multibyte encodings). And you cannot pass a sting to foreach.

@Fleshgrinder

Strings cannot become iterables, the fact that you are able to iterate them byte by byte does not qualify them for this pseudo compound type. Especially because you cannot use them in a foreach and the iterable is all about that; you cannot use a Generator in a for. 😉

@mokeev1995

ok, thanks)

@smalyshev smalyshev added the RFC label Jun 14, 2016
@Majkl578

I'm afraid this would be a serious BC break in minor version. It would be perfectly fine for 7.0 (at least as part of reserved types RFC), but since it's already out, the only reasonable version to target is 8.0.
See GH results for:
class Iterable
interface iterable

@Danack

@Majkl578 From the first page of your first link, there are 10 entries. 8 have extra words (e.g. class IterableFilterIterator) and so wouldn't be affected. 1 of the others is in a namespace and so wouldn't be affected. Which leaves 1 example.

I'm afraid this would be a serious BC break in minor version.

Most people don't consider adding new classes to be a BC break. It's only changing behaviour of existing classes/functions, or removing stuff that is a 'real' BC break.

@Majkl578

Which leaves 1 example.

Which is still more than 0. And we are not even considering all the private code out there.

1 of the others is in a namespace and so wouldn't be affected

Not true, real-world applications typically import classes with use which implies breaking the usage of such classes/interfaces.

Most people don't consider adding new classes to be a BC break.

This proposal is not to add new class, but a pseudotype that would break all code using typehint for interface/class named Iterable. And that is a real BC break.

@trowski

@Majkl578 Behavioral changes are considered serious BC breaks, while this would only cause a BC break due to naming, which can easily be fixed with simple find-and-replace. Yes, it is technically a BC break, but one that can be fixed in a matter of minutes without actually examining any code.

@dstogov dstogov commented on the diff Jun 20, 2016
Zend/zend_inheritance.c
@@ -167,6 +167,20 @@ char *zend_visibility_string(uint32_t fn_flags) /* {{{ */
}
/* }}} */
+static zend_bool zend_iterable_type_check(zend_arg_info *arg_info) /* {{{ */
+{
+ if (arg_info->type_hint == IS_ITERABLE || arg_info->type_hint == IS_ARRAY) {
+ return 1;
+ }
+
+ if (arg_info->class_name && zend_string_equals_literal_ci(arg_info->class_name, "Traversable")) {
+ return 1;
+ }
@dstogov
dstogov added a note Jun 20, 2016

why do we need these special compatibility rules? why not just check for IS_ITERABLE?

@trowski
trowski added a note Jun 20, 2016

Allows a degree of co/contravariance in extending/implementing classes. Parameter types of array or Traversable can be broadened to iterable or return types declaring iterable can be restricted to either array or Traversable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@php-pulls php-pulls merged commit d9a9cf8 into php:master Jul 4, 2016

1 check passed

Details continuous-integration/travis-ci/pr The Travis CI build passed
@brutuscat

It would be pretty cool if, somehow (I have no idea if this is possible), after this pseudo type has been added, we could start using the array_* functions indiscriminately across all the iterable types.
Is there any discussion going on regarding this that I could read/join?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment