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

widen join(::Any...) return type to AbstractString #56132

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

topolarity
Copy link
Member

@JeffBezanson and @vtjnash have mentioned that we might want to generalize join / * to be more like our binary operators over numbers. In particular, that would mean dynamically choosing a result AbstractString that can "faithfully" encode the concatenation of all provided sub-strings.

That means significantly weakening the result type for join(...) from ::Union{String,AnnotatedString} to ::AbstractString. This PR takes that inference hit early, without changing how join actually works.

This also happens to resolve some of the invalidation problems with StyledStrings.

We'll need more fixes though, because #56005 unfortunately means that StyledStrings invalidates itself (and its dependencies) when require_stdlib gets involved

This is technically imprecise, but this widening is useful for two
reasons:
  1. It prevents the compiler from creating `AnnotatedString` types "out
     of thin air", such as when doing `join(expr.args...)` which is
     inferred as `::Union{AbstractString,String}`
  2. It is close to what inference will be able to determine if we try
     to make `*` / `join` adopt `promote_type`-like behavior in general,
     rather just as a special case for AnnotatedString (like it is now).

(1) is particularly important because the out-of-thin-air behavior can lead
to the compiler inspecting AnnotatedString methods that aren't available
until StyledStrings loads (and therefore invalidate)

An obvious (and quite common) case of this is:
  `print(io, join(v..., " "))`
which w/o this change is a bad invalidater
@topolarity
Copy link
Member Author

Given the self-invalidation issues I mentioned in the OP, I should probably update #56005 to ban all type-piracy in stdlibs (regardless of whether the behavior is actually problematic or not - the invalidation hit is not worth it)

@nsajko nsajko added the strings "Strings!" label Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
strings "Strings!"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants