Add support for automatically calling unsafe_load() in getproperty()

[JamesWrigley]

Copying the description from the code:

By default the getproperty!(x::Ptr, ::Symbol) methods created for wrapped
types will return pointers (Ptr{T}) to the struct fields. That behaviour is
useful for accessing nested struct fields but it does require explicitly
calling unsafe_load() every time. When enabled this option will automatically
call unsafe_load() for you except on nested struct fields and arrays, which
should make explicitly calling unsafe_load() unnecessary in most cases.

Here's the generated code for the struct-properties.h header in the tests:

using CEnum: CEnum, @cenum                                                                                                                                                                                                           [75/97867]
                                                                                                                                                                                                                                               
struct TypedefStruct                                                                                                                                                                                                                           
    i::Cint                                                                                                                                                                                                                                    
end                                                                                                                                                                                                                                            
                                                                                                                                                                                                                                               
struct Other                                                                                                                                                                                                                                   
    i::Cint                                                                                                                                                                                                                                    
end                                                                                                                                                                                                                                            
function _getptr(x::Ptr{Other}, f::Symbol)                                                                                                                                                                                                     
    f === :i && return Ptr{Cint}(x + 0)                                                                                                                                                                                                        
    error("Unrecognized field of type `Other`" * ": $(f)")                                                                                                                                                                                     
end                                                                                                                                                                                                                                            
                                                                                                                                                                                                                                               
function Base.getproperty(x::Ptr{Other}, f::Symbol)                                                                                                                                                                                            
    f === :i && return unsafe_load(_getptr(x, f))                                                                                                                                                                                              
    return getfield(x, f)                                                                                                                                                                                                                      
end                                                                                                                                                                                                                                            
                                                                                                                                                                                                                                               
function Base.setproperty!(x::Ptr{Other}, f::Symbol, v)                                                                                                                                                                                        
    unsafe_store!(_getptr(x, f), v)                                                                                                                                                                                                            
end                                                                                                                                                                                                                                            
                                                                                                                                                                                                                                               
                                                                                                                                                                                                                                               
mutable struct WithFields                                                                                                                                                                                                                      
    int_value::Cint                                                                                                                                                                                                                            
    int_ptr::Ptr{Cint}                                                                                                                                                                                                                         
    struct_value::Other                                                                                                                                                                                                                        
    struct_ptr::Ptr{Other}                                                                                                                                                                                                                     
    typedef_struct_value::TypedefStruct                                                                                                                                                                                                        
    array::NTuple{2, Cint}                                                                                                                                                                                                                     
end                                                                                                                                                                                                                                            
function _getptr(x::Ptr{WithFields}, f::Symbol)
    f === :int_value && return Ptr{Cint}(x + 0)
    f === :int_ptr && return Ptr{Ptr{Cint}}(x + 8)
    f === :struct_value && return Ptr{Other}(x + 16)
    f === :struct_ptr && return Ptr{Ptr{Other}}(x + 24)
    f === :typedef_struct_value && return Ptr{TypedefStruct}(x + 32)
    f === :array && return Ptr{NTuple{2, Cint}}(x + 36)
    error("Unrecognized field of type `WithFields`" * ": $(f)")
end

function Base.getproperty(x::Ptr{WithFields}, f::Symbol)
    f === :int_value && return unsafe_load(_getptr(x, f))
    f === :int_ptr && return unsafe_load(_getptr(x, f))
    f === :struct_value && return _getptr(x, f)
    f === :struct_ptr && return unsafe_load(_getptr(x, f))
    f === :typedef_struct_value && return _getptr(x, f)
    f === :array && return _getptr(x, f)
    return getfield(x, f)
end

function Base.setproperty!(x::Ptr{WithFields}, f::Symbol, v)
    unsafe_store!(_getptr(x, f), v)
end

I've also tested it with CImGui.jl: Gnimuc/CImGui.jl#131
I will admit the code is a bit hairy 🐙 Not sure if it's the cleanest implementation.

EDIT: marked as a draft because JET.jl found some issues with it.

[JamesWrigley]

The problem JET found was in how bitfields are handled, I believe that's fixed now. My approach was to:

1. Move the get/set code for bitfields into standalone functions in 129555d.
2. Have Base.getproperty(x::T, f) and Base.getproperty(x::Ptr{T}, f) both call those functions, so they should both behave the same.

One downside I noticed when playing around with the new wrappers in CImGui is that when we want to write to the struct we really do want to get a pointer instead of unsafe_load()'ing it automatically, e.g.:

# io.ConfigFlags is now a Cint, but we really want a Ptr{Cint}
CImGui.CheckboxFlags("io.ConfigFlags: NavEnableKeyboard", io.ConfigFlags, CImGui.ImGuiConfigFlags_NavEnableKeyboard)

The simple fix there would be to use _getptr(io, :ConfigFlags), but that's rather unsatisfying. So I had an evil thought 😛 What if we add a third parameter to our Base.getproperty() methods, like Base.getproperty(x::Ptr{T}, f::Symbol, as_ptr::Bool=false), and implement a @ptr macro that turns @ptr foo.bar into Base.getproperty(foo, :bar, true)? Then folks could write e.g. @ptr io.ConfigFlags whenever they want a pointer.

On a side note, turns out that all of our wrappers are fantastically inferrable so JET is very good at finding problems with them :)

[Gnimuc]

What's the behavior of loading a chain of pointers?

[JamesWrigley]

There's an example of that in the tests with @test obj_ptr.struct_value.i == obj.struct_value.i. The rule is that the dereferencing only occurs if the field type isn't a struct or array, so if there's a chain of nested structs Base.getproperty() will return a pointer to each of them just like the current behaviour. But if there's a non-struct/array type at the end of the chain, like an int, then that field will be dereferenced and Base.getproperty() on it will return a Cint.

[Gnimuc]

The rule is that the dereferencing only occurs if the field type isn't a struct or array,

I'm pretty sure this gonna confuse newbies, probably LLMs too...

[Gnimuc]

see https://giordano.github.io/blog/2019-05-03-julia-get-pointer-value/ for a hack of dereference operator in Julia.

[JamesWrigley]

I'm pretty sure this gonna confuse newbies, probably LLMs too...

I actually think it'll make it easier for newbies since they can blindly access nested properties and always get the right result. A good example of this is something like an ImVec2 field:

struct Foo
    vec::ImVec2
end

Currently to access .x and .y they would need to do:

vec = unsafe_load(obj.vec)
vec.x, vec.y

Whereas with this PR they can simply do obj.vec.x and obj.vec.y. In any case, the feature is off by default so folks will have to explicitly opt in to use it. And I confess I don't care very much about LLM's 😛

see https://giordano.github.io/blog/2019-05-03-julia-get-pointer-value/ for a hack of dereference operator in Julia.

That's interesting, but it still makes reading fields harder than writing to them. In my experience fields are read more often than written to, so I think it makes sense to sacrifice some write syntax for the sake of making reads simpler.

[Gnimuc]

hmm... I kinda agree, but if some packages adopt one rule but not the other, there could be a Vim-vs-Emacs war in the future... 😄 however, I don't think that's something we can control. and it's not easy to implement this feature out of the source tree, so it's ok to merge I guess.

[JamesWrigley]

I would actually like to make this the default at some point in the future 👀 But that would be wildly breaking so we should probably wait to see if people actually like it first.

But getting a pointer to a field is still a bit tricky :\ What do you think about this idea?

The simple fix there would be to use _getptr(io, :ConfigFlags), but that's rather unsatisfying. So I had an evil thought 😛 What if we add a third parameter to our Base.getproperty() methods, like Base.getproperty(x::Ptr{T}, f::Symbol, as_ptr::Bool=false), and implement a @ptr macro that turns @ptr foo.bar into Base.getproperty(foo, :bar, true)? Then folks could write e.g. @ptr io.ConfigFlags whenever they want a pointer.

[Gnimuc]

If it needs another macro anyway, I would rather integrate auto-dereference in the macro as well.

I implemented @c macro in https://github.com/Gnimuc/CSyntax.jl to mimic the & operator in C.

Maybe it can be extended to call some particular field access methods generated by Clang.jl. In this way, the field access behavior are kept as-is, and end users get a handy macro.

[Gnimuc]

the main point here is that we can turn @ptr foo.bar into any custom function call, there is not necessary to pirates Base.getproperty.

[JamesWrigley]

Yeah @c would actually be perfect 🤔 Unless I'm mistaken, I think we do need to use Base.getproperty() though? The reason is that some code living outside of the bindings has no other way to know which getptr() function to call. For example:

module MyPackage

module lib
include("../bindings.jl")
end

using CSyntax
x() = @c lib.foo().bar

Presumably the field access methods are in lib, but @c has no idea about that so it can't know it has to put in a call to lib.getptr instead of getptr. An alternative would be to define methods of functions defined in either Clang.jl or CSyntax.jl or CEnum.jl, but that would make either package a hard dependency.

[Gnimuc]

Presumably the field access methods are in lib, but @c has no idea about that so it can't know it has to put in a call to lib.getptr instead of getptr. An alternative would be to define methods of functions defined in either Clang.jl or CSyntax.jl or CEnum.jl, but that would make either package a hard dependency.

parentmodule(typeof(Foo())).getptr can do the trick.

[Gnimuc]

julia> module Foo
         struct Bar
            x::Int
         end
         getx(x::Bar) = "x"
         export Bar
       end
Main.Foo

julia> using .Foo

