From 29f695029675312e16f8fe551535b7ebe556a33a Mon Sep 17 00:00:00 2001 From: Timothy Date: Thu, 25 Jul 2024 04:39:49 +0800 Subject: [PATCH] Fix annotated join with non-concrete eltype iters (#54919) As raised in , when the eltype of an iterator is non-concrete, `_isannotated` will return false. To properly check such cases, we need to see if any of the elements of the iterator are annotated. This is a bit of an interesting case, since: - Most of the time it shouldn't be hit, we can reasonably expect most iterables to infer as producing concrete types - The eltype of the iterator is (generally) known at compile-time, and so in any case other than the ambiguous non-concrete one, this check remains able to be done at compile-time. With this change, join always preserves annotations. The compromise made is that iterators with non-concrete eltypes can result in join inferring a union return type (i.e. type instability with non-concrete iterators), but that doesn't seem too bad to me (I don't see how we could be completely type stable without concrete types here). (cherry picked from commit 462d7f4c4cb2d4a13a1e056b39cb3ea84fb5082d) --- base/strings/io.jl | 16 ++++++++++++---- test/strings/annotated.jl | 2 ++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/base/strings/io.jl b/base/strings/io.jl index 3f8f531ae6ec4..f9682ffaacf29 100644 --- a/base/strings/io.jl +++ b/base/strings/io.jl @@ -354,12 +354,20 @@ function join(io::IO, iterator, delim="") end function _join_preserve_annotations(iterator, args...) - if _isannotated(eltype(iterator)) || any(_isannotated, args) + if isconcretetype(eltype(iterator)) && !_isannotated(eltype(iterator)) && !any(_isannotated, args) + sprint(join, iterator, args...) + else io = AnnotatedIOBuffer() join(io, iterator, args...) - read(seekstart(io), AnnotatedString{String}) - else - sprint(join, iterator, args...) + # If we know (from compile time information, or dynamically in the case + # of iterators with a non-concrete eltype), that the result is annotated + # in nature, we extract an `AnnotatedString`, otherwise we just extract + # a plain `String` from `io`. + if isconcretetype(eltype(iterator)) || !isempty(io.annotations) + read(seekstart(io), AnnotatedString{String}) + else + String(take!(io.io)) + end end end diff --git a/test/strings/annotated.jl b/test/strings/annotated.jl index 944608fa8c4a9..62bf9a0c91c00 100644 --- a/test/strings/annotated.jl +++ b/test/strings/annotated.jl @@ -103,6 +103,8 @@ end [(1:4, :label => 5), (5:5, :label => 2), (6:9, :label => 5)]) + @test join((String(str1), str1), ' ') == + Base.AnnotatedString("test test", [(6:9, :label => 5)]) @test repeat(str1, 2) == Base.AnnotatedString("testtest", [(1:8, :label => 5)]) @test repeat(str2, 2) == Base.AnnotatedString("casecase", [(2:3, :label => "oomph"), (6:7, :label => "oomph")])