Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Different promotion behavior on 1.10 vs 1.11 #102

Open
MilesCranmer opened this issue Oct 29, 2024 · 3 comments
Open

Different promotion behavior on 1.10 vs 1.11 #102

MilesCranmer opened this issue Oct 29, 2024 · 3 comments

Comments

@MilesCranmer
Copy link
Member

MilesCranmer commented Oct 29, 2024

It seems like there's a different behavior in the promotion rules between 1.10 and 1.11. I wasn't sure if this was intentional or not?

On 1.11:

julia> styled"foo" |> typeof
Base.AnnotatedString{String}

julia> "foo" * styled"foo" |> typeof
Base.AnnotatedString{String}

On 1.10:

julia> styled"foo" |> typeof
StyledStrings.AnnotatedStrings.AnnotatedString{String}

julia> "foo" * styled"foo" |> typeof
String

It was causing some type instabilities in my code as I had presumed everything would be an annotated string after concatenating with strings, but on 1.10 it seems that a regular String overpowers the annotations.

@MilesCranmer
Copy link
Member Author

I think this also means this example from the docs:

julia> str * str2 == annotatedstring(str, str2) # *-concatenation works
true

Is true, but not really true on v1.10:

julia> StyledStrings.annotatedstring("foo", styled"foo") |> typeof
StyledStrings.AnnotatedStrings.AnnotatedString{String}

julia> "foo" * styled"foo" |> typeof
String

i.e., they are actually different

MilesCranmer added a commit to MilesCranmer/SymbolicRegression.jl that referenced this issue Oct 30, 2024
@MilesCranmer
Copy link
Member Author

MilesCranmer commented Oct 30, 2024

I just read #57 by @kimikage @tecosaur. I think I understand the issue a bit more. I agree that avoiding method overwriting is a higher priority than fixing this issue so I won't suggest bringing that back.

However, perhaps there is a way to avoid method overwriting while also enabling * to work on the LTS version of Julia – by defining the following methods?

@static if VERSION < v"1.11-"
    for i in 0:10
        front = [Symbol('s', j) for j in 1:i]
        front_typed = [:($s::Union{AbstractChar,AbstractString}) for s in front]
        @eval function Base.:*($(front_typed...), s_annot::AnnotatedString, s::Union{AbstractChar,AbstractString}...)
            annotatedstring($(front...), s_annot, s...)
        end
    end
end

this would generate the following code:

Base.:*(s_annot::AnnotatedString, s::Union{AbstractChar,AbstractString}...) = annotatedstring(s_annot, s...)
Base.:*(s1::Union{AbstractChar,AbstractString}, s_annot::AnnotatedString, s::Union{AbstractChar,AbstractString}...) = annotatedstring(s1, s_annot, s...)
Base.:*(s1::Union{AbstractChar,AbstractString}, s2::Union{AbstractChar,AbstractString}, s_annot::AnnotatedString, s::Union{AbstractChar,AbstractString}...) = annotatedstring(s1, s2, s_annot, s...)
Base.:*(s1::Union{AbstractChar,AbstractString}, s2::Union{AbstractChar,AbstractString}, s3::Union{AbstractChar,AbstractString}, s_annot::AnnotatedString, s::Union{AbstractChar,AbstractString}...) = annotatedstring(s1, s2, s3, s_annot, s...)
Base.:*(s1::Union{AbstractChar,AbstractString}, s2::Union{AbstractChar,AbstractString}, s3::Union{AbstractChar,AbstractString}, s4::Union{AbstractChar,AbstractString}, s_annot::AnnotatedString, s::Union{AbstractChar,AbstractString}...) = annotatedstring(s1, s2, s3, s4, s_annot, s...)

and so on.

This would avoid type piracy on Base.

However, the issue with this is that it would only work up to whatever limit you use, after which it would fall back to the same String-creating behavior that currently occurs.

@tecosaur
Copy link
Collaborator

tecosaur commented Dec 2, 2024

Hi Miles,

I'm sorry to say that the notification for this issue slipped past me 🙈. I did try exactly this (with PrecompileTools as well), but unfortunately it still turned out to be highly invalidating 😔.

I'm really not a fan of this difference in behaviour, but since things aren't looking (as) pretty is generally preferable to a major latency spike every Julia session, I don't see a better path forwards than waiting until 1.11+ becomes the norm (as long as that will be).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants