-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 #29451: parse(Int, s::AbstractString) when ncodeunits(s) > lastindex(s) #29465
Conversation
Needs a sanity check from someone who knows the detail of the parser code @Keno? @StefanKarpinski?
@KristofferC, do you think the test needs to include a working julia> s = SubString("0x")
"0x"
julia> Base.lastindex(s::SubString) = s.string == "0x" ? 1 : thisind(s, s.ncodeunits)
julia> function Base.iterate(s::SubString, i::Integer=firstindex(s))
i == lastindex(s)+1 && return nothing
# Was: i == ncodeunits(s)+1 && return nothing
@boundscheck checkbounds(s, i)
y = iterate(s.string, s.offset + i)
y === nothing && return nothing
c, i = y
return c, i - s.offset
end
julia> s
"0"
julia> parse(Int, s)
ERROR: TypeError: in parseint_preamble, in typeassert, expected Tuple{Char,Int64}, got Nothing
Stacktrace:
[1] parseint_preamble(::Bool, ::Int64, ::SubString{String}, ::Int64, ::Int64) at ./parse.jl:77 |
There is an existing |
How about this: struct Issue29451String <: AbstractString end
Base.ncodeunits(::Issue29451String) = 12345
Base.isvalid(::Issue29451String, i::Integer) = i == 1
Base.lastindex(::Issue29451String) = 1
Base.iterate(::Issue29451String, i::Integer=1) = i == 1 ? ('0', 2) : nothing
@test Issue29451String() == "0"
@test Issue29451String()[1] == '0'
@test isvalid(Issue29451String(), 1)
@test !isvalid(Issue29451String(), 2)
@test parse(Int, Issue29451String()) == 0 Or even struct Issue29451String <: AbstractString end
Base.ncodeunits(::Issue29451String) = 12345
Base.lastindex(::Issue29451String) = 1
Base.iterate(::Issue29451String, i::Integer=1) = i == 1 ? ('0', 2) : nothing
@test Issue29451String() == "0"
@test parse(Int, Issue29451String()) == 0 |
Nice test case! Looks good to me once it passes tests. |
CI looks good except for something about |
#29451 :
Bug introduced here ? 1a1d6b6#r30713661
Needs a sanity check from someone who knows the detail of the parser code @Keno? @StefanKarpinski?