Skip to content

Commit 53dfef5

Browse files
quinnjStefanKarpinskinickrobinson251
authored and
KristofferC
committed
Generalize Bool parse method to AbstractString (#47782)
* Generalize Bool parse method to AbstractString Fixes JuliaStrings/InlineStrings.jl#57. We currently have a specialization for `parse(Bool, ::Union{String, SubString{String})` where `true` and `false` are parsed appropriately. The restriction to `Union{String, SubString{String}}`, however, means we don't get this behavior for other `AbstractString`s. In the linked issue above, for InlineStrings, we end up going through the generic integer parsing codepath which results in an `InexactError` when we try to do `Bool(10)`. The proposal in this PR takes advantage of the fact that there is only the 2 comparisons where we do `_memcmp` that require the input string to be "dense" (in memory), and otherwise, we just do a comparison against a `SubString` of the input string. Relatedly, I've wanted to introduce the concept of an abstrac type like: ```julia abstract type MemoryAddressableString <: AbstractString ``` where the additional required interface would be being able to call `pointer(::MemoryAddressableString)`, since a lot of our string algorithms depend on doing these kind of pointer operations and hence makes it quite a pain to implement your own custom string type. * Apply suggestions from code review Co-authored-by: Stefan Karpinski <stefan@karpinski.org> Co-authored-by: Nick Robinson <npr251@gmail.com> Co-authored-by: Stefan Karpinski <stefan@karpinski.org> Co-authored-by: Nick Robinson <npr251@gmail.com> (cherry picked from commit 63830a6)
1 parent d3791a0 commit 53dfef5

File tree

2 files changed

+20
-5
lines changed

2 files changed

+20
-5
lines changed

base/parse.jl

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ function tryparse_internal(::Type{T}, s::AbstractString, startpos::Int, endpos::
176176
return n
177177
end
178178

179-
function tryparse_internal(::Type{Bool}, sbuff::Union{String,SubString{String}},
179+
function tryparse_internal(::Type{Bool}, sbuff::AbstractString,
180180
startpos::Int, endpos::Int, base::Integer, raise::Bool)
181181
if isempty(sbuff)
182182
raise && throw(ArgumentError("input string is empty"))
@@ -202,10 +202,15 @@ function tryparse_internal(::Type{Bool}, sbuff::Union{String,SubString{String}},
202202
end
203203

204204
len = endpos - startpos + 1
205-
p = pointer(sbuff) + startpos - 1
206-
GC.@preserve sbuff begin
207-
(len == 4) && (0 == _memcmp(p, "true", 4)) && (return true)
208-
(len == 5) && (0 == _memcmp(p, "false", 5)) && (return false)
205+
if sbuff isa Union{String, SubString{String}}
206+
p = pointer(sbuff) + startpos - 1
207+
GC.@preserve sbuff begin
208+
(len == 4) && (0 == _memcmp(p, "true", 4)) && (return true)
209+
(len == 5) && (0 == _memcmp(p, "false", 5)) && (return false)
210+
end
211+
else
212+
(len == 4) && (SubString(sbuff, startpos:startpos+3) == "true") && (return true)
213+
(len == 5) && (SubString(sbuff, startpos:startpos+4) == "false") && (return false)
209214
end
210215

211216
if raise

test/parse.jl

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,16 @@ Base.iterate(::Issue29451String, i::Integer=1) = i == 1 ? ('0', 2) : nothing
4141
@test Issue29451String() == "0"
4242
@test parse(Int, Issue29451String()) == 0
4343

44+
# https://github.com/JuliaStrings/InlineStrings.jl/issues/57
45+
struct InlineStringIssue57 <: AbstractString end
46+
Base.ncodeunits(::InlineStringIssue57) = 4
47+
Base.lastindex(::InlineStringIssue57) = 4
48+
Base.isvalid(::InlineStringIssue57, i::Integer) = 0 < i < 5
49+
Base.iterate(::InlineStringIssue57, i::Integer=1) = i == 1 ? ('t', 2) : i == 2 ? ('r', 3) : i == 3 ? ('u', 4) : i == 4 ? ('e', 5) : nothing
50+
Base.:(==)(::SubString{InlineStringIssue57}, x::String) = x == "true"
51+
52+
@test parse(Bool, InlineStringIssue57())
53+
4454
@testset "Issue 20587, T=$T" for T in Any[BigInt, Int128, Int16, Int32, Int64, Int8, UInt128, UInt16, UInt32, UInt64, UInt8]
4555
T === BigInt && continue # TODO: make BigInt pass this test
4656
for s in ["", " ", " "]

0 commit comments

Comments
 (0)