Skip to content
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

more precise eltype for some Generator subtypes #56899

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

nsajko
Copy link
Contributor

@nsajko nsajko commented Dec 24, 2024

Fixes #41519

Fixes #48448

@nsajko nsajko added collections Data structures holding multiple items, e.g. sets iteration Involves iteration or the iteration protocol and removed collections Data structures holding multiple items, e.g. sets labels Dec 24, 2024
base/generator_eltype.jl Outdated Show resolved Hide resolved
Comment on lines +1 to +3
function eltype(::Type{Generator{A, typeof(identity)}}) where {A}
eltype(A)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL that x for x in A is noticed by the parser, and produces this type:

julia> (x for x in 1:3)
Base.Generator{UnitRange{Int64}, typeof(identity)}(identity, 1:3)

julia> (identity(x) for x in 1:3)
Base.Generator{UnitRange{Int64}, var"#13#14"}(var"#13#14"(), 1:3)

(Was true on Julia 1.6, not true on 1.0.)

I wonder what would break if this instead returned A, i.e. we made this (x for x in 1:3) === 1:3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I discovered the same thing while working on the PR, and wonder the same thing, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I know what would break. map(f, x) is implemented as something like collect(Iterators.map(f, x)). This relies on the fact that Generator has EltypeUnknown.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lots of code assumes that, e.g., map(identity, x) will return a collection with more precise eltype than x has. This is what relies on Generator having EltypeUnknown.

test/iterators.jl Outdated Show resolved Hide resolved
@nsajko nsajko force-pushed the eltype_generator branch 3 times, most recently from d1bb027 to d620954 Compare December 24, 2024 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iteration Involves iteration or the iteration protocol
Projects
None yet
2 participants