Skip to content

iterator_to_array() and iterator_count() now accepts arrays #1938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 4, 2022

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Nov 4, 2022

No description provided.

Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically the stub uses iterable, but I guess it makes sense to use the explicit union here, especially when combined with the internal change.

@Girgias
Copy link
Member Author

Girgias commented Nov 4, 2022

Technically the stub uses iterable, but I guess it makes sense to use the explicit union here, especially when combined with the internal change.

I've generated the change via gen_stubs, so maybe that's why. But in either case it's equivalent :)

Co-authored-by: Tim Düsterhus <timwolla@googlemail.com>
@Girgias Girgias merged commit 70ac605 into php:master Nov 4, 2022
@Girgias Girgias deleted the iterator-iterable branch November 4, 2022 12:54
@Girgias Girgias added this to the PHP 8.2 milestone Nov 12, 2022
tiffany-taylor pushed a commit to tiffany-taylor/doc-en that referenced this pull request Jan 16, 2023
Co-authored-by: Tim Düsterhus <timwolla@googlemail.com>
claudepache pushed a commit to claudepache/php-doc-en that referenced this pull request Jun 1, 2023
Co-authored-by: Tim Düsterhus <timwolla@googlemail.com>
@vudaltsov
Copy link
Contributor

Today I suddenly learned about this feature and was very happy to remove if ($myIterable instanceof Traversable). Nevertheless, it was illogical to see array|Traversable instead of iterable in the documentation. I started researching whether this has any special meaning or not, and checked the implementation PR first, and finally this one.

Do you mind if I change array|Traversable to iterable in a separate PR?

@TimWolla
Copy link
Member

Do you mind if I change array|Traversable to iterable in a separate PR?

According to Girgias, the explicit union is how it falls out of the automated generator consuming the stubs. Thus a manual change would be undesirable. This would need to be handled in the generator.

@Girgias
Copy link
Member Author

Girgias commented May 20, 2024

iterable is converted to array|Traversable at compile time and this is the information that Reflection sees, I don't really see a benefit in forcing it to be iterable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants