invalid character index in printf with unicode

[JeffBezanson]

I'm seeing this:

julia> @printf("%d×%d", 1, 2)
ERROR: LoadError: UnicodeError: invalid character index
Stacktrace:
 [1] getindex(::String, ::UnitRange{Int64}) at ./strings/string.jl:253
 [2] parse(::String) at ./printf.jl:45
 [3] gen(::String) at ./printf.jl:14
 [4] _printf(::String, ::Symbol, ::String, ::Tuple{Int64,Int64}) at ./printf.jl:1148
 [5] @printf(::LineNumberNode, ::Module, ::Vararg{Any,N} where N) at ./printf.jl:1218
in expression starting at REPL[1]:1

[KlausC]

That seems to be a consequence of a change in getindex() in strings/string.jl:253.
In v0.6 we had no exception, but now we get:

julia> s = ("%d×%d")
"%d×%d"
julia> s[3:4]
ERROR: UnicodeError: invalid character index
Stacktrace:
 [1] getindex(::String, ::UnitRange{Int64}) at ./strings/string.jl:253

As 4 is not the index in s of the first byte of a UTF8 sequence, an exception is thrown. It is not clear to me, if that is designed to be the case. Documentation is quiet about byte index ranges in strings.
I commented out strings/string.jl:253 to look for failing test cases in test/strings: one test case failed: test/strings/basic.jl:173.
So I must assume the error exception is intentionally. Nevertheless it becomes inconvenient to parse strings by byte indices, if we do so.

At least parse in printf.jl:45 is assuming, it is valid to access 3:4.

[KlausC]

In the meanwhile I ran all tests, and there is no other related failure.
I am in doubt, if the change in getindex was a good idea.
Should we revert this change, or should we fix the parse function using the now discriminated index range (and potentially other applications).

[KristofferC]

This is intended: #22572

[KlausC]

Ok. Then I will have a look at parse in printf.jl:45.
Found easy fix; will open pull request with printf.jl and test cases to cover the issue.

[bkamins]

printf.jl:45 should be changed to:

i < j && push!(list, s[i:prevind(s, j)])

Other possible cleanups:

1. printf.jl:55 can be replaced with:

i > endof(s) || push!(list, s[i:end])

2. When Improved nextind and prevind #23805 is merged I would also think to replace printf.jl:101 with:

flags = String(s[j:prevind(s, k, 2)])

It is not strictly necessary now, as in practice as we can expect that in any implementation of AbstractString we will have that %, #, 0, etc. have width of 1 byte index, but in general it is not guaranteed.

[KlausC]

PR # 23899: https://github.com/JuliaLang/julia/pull/23899