Re: [PHP-DEV] Canonicalize "iterable" into "array|Traversable" and Reflection
On Sat, 23 Apr 2022 at 19:19, Marco Pivetta wrote:
> Hey George,
>
> How would the engine behave (with this RFC included) with an inheritance
> check?
>
> Specifically:
>
> interface A {
> function foo(interable $param): iterable;
> }
> interface B {
> function foo(array|\Trabersable $param): array|\Traversable:
> }
>
> Would they be compatible with each other, if put in inheritance between
> each other in any order?
>
They would, that's the main point of this change is to make handling this
sort of code easier.
As currently we need to manually check that array is indeed compatible with
iterable and similarly with Traversable.
With this change, the engine would effectively see:
interface A {
function foo(array|\Traversable $param): array|\Traversable;
}
interface B {
function foo(array|\Trabersable $param): array|\Traversable:
}
Allowing us to use the generic LSP handling (and in this case it is trivial)
Re: [PHP-DEV] Canonicalize "iterable" into "array|Traversable" and Reflection
Hey George,
How would the engine behave (with this RFC included) with an inheritance
check?
Specifically:
interface A {
function foo(interable $param): iterable;
}
interface B {
function foo(array|\Trabersable $param): array|\Traversable:
}
Would they be compatible with each other, if put in inheritance between
each other in any order?
On Sat, 23 Apr 2022, 14:28 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
>
Re: [PHP-DEV] Canonicalize "iterable" into "array|Traversable" and Reflection
On Sat, 23 Apr 2022 at 19:06, Larry Garfield wrote: > 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 > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > Correct, and a better way of visualising than I did. I do think option 2 is the most reasonable. Best regards, George P. Banyard
Re: [PHP-DEV] Canonicalize "iterable" into "array|Traversable" and Reflection
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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
[PHP-DEV] Canonicalize "iterable" into "array|Traversable" and Reflection
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
