`getproperty` for `PointerWrapper`s

[JoshuaLampert]

I have realized that some structs from p4est only have data as single fieldname and the access to the actual fields is defined by Base.getproperty, e.g. for p4est_quadrant. This currently makes it impossible to access the data for PointerWrapper-wrapped pointers by pw.fieldname[], for example:

julia> using P4est

julia> using MPI; MPI.Init()
MPI.ThreadLevel(2)

julia> connectivity = p4est_connectivity_new_periodic()
Ptr{p4est_connectivity} @0x0000000002fccd80

julia> p4est = p4est_new_ext(MPI.COMM_WORLD, connectivity, 0, 2, 0, 0, C_NULL, C_NULL)
Into p4est_new with min quadrants 0 level 2 uniform 0
New p4est with 1 trees on 1 processors
Initial level 2 potential global quadrants 16 per tree 16
Done p4est_new with 10 total quadrants
Ptr{P4est.LibP4est.p4est} @0x0000000002b57d80

julia> p4est_pw = PointerWrapper(p4est)
PointerWrapper{P4est.LibP4est.p4est}(Ptr{P4est.LibP4est.p4est} @0x0000000002b57d80)

julia> p4est_pw.global_first_position.level[] # This should work
ERROR: type p4est_quadrant has no field level
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] getproperty(pw::PointerWrapper{p4est_quadrant}, name::Symbol)
   @ P4est.PointerWrappers ~/.julia/packages/P4est/XU3GX/src/pointerwrappers.jl:61
 [3] top-level scope
   @ REPL[7]:1

julia> p4est_pw.global_first_position[].level # This is a way out, but shouldn't be necessary
29

I guess to cure this issue we need to adjust the getproperty function from PointerWrapper to not only look for fieldnames(T) (since e.g. fieldnames(p4est_quadrant) is (:data,) and not (:x, :y, :level, :pad8, :pad16, :p,)), but for the actual fields we can access via Base.getproperty. However, I'm not quite sure how exactly to do that. Any ideas on that or other possibilities to solve the problem, @ranocha, @sloede?

[JoshuaLampert]

Another option would be to overload propertynames for these structs as also recommended here. But this is a bit more cumbersome to do automatically, I guess (e.g. in fixes.sh)? Or we ask the people from Clang.jl to overload propertynames whenever getproperty is overloaded?

[sloede]

Do you know why p4est_quadrant was generated in such a weird way in the first place?

[JoshuaLampert]

No, I don't know. That's definitely also a good question.

[JoshuaLampert]

I saw that there is an option field_access_method_list (see here) in the generator.toml for Clang, but that is empty in our case...

[JoshuaLampert]

I created an MWE and I think C unions and C structs with a union as field create these weird julia structs with data as only field and the additional Base.getproperty method.

[ranocha]

It would indeed be nice to use propertynames here 👍

[ranocha]

Maybe you could check with the Clang.jl developers? In the meantime, we could add a draft for a struct where we would like to use this in Trixi.jl manually? Just to see whether we can go ahead with it?

[JoshuaLampert]

By

add a draft for a struct

you mean manually overloading propertynames for a struct in P4est.jl, e.g. for p4est_quadrant (and maybe others we need in Trixi.jl)?

[JoshuaLampert]

I have submitted an issue at Clang.jl (JuliaInterop/Clang.jl#426). Overloading propertynames wouldn't suffice for us, we also need fieldtype and fieldoffset here, which is a problem since fieldtype is a builtin function and therefore we cannot add a method to it.

[Gnimuc]

Overloading propertynames wouldn't suffice for us, we also need fieldtype and fieldoffset here, which is a problem since fieldtype is a builtin function and therefore we cannot add a method to it.

If you need custom behavior, you could write a custom generator with Clang.jl using a rewriter or adding new passes.

[Gnimuc]

Maybe you could check with the Clang.jl developers? In the meantime, we could add a draft for a struct where we would like to use this in Trixi.jl manually? Just to see whether we can go ahead with it?

yes. it would be great if you can provide a pattern for such structs. general support in Clang.jl is not easy. consider what it could be in the case of a nested anonymous union in a struct.

[Gnimuc]

just give a quick check on the source, so this might be wrong.

it looks like we can add a dispatch method to return the pointer(we don't need to do pointer arithmetics as Clang.jl already did it for us):

Base.getproperty(pw::PointerWrapper{p4est_quadrant}, name::Symbol) = PointerWrapper(Base.getproperty(pw.pointer, name))

[JoshuaLampert]

Thanks @Gnimuc for your comments!

Base.getproperty(pw::PointerWrapper{p4est_quadrant}, name::Symbol) = PointerWrapper(Base.getproperty(pw.pointer, name))

Indeed, this is a workaround, which works for p4est_quadrant, but would not be a general solution since for structs that don't overload Base.getproperty for pointers, e.g. p4est it wouldn't work. I've made a suggestion in #74.