Skip to content

Commit

Permalink
fix some type instabilities introduced by #39607 (#41094)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aviatesk authored Jun 7, 2021
1 parent b43f04d commit 9687260
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 8 deletions.
2 changes: 1 addition & 1 deletion base/compiler/ssair/domtree.jl
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ function naive_idoms(blocks::Vector{BasicBlock})
if isempty(blocks[n].preds)
continue
end
firstp, rest = Iterators.peel(Iterators.filter(p->p != 0, blocks[n].preds))
firstp, rest = Iterators.peel(Iterators.filter(p->p != 0, blocks[n].preds))::NTuple{2,Any}
new_doms = copy(dominators[firstp])
for p in rest
intersect!(new_doms, dominators[p])
Expand Down
5 changes: 3 additions & 2 deletions base/io.jl
Original file line number Diff line number Diff line change
Expand Up @@ -885,8 +885,9 @@ end

function readuntil(io::IO, target::AbstractString; keep::Bool=false)
# small-string target optimizations
isempty(target) && return ""
c, rest = Iterators.peel(target)
x = Iterators.peel(target)
isnothing(x) && return ""
c, rest = x
if isempty(rest) && c <= '\x7f'
return readuntil_string(io, c % UInt8, keep)
end
Expand Down
5 changes: 3 additions & 2 deletions base/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1330,9 +1330,10 @@ julia> Meta.isidentifier(:x), Meta.isidentifier("1x")
```
"""
function isidentifier(s::AbstractString)
isempty(s) && return false
x = Iterators.peel(s)
isnothing(x) && return false
(s == "true" || s == "false") && return false
c, rest = Iterators.peel(s)
c, rest = x
is_id_start_char(c) || return false
return all(is_id_char, rest)
end
Expand Down
7 changes: 4 additions & 3 deletions base/strings/search.jl
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,12 @@ in(c::AbstractChar, s::AbstractString) = (findfirst(isequal(c),s)!==nothing)
function _searchindex(s::Union{AbstractString,ByteArray},
t::Union{AbstractString,AbstractChar,Int8,UInt8},
i::Integer)
if isempty(t)
x = Iterators.peel(t)
if isnothing(x)
return 1 <= i <= nextind(s,lastindex(s))::Int ? i :
throw(BoundsError(s, i))
end
t1, trest = Iterators.peel(t)
t1, trest = x
while true
i = findnext(isequal(t1),s,i)
if i === nothing return 0 end
Expand Down Expand Up @@ -420,7 +421,7 @@ function _rsearchindex(s::AbstractString,
return 1 <= i <= nextind(s, lastindex(s))::Int ? i :
throw(BoundsError(s, i))
end
t1, trest = Iterators.peel(Iterators.reverse(t))
t1, trest = Iterators.peel(Iterators.reverse(t))::NTuple{2,Any}
while true
i = findprev(isequal(t1), s, i)
i === nothing && return 0
Expand Down

0 comments on commit 9687260

Please sign in to comment.