Skip to content

Commit f13ee94

Browse files
committed
Improve annotation *-concatenation behaviour
SubStrings have been overlooked, and thanks to a few compiler quirks (relating to inlining and effect analysis), adding support for them is unfortunately a little more complicated than adding a "|| s isa SubString{<:AnnotatedString}" clause thanks to the new generated runtime-checks. To maintain the zero-overhead non-annotated code path, we need to implement a separate function _isannotated, which we also make use of to simplify the current join method.
1 parent d5c30d8 commit f13ee94

File tree

4 files changed

+12
-9
lines changed

4 files changed

+12
-9
lines changed

base/strings/basic.jl

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -257,9 +257,7 @@ julia> 'j' * "ulia"
257257
```
258258
"""
259259
function (*)(s1::Union{AbstractChar, AbstractString}, ss::Union{AbstractChar, AbstractString}...)
260-
isannotated = s1 isa AnnotatedString || s1 isa AnnotatedChar ||
261-
any(s -> s isa AnnotatedString || s isa AnnotatedChar, ss)
262-
if isannotated
260+
if _isannotated(s1) || any(_isannotated, ss)
263261
annotatedstring(s1, ss...)
264262
else
265263
string(s1, ss...)
@@ -268,6 +266,12 @@ end
268266

269267
one(::Union{T,Type{T}}) where {T<:AbstractString} = convert(T, "")
270268

269+
# This could be written as a single statement with three ||-clauses, however then effect
270+
# analysis thinks it may throw and runtime checks are added.
271+
# Also see `substring.jl` for the `::SubString{T}` method.
272+
_isannotated(S::Type) = S != Union{} && (S <: AnnotatedString || S <: AnnotatedChar)
273+
_isannotated(s) = _isannotated(typeof(s))
274+
271275
## generic string comparison ##
272276

273277
"""

base/strings/io.jl

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -365,10 +365,7 @@ function join_annotated(iterator, delim="", last=delim)
365365
end
366366

367367
function _join_maybe_annotated(args...)
368-
if any(function (arg)
369-
t = eltype(arg)
370-
!(t == Union{}) && (t <: AnnotatedString || t <: AnnotatedChar)
371-
end, args)
368+
if any(_isannotated eltype, args)
372369
join_annotated(args...)
373370
else
374371
sprint(join, args...)

base/strings/substring.jl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ function hash(s::SubString{String}, h::UInt)
140140
ccall(memhash, UInt, (Ptr{UInt8}, Csize_t, UInt32), s, sizeof(s), h % UInt32) + h
141141
end
142142

143+
_isannotated(::SubString{T}) where {T} = _isannotated(T)
144+
143145
"""
144146
reverse(s::AbstractString) -> AbstractString
145147

base/strings/util.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ function lpad(
459459
n::Integer,
460460
p::Union{AbstractChar,AbstractString}=' ',
461461
)
462-
stringfn = if any(isa.((s, p), Union{AnnotatedString, AnnotatedChar, SubString{<:AnnotatedString}}))
462+
stringfn = if _isannotated(s) || _isannotated(p)
463463
annotatedstring else string end
464464
n = Int(n)::Int
465465
m = signed(n) - Int(textwidth(s))::Int
@@ -491,7 +491,7 @@ function rpad(
491491
n::Integer,
492492
p::Union{AbstractChar,AbstractString}=' ',
493493
)
494-
stringfn = if any(isa.((s, p), Union{AnnotatedString, AnnotatedChar, SubString{<:AnnotatedString}}))
494+
stringfn = if _isannotated(s) || _isannotated(p)
495495
annotatedstring else string end
496496
n = Int(n)::Int
497497
m = signed(n) - Int(textwidth(s))::Int

0 commit comments

Comments
 (0)