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

MethodError: isvalid(::String31) #60

Closed
jariji opened this issue Jan 4, 2023 · 4 comments
Closed

MethodError: isvalid(::String31) #60

jariji opened this issue Jan 4, 2023 · 4 comments

Comments

@jariji
Copy link

jariji commented Jan 4, 2023

This method would be useful:

ERROR: MethodError: no method matching isvalid(::String31)
@nickrobinson251
Copy link
Collaborator

nickrobinson251 commented Jan 4, 2023

i wonder if the correct definition is just the same as for String
https://github.com/JuliaLang/julia/blob/cdcb07f13c10611f3a2fe4d491a8d0342ad11d12/base/strings/string.jl#L194-L201

(here's the u8_isvalid definition: https://github.com/JuliaLang/julia/blob/cdcb07f13c10611f3a2fe4d491a8d0342ad11d12/src/support/utf8.c#L542-L596)

i.e. something like

Base.isvalid(s::InlineString) = ccall(:u8_isvalid, Int32, (Ptr{UInt8}, Int), s, sizeof(s)) != 0

i think the only thing this doesn't do is check the last byte (the length of the InlineString) in any way -- do we need to check that?

@quinnj
Copy link
Member

quinnj commented Jan 4, 2023

Nah, we don't need to check the length; yeah, that definition sounds good to me. @jariji, mind making a PR with a test?

@stevengj
Copy link
Member

stevengj commented Jun 23, 2023

With JuliaLang/julia#47880 (which should be in Julia 1.10), you shouldn't need to do anything — it adds a fallback method isvalid(s::AbstractString) = @inline isvalid(String, codeunits(s)), which in turn calls isvalid(::Type{String}, bytes::AbstractVector{UInt8}). So it should just work for InlineString.

@KristofferC
Copy link
Member

I think it is fine to close this with the feature being included into 1.10 which will soon be the new LTS:

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

5 participants