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
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
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,BeepOption 2:
- function foo(iterable $i) // $i is ReflectionNamedType of iterable
- function foo(iterable|Beep $i) // $i is ReflectionUnionType of
array,Traversable,BeepOption 3:
- function foo(iterable $i) // $i is ReflectionNamedType of iterable
- function foo(iterable|Beep $i) // $i is ReflectionUnionType of
iterable,BeepAm 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
--
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
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?
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
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)