From 538676288edf6471c6082e0eb3ef1834ebcdb736 Mon Sep 17 00:00:00 2001 From: Jake Ireland Date: Thu, 30 Sep 2021 19:59:35 +1300 Subject: [PATCH 1/2] Performance improvement and string safety in addcommas --- src/cformat.jl | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/cformat.jl b/src/cformat.jl index fec6822..5cfc043 100644 --- a/src/cformat.jl +++ b/src/cformat.jl @@ -73,19 +73,20 @@ function checkcommas(s) s end - -function addcommas( s::String ) +function addcommas(s::S) where {S <: AbstractString} len = length(s) + inds = eachindex(s) + firstind = firstindex(s) t = "" for i in 1:3:len - subs = s[max(1,len-i-1):len-i+1] + substr = SubString(s, max(firstind, prevind(s, len, i + 1)), nextind(s, prevind(s, len, i))) if i == 1 - t = subs + t = substr else - if match( r"[0-9]", subs ) != nothing - t = subs * "," * t + if any(isdigit, substr) + t = substr * "," * t else - t = subs * t + t = substr * t end end end From a55880124492fa1d5c280830919ef66761ff9a27 Mon Sep 17 00:00:00 2001 From: Jake Ireland Date: Thu, 30 Sep 2021 20:04:01 +1300 Subject: [PATCH 2/2] Improved string safety, and made performance considerations in findfirst --- src/cformat.jl | 50 ++++++++++++++++++++++++++--------------------- src/formatexpr.jl | 27 +++++++++++++------------ 2 files changed, 42 insertions(+), 35 deletions(-) diff --git a/src/cformat.jl b/src/cformat.jl index 5cfc043..e3cc5a8 100644 --- a/src/cformat.jl +++ b/src/cformat.jl @@ -48,29 +48,30 @@ function generate_formatter( fmt::String ) end function addcommasreal(s) - dpos = findfirst( isequal('.'), s ) - dpos !== nothing && return string(addcommas( s[1:dpos-1] ), s[ dpos:end ]) + dpos = findfirst(==('.'), s) + firstind = firstindex(s) + dpos !== nothing && return string(addcommas(SubString(s, firstind, prevind(s, dpos))), SubString(s, dpos)) # find the rightmost digit - for i in length( s ):-1:1 - isdigit( s[i] ) && return string(addcommas( s[1:i] ), s[i+1:end]) + for i in Iterators.reverse(eachindex(s)) + isdigit(s[i]) && return string(addcommas(SubString(s, firstind, i)), SubString(s, nextind(s, i))) end - s + return s end function addcommasrat(s) # commas are added to only the numerator - spos = findfirst( isequal('/'), s ) - string(addcommas( s[1:spos-1] ), s[spos:end]) + spos = findfirst('/', s) + string(addcommas(SubString(s, firstindex(s), prevind(s, spos))), SubString(s, spos)) end function checkcommas(s) - for i in length( s ):-1:1 - if isdigit( s[i] ) - s = string(addcommas( s[1:i] ), s[i+1:end]) - break + firstind = firstindex(s) + for i in Iterators.reverse(eachindex(s)) + if isdigit(s[i]) + return string(addcommas(SubString(s, firstind, i)), SubString(s, nextind(s, i))) end end - s + return s end function addcommas(s::S) where {S <: AbstractString} @@ -290,38 +291,43 @@ function format( x::T; checkwidth = true end elseif stripzeros && in( actualconv[1], "fFeEs" ) - dpos = findfirst( isequal('.'), s ) + dpos = findfirst('.', s) if in( actualconv[1], "eEs" ) if in( actualconv[1], "es" ) - epos = findfirst( isequal('e'), s ) + epos = findfirst('e', s) else - epos = findfirst( isequal('E'), s ) + epos = findfirst('E', s) end if epos === nothing - rpos = length( s ) + rpos = lastindex(s) else - rpos = epos-1 + rpos = prevind(s, epos) end else rpos = length(s) end # rpos at this point is the rightmost possible char to start # stripping - stripfrom = rpos+1 - for i = rpos:-1:dpos+1 + stripfrom = nextind(s, rpos) + lower_ind = nextind(s, dpos) + # for i = rpos:-1:dpos+1 + for i in Iterators.reverse(eachindex(s)) + if i > rpos || i < lower_ind + continue + end if s[i] == '0' stripfrom = i - elseif s[i] ==' ' + elseif s[i] == ' ' continue else break end end if stripfrom <= rpos - if stripfrom == dpos+1 # everything after decimal is 0, so strip the decimal too + if stripfrom == nextind(s, dpos) # everything after decimal is 0, so strip the decimal too stripfrom = dpos end - s = s[1:stripfrom-1] * s[rpos+1:end] + s = string(SubString(s, firstindex(s), prevind(s, stripfrom)), SubString(s, nextind(s, rpos))) checkwidth = true end end diff --git a/src/formatexpr.jl b/src/formatexpr.jl index 21edb69..f31f88a 100644 --- a/src/formatexpr.jl +++ b/src/formatexpr.jl @@ -28,12 +28,12 @@ function make_argspec(s::AbstractString, pos::Int) if !isempty(s) filrange = findfirst("|>", s) if filrange === nothing - iarg = parse(Int,s) + iarg = parse(Int, s) else ifil = first(filrange) - iarg = ifil > 1 ? parse(Int,s[1:prevind(s, ifil)]) : -1 + iarg = ifil > 1 ? parse(Int, SubString(s, 1, prevind(s, ifil))) : -1 hasfil = true - ff = eval(Symbol(s[ifil+2:end])) + ff = eval(Symbol(SubString(s, nextind(s, ifil, 2)))) end end @@ -62,15 +62,16 @@ struct FormatEntry end function make_formatentry(s::AbstractString, pos::Int) - @assert s[1] == '{' && s[end] == '}' - sc = s[2:prevind(s, lastindex(s))] - icolon = findfirst(isequal(':'), sc) + @assert s[firstindex(s)] == '{' && s[lastindex(s)] == '}' + firstind = firstindex(s) + sc = SubString(s, nextind(s, firstind), prevind(s, lastindex(s))) + icolon = findfirst(':', sc) if icolon === nothing # no colon (argspec, pos) = make_argspec(sc, pos) spec = FormatSpec('s') else - (argspec, pos) = make_argspec(sc[1:prevind(sc, icolon)], pos) - spec = FormatSpec(sc[nextind(sc, icolon):end]) + (argspec, pos) = make_argspec(SubString(sc, firstind, prevind(sc, icolon)), pos) + spec = FormatSpec(SubString(sc, nextind(sc, icolon))) end return (FormatEntry(argspec, spec), pos) end @@ -89,14 +90,14 @@ _raise_unmatched_lbrace() = error("Unmatched { in format expression.") function find_next_entry_open(s::AbstractString, si::Int) slen = lastindex(s) - p = findnext(isequal('{'), s, si) + p = findnext('{', s, si) (p === nothing || p < slen) || _raise_unmatched_lbrace() - while p !== nothing && s[p+1] == '{' # escape `{{` - p = findnext(isequal('{'), s, p+2) + while p !== nothing && s[nextind(s, p)] == '{' # escape `{{` + p = findnext('{', s, nextind(s, p, 2)) (p === nothing || p < slen) || _raise_unmatched_lbrace() end # println("open at $p") - pre = p !== nothing ? s[si:prevind(s, p)] : s[si:end] + pre = p !== nothing ? SubString(s, si, prevind(s, p)) : SubString(s, si) if !isempty(pre) pre = replace(pre, "{{" => '{') pre = replace(pre, "}}" => '}') @@ -105,7 +106,7 @@ function find_next_entry_open(s::AbstractString, si::Int) end function find_next_entry_close(s::AbstractString, si::Int) - p = findnext(isequal('}'), s, si) + p = findnext('}', s, si) p !== nothing || _raise_unmatched_lbrace() # println("close at $p") return p