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

CodeUnits <: DenseVector, but does not fulfill its only criteria #53996

Open
jakobnissen opened this issue Apr 8, 2024 · 2 comments
Open

CodeUnits <: DenseVector, but does not fulfill its only criteria #53996

jakobnissen opened this issue Apr 8, 2024 · 2 comments
Labels
bug Indicates an unexpected problem or unintended behavior strings "Strings!"

Comments

@jakobnissen
Copy link
Contributor

The only (definitional) criteria of DenseArray is that it's stored contiguously in memory.
We also have Codeunits <: DenseVector. However, there is no requirement that the string backing the codeunits are stored densely in memory. For example, InlineString.jl provide string types which are 8-byte primitives.

I'm not sure what to do about this. It might be breaking to remove Codeunits <: DenseVector, but it's also silly to have it subtype DenseVector when the only thing that matters about DenseVector is not upheld.

@jakobnissen jakobnissen added bug Indicates an unexpected problem or unintended behavior strings "Strings!" labels Apr 8, 2024
@jakobnissen
Copy link
Contributor Author

See discssion in #26085

@mbauman
Copy link
Member

mbauman commented Apr 8, 2024

julia/base/strings/basic.jl

Lines 793 to 796 in 0e28cf6

struct CodeUnits{T,S<:AbstractString} <: DenseVector{T}
s::S
CodeUnits(s::S) where {S<:AbstractString} = new{codeunit(s),S}(s)
end

Ooof, that's tricky. I guess we could try the simple thing — change it to AbstractVector — and check both of nanosoldier's reports. If we need to preserve CodeUnits{UInt8, String} <: DenseVector{UInt8} the realm of possible fixes (and their associated breakages) becomes much more complicated and the tradeoffs will be hard to evaluate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior strings "Strings!"
Projects
None yet
Development

No branches or pull requests

2 participants