Skip to content

Commit

Permalink
Make concatenation of SubString{AnnotatedString} preserve annotations (
Browse files Browse the repository at this point in the history
…#51806)

`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.
  • Loading branch information
tecosaur authored Jan 24, 2024
1 parent e42ffa6 commit ca7b9c3
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 11 deletions.
10 changes: 7 additions & 3 deletions base/strings/basic.jl
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,7 @@ julia> 'j' * "ulia"
```
"""
function (*)(s1::Union{AbstractChar, AbstractString}, ss::Union{AbstractChar, AbstractString}...)
isannotated = s1 isa AnnotatedString || s1 isa AnnotatedChar ||
any(s -> s isa AnnotatedString || s isa AnnotatedChar, ss)
if isannotated
if _isannotated(s1) || any(_isannotated, ss)
annotatedstring(s1, ss...)
else
string(s1, ss...)
Expand All @@ -270,6 +268,12 @@ end

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

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

## generic string comparison ##

"""
Expand Down
5 changes: 1 addition & 4 deletions base/strings/io.jl
Original file line number Diff line number Diff line change
Expand Up @@ -365,10 +365,7 @@ function join_annotated(iterator, delim="", last=delim)
end

function _join_maybe_annotated(args...)
if any(function (arg)
t = eltype(arg)
!(t == Union{}) && (t <: AnnotatedString || t <: AnnotatedChar)
end, args)
if any(_isannotated eltype, args)
join_annotated(args...)
else
sprint(join, args...)
Expand Down
2 changes: 2 additions & 0 deletions base/strings/substring.jl
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ function hash(s::SubString{String}, h::UInt)
ccall(memhash, UInt, (Ptr{UInt8}, Csize_t, UInt32), s, sizeof(s), h % UInt32) + h
end

_isannotated(::SubString{T}) where {T} = _isannotated(T)

"""
reverse(s::AbstractString) -> AbstractString
Expand Down
4 changes: 2 additions & 2 deletions base/strings/util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ function lpad(
n::Integer,
p::Union{AbstractChar,AbstractString}=' ',
)
stringfn = if any(isa.((s, p), Union{AnnotatedString, AnnotatedChar, SubString{<:AnnotatedString}}))
stringfn = if _isannotated(s) || _isannotated(p)
annotatedstring else string end
n = Int(n)::Int
m = signed(n) - Int(textwidth(s))::Int
Expand Down Expand Up @@ -491,7 +491,7 @@ function rpad(
n::Integer,
p::Union{AbstractChar,AbstractString}=' ',
)
stringfn = if any(isa.((s, p), Union{AnnotatedString, AnnotatedChar, SubString{<:AnnotatedString}}))
stringfn = if _isannotated(s) || _isannotated(p)
annotatedstring else string end
n = Int(n)::Int
m = signed(n) - Int(textwidth(s))::Int
Expand Down
5 changes: 3 additions & 2 deletions test/strings/annotated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@
Base.AnnotatedString("ab", [(1:1, :a => 1), (2:2, :b => 2)])
let allstrings =
['a', Base.AnnotatedChar('a'), Base.AnnotatedChar('a', [:aaa => 0x04]),
"a string", Base.AnnotatedString("a string"),
Base.AnnotatedString("a string", [(1:2, :hmm => '%')])]
"a string", Base.AnnotatedString("a string"),
Base.AnnotatedString("a string", [(1:2, :hmm => '%')]),
SubString(Base.AnnotatedString("a string", [(1:2, :hmm => '%')]), 1:1)]
for str1 in repeat(allstrings, 2)
for str2 in repeat(allstrings, 2)
@test String(str1 * str2) ==
Expand Down

0 comments on commit ca7b9c3

Please sign in to comment.