Skip to content

Commit

Permalink
Specialised substring equality for annotated strs (#54302)
Browse files Browse the repository at this point in the history
The least-bad idea I've had so far for fixing #53042.

I figure this fixes the bug raised there, and we can always switch to a
clearly-better solution if one appears.

The fact that only a string without annotations is equal to a
non-annotated string (in order to preserve the transitivity of
equality), makes the generic fallback methods for string comparison
insufficient.

As such, ==(::AnnoatedString, ::AbstractString) is implemented in
annotated.jl, but this issue re-appears when dealing with substrings.

The obvious solution is to just implement a specialised method for
substrings. This does seem potentially a bit whack-a-mole, but I'm
worried that cleverer solutions might come with subtle issues of their
own. For now, let's try the simple and obvious solution, and improve it
later if we can work out a nicer way of handling this issue in general.

(cherry picked from commit 185f058)
  • Loading branch information
tecosaur authored and KristofferC committed May 6, 2024
1 parent 38e42d9 commit d5237bf
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 0 deletions.
18 changes: 18 additions & 0 deletions base/strings/annotated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,24 @@ cmp(a::AnnotatedString, b::AnnotatedString) = cmp(a.string, b.string)
==(a::AnnotatedString, b::AbstractString) = isempty(a.annotations) && a.string == b
==(a::AbstractString, b::AnnotatedString) = isempty(b.annotations) && a == b.string

# To prevent substring equality from hitting the generic fallback

function ==(a::SubString{<:AnnotatedString}, b::SubString{<:AnnotatedString})
SubString(a.string.string, a.offset, a.ncodeunits, Val(:noshift)) ==
SubString(b.string.string, b.offset, b.ncodeunits, Val(:noshift)) &&
annotations(a) == annotations(b)
end

==(a::SubString{<:AnnotatedString}, b::AnnotatedString) =
annotations(a) == annotations(b) && SubString(a.string.string, a.offset, a.ncodeunits, Val(:noshift)) == b.string

==(a::SubString{<:AnnotatedString}, b::AbstractString) =
isempty(annotations(a)) && SubString(a.string.string, a.offset, a.ncodeunits, Val(:noshift)) == b

==(a::AbstractString, b::SubString{<:AnnotatedString}) = b == a

==(a::AnnotatedString, b::SubString{<:AnnotatedString}) = b == a

"""
annotatedstring(values...)
Expand Down
4 changes: 4 additions & 0 deletions test/strings/annotated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
@test "a" * str == Base.AnnotatedString("asome string")
@test str * "a" == Base.AnnotatedString("some stringa")
@test str * str == Base.AnnotatedString("some stringsome string")
@test str[3:4] == SubString("me")
@test SubString("me") == str[3:4]
Base.annotate!(str, 1:4, :thing => 0x01)
Base.annotate!(str, 6:11, :other => 0x02)
Base.annotate!(str, 1:11, :all => 0x03)
Expand All @@ -21,6 +23,8 @@
# └───┰─────┘
# :all
@test str[3:4] == SubString(str, 3, 4)
@test str[3:4] != SubString("me")
@test SubString("me") != str[3:4]
@test Base.AnnotatedString(str[3:4]) ==
Base.AnnotatedString("me", [(1:2, :thing => 0x01), (1:2, :all => 0x03)])
@test Base.AnnotatedString(str[3:6]) ==
Expand Down

0 comments on commit d5237bf

Please sign in to comment.