Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117578
User-Agent: Cyrus-JMAP/3.7.0-alpha0-569-g7622ad95cc-fm-20220421.002-g7622ad95
Mime-Version: 1.0
Message-ID: <1658b0e3-4c0c-447e-85ed-531f70e4b6f7@www.fastmail.com>
In-Reply-To: 
 <CAFPFaMLgtm04xwgPsrUBXtOCNRwKgDv5oiiS7aPRr0sqMd4+_g@mail.gmail.com>
References: 
 <CAFPFaMLgtm04xwgPsrUBXtOCNRwKgDv5oiiS7aPRr0sqMd4+_g@mail.gmail.com>
Date: Sat, 23 Apr 2022 13:06:14 -0500
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain
Subject: Re: [PHP-DEV] Canonicalize "iterable" into "array|Traversable" and Reflection
From: larry@garfieldtech.com ("Larry Garfield")

On Sat, Apr 23, 2022, at 7:27 AM, G. P. B. wrote:
> Hello internals,
>
> One area the engine currently needs to take special care is dealing with
> the typing relation between iterable, Traversable, and array.
> The change is to canonicalize "iterable" into "array|Traversable" as this
> then removes this special case.
>
> However, doing so breaks Reflection for iterable types and will now return
> a ReflectionUnionType instead of a ReflectionNamed type.
> There are a couple of options to proceed:
>  - Accept the BC break, and expect end users to already be handling union
> types for code running on 8.2 (least complexity)
>  - Only provide a BC layer for (?)iterable to return a ReflectionNamedType
> and have usages of iterable in a union type be exposed as Traversable|array
> (OK complexity)
>  - Full BC such that even in union types where iterabl was used
> Traversable|array gets converted back to iterable (high complexity)
>
> The PR for this change is https://github.com/php/php-src/pull/7309
>
> This PR is also blocking the implementation for DNF types, as it vastly
> simplifies the implementation of it.
>
> Best regards,
>
> George P. Banyard

To make sure I understand, 

Option 1:

* function foo(iterable $i) // $i is ReflectionUnionType of array,traversable
* function foo(iterable|Beep $i) // $i is ReflectionUnionType of array,Traversable,Beep

Option 2:

* function foo(iterable $i) // $i is ReflectionNamedType of iterable
* function foo(iterable|Beep $i) // $i is ReflectionUnionType of array,Traversable,Beep

Option 3:

* function foo(iterable $i) // $i is ReflectionNamedType of iterable
* function foo(iterable|Beep $i) // $i is ReflectionUnionType of iterable,Beep

Am I following correctly?

If so, I'm fine with 1 or 2.  I... cannot imagine a situation where someone is doing iterable|Beep to begin with, so I don't really care if that is a slight break.

Though... I do have some code that might get cranky with Option 1, as it doesn't deal with union types at all (for various reasons).  I don't think it currently deals with iterables either, which is possibly an oversight...  So, I think my preference would be 2, 1, no 3.

--Larry Garfield