julia> @generated function getxxx(x::T) where {T}
         mod = parentmodule(T)
         :($mod.getx(x))
       end
getxxx (generic function with 1 method)

julia> macro c(x, ex...)
        :(getxxx($(esc(x))))
       end
@c (macro with 1 method)

julia> @c Bar(1)
"x"

[JamesWrigley]

Ah, nice 👍 And with luck that'll constant fold away.

[JamesWrigley]

I ended up implementing @ptr in the module itself in 89be1a6 because then we can just use @__MODULE__.

Latest codegen

using CEnum: CEnum, @cenum

macro ptr(expr)
    if !isa(expr, Expr) || expr.head != :.
        error("Expression is not a property access")
    end

    quote
        local penultimate_obj = $(esc(expr.args[1]))
        (@__MODULE__).getptr(penultimate_obj, $(esc(expr.args[2])))
    end
end

struct TypedefStruct
    i::Cint
end

struct Other
    i::Cint
end
function getptr(x::Ptr{Other}, f::Symbol)
    f === :i && return Ptr{Cint}(x + 0)
    error("Unrecognized field of type `Other`" * ": $(f)")
end

function Base.getproperty(x::Ptr{Other}, f::Symbol)
    f === :i && return unsafe_load(getptr(x, f))
    return getfield(x, f)
end

function Base.setproperty!(x::Ptr{Other}, f::Symbol, v)
    unsafe_store!(getptr(x, f), v)
end


mutable struct WithFields
    int_value::Cint
    int_ptr::Ptr{Cint}
    struct_value::Other
    struct_ptr::Ptr{Other}
    typedef_struct_value::TypedefStruct
    array::NTuple{2, Cint}
end
function getptr(x::Ptr{WithFields}, f::Symbol)
    f === :int_value && return Ptr{Cint}(x + 0)
    f === :int_ptr && return Ptr{Ptr{Cint}}(x + 8)
    f === :struct_value && return Ptr{Other}(x + 16)
    f === :struct_ptr && return Ptr{Ptr{Other}}(x + 24)
    f === :typedef_struct_value && return Ptr{TypedefStruct}(x + 32)
    f === :array && return Ptr{NTuple{2, Cint}}(x + 36)
    error("Unrecognized field of type `WithFields`" * ": $(f)")
end

function Base.getproperty(x::Ptr{WithFields}, f::Symbol)
    f === :int_value && return unsafe_load(getptr(x, f))
    f === :int_ptr && return unsafe_load(getptr(x, f))
    f === :struct_value && return getptr(x, f)
    f === :struct_ptr && return unsafe_load(getptr(x, f))
    f === :typedef_struct_value && return getptr(x, f)
    f === :array && return getptr(x, f)
    return getfield(x, f)
end

function Base.setproperty!(x::Ptr{WithFields}, f::Symbol, v)
    unsafe_store!(getptr(x, f), v)
end

I want to play around with it a bit more in CImGui.jl before merging, but what do you think?

[JamesWrigley]

Hmm, looks like nightly is broken:

• Because BinaryBuilderBase.jl is broken on nightly: https://github.com/JuliaPackaging/BinaryBuilderBase.jl/blob/406ec5c26348a7965cdc35aba15c03f84c02b525/src/BinaryBuilderBase.jl#L226
  • Because nightly just bumped Pkg: 🤖 [master] Bump the Pkg stdlib from 8c996799b to 046df8ce4 JuliaLang/julia#55112
    • And Pkg just changed an internal variable: Use historical stdlib dependency information JuliaLang/Pkg.jl#3911

[Gnimuc]

IIUC, with the new flag off, we can get the same output as before, right?

[JamesWrigley]

Almost the same, the only difference is that with 129555d the logic for bitfields will be in standalone getbitfieldproperty()/setbitfieldproperty!() functions instead of being inline with Base.getproperty()/Base.setproperty!(). But the behaviour should be identical 🤞 I already added a test for that in the bitfield tests in 2e7e00d, but I'll add some more to make it doesn't regress.

EDIT: added some tests for the default behaviour in 73ed7ff.

[Gnimuc]

I don't know how many people are using this package. But let’s take a poll and see if there are any unexpected problems.

This PR provides an alternative method for field access.

The default behavior is that accessing a field always returns a Ptr and one needs to call unsafe_load manually to load/copy the value.

The opt-in behavior provided by this PR is that:

1. Accessing a field will automatically call unsafe_load to load/copy the value if the field type isn't a struct or array.
2. When a pointer is needed, the macro @ptr needs to be called manually.
3. The rest of the behavior is the same as the default behavior.

Please leave a 👍🏻 or 👎🏻 reaction, and any feedback is appreciated.

[JamesWrigley]

Sounds good :) In the meantime I'll try fixing nightly.

[JamesWrigley]

Alrighty, now we're back in the green 🟢