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

Adding InlineStrings and conversion semantics for NamedAtom structs #146

Closed
wants to merge 4 commits into from

Conversation

brainandforce
Copy link
Owner

Previously, the NamedAtom struct was defined like such:

struct NamedAtom
    name::NTuple{8,Char}
    num::Int
    function NamedAtom(name::AbstractString, num::Integer)
        codepoints = ntuple(Val{8}()) do i
            i <= length(name) ? name[i] : '\0'
        end
        return new(codepoints, num)
    end
end

function Base.getproperty(atom::NamedAtom, p::Symbol)
    if p == :name
        l = findfirst(isequal('\0'), getfield(atom, p))
        return string(getfield(atom, p)[1:l-1]...)
    end
    return getfield(atom, p)
end

We used an NTuple{8,Char} to store a fixed-length C string so that NamedAtom can remain a bits type, and then overloaded Base.getproperty() and the constructors to deal with the fact that we want to treat the fixed string data as a Julia string. However, the InlineStrings.jl package defines fixed-length strings as their own data type, with all other Julia string operations supported, so this should significantly simplify the code. I've also allowed the name field to hold up to 15 characters, an improvement from the previous 8.

On top of that, I've included conversion semantics for NamedAtom structs, allowing them to be converted to strings or numbers easily.

@brainandforce
Copy link
Owner Author

It appears that this branch is failing checks because of method ambiguities that not only I introduced, but the InlineStrings.jl package introduced. I'll hold off on merging this for now.

@brainandforce
Copy link
Owner Author

Link to the issue I've opened on InlineStrings.jl regarding this problem: JuliaStrings/InlineStrings.jl#64

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

Successfully merging this pull request may close these issues.

1 participant