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

fix some type instabilities introduced by #39607 #41094

Merged
merged 1 commit into from
Jun 7, 2021

Conversation

aviatesk
Copy link
Sponsor Member

@aviatesk aviatesk commented Jun 5, 2021

After #39607, Iterators.peel can return nothing and the return type
can be more inaccurate than before for certain iterative inputs where
inference doesn't understand its length, e.g.
Iterators.peel(::<:AbstractString).

The following JET analysis shows an example of this type instability:

═════ 2 possible errors found ═════
┌ @ coreio.jl:4 Base.println(Core.tuple(Core.typeassert(Base.stdout, Base.IO)), xs...)
│┌ @ strings/io.jl:73 Base.print(Core.tuple(io), xs, Core.tuple("\n")...)
││┌ @ strings/io.jl:46 Base.print(io, x)
│││┌ @ show.jl:1283 Base.show_unquoted(Base.IOContext(io, Base.=>(:unquote_fallback, false)), ex, 0, -1)
││││┌ @ show.jl:1634 Base.show_unquoted_quote_expr(io, Base.getproperty(ex, :value), indent, prec, 0)
│││││┌ @ show.jl:1651 Base.isidentifier(sym)
││││││┌ @ show.jl:1339 Base.isidentifier(Base.string(s))
│││││││┌ @ show.jl:1335 Base.indexed_iterate(Base.Iterators.peel(s), 1)
││││││││┌ @ tuple.jl:92 Base.iterate(I)
│││││││││ no matching method found for call signature (Tuple{typeof(iterate), Nothing}): Base.iterate(I::Nothing)
││││││││└───────────────
│││││││┌ @ show.jl:1335 Base.indexed_iterate(Base.Iterators.peel(s), 2, _3)
││││││││┌ @ tuple.jl:97 Base.iterate(I, state)
│││││││││ no matching method found for call signature (Tuple{typeof(iterate), Nothing, Int64}): Base.iterate(I::Nothing, state::Int64)
││││││││└───────────────
Core.Const(nothing)

This PR adds some manual annotations and seems to fix some regressions
within Base.

@aviatesk
Copy link
Sponsor Member Author

aviatesk commented Jun 6, 2021

When there is no objection against this, I'm going to merge this tmrw.

After #39607, `Iterators.peel` can return `nothing` and the return type
can be more inaccurate than before for certain iterative inputs where
inference doesn't understand its length, e.g.
`Iterators.peel(::<:AbstractString)`.

The following JET analysis shows an example of this type instability:
```julia
═════ 2 possible errors found ═════
┌ @ coreio.jl:4 Base.println(Core.tuple(Core.typeassert(Base.stdout, Base.IO)), xs...)
│┌ @ strings/io.jl:73 Base.print(Core.tuple(io), xs, Core.tuple("\n")...)
││┌ @ strings/io.jl:46 Base.print(io, x)
│││┌ @ show.jl:1283 Base.show_unquoted(Base.IOContext(io, Base.=>(:unquote_fallback, false)), ex, 0, -1)
││││┌ @ show.jl:1634 Base.show_unquoted_quote_expr(io, Base.getproperty(ex, :value), indent, prec, 0)
│││││┌ @ show.jl:1651 Base.isidentifier(sym)
││││││┌ @ show.jl:1339 Base.isidentifier(Base.string(s))
│││││││┌ @ show.jl:1335 Base.indexed_iterate(Base.Iterators.peel(s), 1)
││││││││┌ @ tuple.jl:92 Base.iterate(I)
│││││││││ no matching method found for call signature (Tuple{typeof(iterate), Nothing}): Base.iterate(I::Nothing)
││││││││└───────────────
│││││││┌ @ show.jl:1335 Base.indexed_iterate(Base.Iterators.peel(s), 2, _3)
││││││││┌ @ tuple.jl:97 Base.iterate(I, state)
│││││││││ no matching method found for call signature (Tuple{typeof(iterate), Nothing, Int64}): Base.iterate(I::Nothing, state::Int64)
││││││││└───────────────
Core.Const(nothing)
```

