Skip to content

relax dispatch for the IteratorSize method for Generator #58110

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

Conversation

nsajko
Copy link
Contributor

@nsajko nsajko commented Apr 14, 2025

Fixes #58109

@nsajko nsajko added bugfix This change fixes an existing bug iteration Involves iteration or the iteration protocol backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 labels Apr 14, 2025
@KristofferC KristofferC mentioned this pull request Apr 16, 2025
51 tasks
@@ -98,7 +98,7 @@ IteratorSize(::Type{Any}) = SizeUnknown()

IteratorSize(::Type{<:Tuple}) = HasLength()
IteratorSize(::Type{<:AbstractArray{<:Any,N}}) where {N} = HasShape{N}()
IteratorSize(::Type{Generator{I,F}}) where {I,F} = IteratorSize(I)
IteratorSize(::Type{<:Generator{I}}) where {I} = (@isdefined I) ? IteratorSize(I) : SizeUnknown()
Copy link
Member

Choose a reason for hiding this comment

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

Apologies for the dumb question, but in which scenario would @isdefined I return false ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The bottom type does it:

julia> struct S{P} end
 
julia> f(::Type{<:S}) = "fallback"
f (generic function with 1 method)
 
julia> f(::Type{<:S{P}}) where {P} = (@isdefined P) ? "is defined" : "is not defined"
f (generic function with 2 methods)
 
julia> f(Union{})
"is not defined"

@isdefined is often used like that in eltype method definitions.

@KristofferC KristofferC mentioned this pull request May 9, 2025
58 tasks
@fingolfin fingolfin added the merge me PR is reviewed. Merge when all tests are passing label May 31, 2025
@nsajko nsajko force-pushed the relax_dispatch_for_Generator_IteratorSize branch from f1dfaaf to 6d55b1e Compare May 31, 2025 09:30
@nsajko
Copy link
Contributor Author

nsajko commented May 31, 2025

Rebased to get rid of the merge commit. Not sure, but I think the merge commit could have prevented automatic backport.

@giordano
Copy link
Member

Most PRs are squash-merged, there's no merge commit anyway.

@nsajko
Copy link
Contributor Author

nsajko commented May 31, 2025

I think the autobackport functionality maybe works directly with the PR, not with the squashed commit, though? Again, not sure.

@giordano giordano merged commit 805f85f into JuliaLang:master May 31, 2025
7 checks passed
@giordano giordano removed the merge me PR is reviewed. Merge when all tests are passing label May 31, 2025
@nsajko nsajko deleted the relax_dispatch_for_Generator_IteratorSize branch May 31, 2025 17:35
KristofferC pushed a commit that referenced this pull request Jun 2, 2025
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Jun 4, 2025
@KristofferC KristofferC mentioned this pull request Jun 4, 2025
75 tasks
KristofferC pushed a commit that referenced this pull request Jun 5, 2025
KristofferC pushed a commit that referenced this pull request Jun 5, 2025
KristofferC pushed a commit that referenced this pull request Jun 5, 2025
KristofferC pushed a commit that referenced this pull request Jun 5, 2025
KristofferC pushed a commit that referenced this pull request Jun 5, 2025
KristofferC pushed a commit that referenced this pull request Jun 25, 2025
KristofferC pushed a commit that referenced this pull request Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 bugfix This change fixes an existing bug iteration Involves iteration or the iteration protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IteratorSize incorrect for some subtypes of Generator
4 participants