This PR adds code refactors or manual annotations and seems to fix
some of regressions within Base.
@aviatesk aviatesk merged commit 9687260 into master Jun 7, 2021
@aviatesk aviatesk deleted the avi/annotations39607 branch June 7, 2021 07:41
shirodkara pushed a commit to shirodkara/julia that referenced this pull request Jun 9, 2021
…41094)

After JuliaLang#39607, `Iterators.peel` can return `nothing` and the return type
can be more inaccurate than before for certain iterative inputs where
inference doesn't understand its length, e.g.
`Iterators.peel(::<:AbstractString)`.

The following JET analysis shows an example of this type instability:
```julia
═════ 2 possible errors found ═════
┌ @ coreio.jl:4 Base.println(Core.tuple(Core.typeassert(Base.stdout, Base.IO)), xs...)
│┌ @ strings/io.jl:73 Base.print(Core.tuple(io), xs, Core.tuple("\n")...)
││┌ @ strings/io.jl:46 Base.print(io, x)
│││┌ @ show.jl:1283 Base.show_unquoted(Base.IOContext(io, Base.=>(:unquote_fallback, false)), ex, 0, -1)
││││┌ @ show.jl:1634 Base.show_unquoted_quote_expr(io, Base.getproperty(ex, :value), indent, prec, 0)
│││││┌ @ show.jl:1651 Base.isidentifier(sym)
││││││┌ @ show.jl:1339 Base.isidentifier(Base.string(s))
│││││││┌ @ show.jl:1335 Base.indexed_iterate(Base.Iterators.peel(s), 1)
││││││││┌ @ tuple.jl:92 Base.iterate(I)
│││││││││ no matching method found for call signature (Tuple{typeof(iterate), Nothing}): Base.iterate(I::Nothing)
││││││││└───────────────
│││││││┌ @ show.jl:1335 Base.indexed_iterate(Base.Iterators.peel(s), 2, _3)
││││││││┌ @ tuple.jl:97 Base.iterate(I, state)
│││││││││ no matching method found for call signature (Tuple{typeof(iterate), Nothing, Int64}): Base.iterate(I::Nothing, state::Int64)
││││││││└───────────────
Core.Const(nothing)
```

This PR adds code refactors or manual annotations and seems to fix
some of regressions within Base.
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
…41094)

After JuliaLang#39607, `Iterators.peel` can return `nothing` and the return type
can be more inaccurate than before for certain iterative inputs where
inference doesn't understand its length, e.g.
`Iterators.peel(::<:AbstractString)`.

The following JET analysis shows an example of this type instability:
```julia
═════ 2 possible errors found ═════
┌ @ coreio.jl:4 Base.println(Core.tuple(Core.typeassert(Base.stdout, Base.IO)), xs...)
│┌ @ strings/io.jl:73 Base.print(Core.tuple(io), xs, Core.tuple("\n")...)
││┌ @ strings/io.jl:46 Base.print(io, x)
│││┌ @ show.jl:1283 Base.show_unquoted(Base.IOContext(io, Base.=>(:unquote_fallback, false)), ex, 0, -1)
││││┌ @ show.jl:1634 Base.show_unquoted_quote_expr(io, Base.getproperty(ex, :value), indent, prec, 0)
│││││┌ @ show.jl:1651 Base.isidentifier(sym)
││││││┌ @ show.jl:1339 Base.isidentifier(Base.string(s))
│││││││┌ @ show.jl:1335 Base.indexed_iterate(Base.Iterators.peel(s), 1)
││││││││┌ @ tuple.jl:92 Base.iterate(I)
│││││││││ no matching method found for call signature (Tuple{typeof(iterate), Nothing}): Base.iterate(I::Nothing)
││││││││└───────────────
│││││││┌ @ show.jl:1335 Base.indexed_iterate(Base.Iterators.peel(s), 2, _3)
││││││││┌ @ tuple.jl:97 Base.iterate(I, state)
│││││││││ no matching method found for call signature (Tuple{typeof(iterate), Nothing, Int64}): Base.iterate(I::Nothing, state::Int64)
││││││││└───────────────
Core.Const(nothing)
```

This PR adds code refactors or manual annotations and seems to fix
some of regressions within Base.
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.

None yet

1 participant