JuliaInterop / Clang.jl

C binding generator and Julia interface to libclang
https://juliainterop.github.io/Clang.jl/
MIT License
217 stars 68 forks source link

Avoid method ambiguities in TypedefMutualRef's #458

Closed JamesWrigley closed 7 months ago

JamesWrigley commented 7 months ago

emit!() for TypedefMutualRef will create some specializations of Base.unsafe_convert(), and previously they didn't declare a type for the Ref/Ptr value being passed to them, which would cause a method ambiguity with Base (detected by Aqua.jl).

The problem is that the typedef will of course be declared in the C header before the struct it references, so the Base.unsafe_convert() specializations couldn't use the actual struct type in their signature because it won't have been created yet. This commit fixes that with the notion of a partially emitted node that contains premature exprs that haven't been emitted yet. Now emit!() for TypedefMutualRef will record the Base.unsafe_convert() methods as premature exprs and mark itself as a partially emitted node. Then later, emit!() for the StructDefinition will look for any partially emitted nodes that are marked for it and emit their premature expressions itself.

Before:

mutable struct __JL_foo_struct
end

function Base.unsafe_load(x::Ptr{__JL_foo_struct})
    unsafe_load(Ptr{foo_struct}(x))
end

function Base.getproperty(x::Ptr{__JL_foo_struct}, f::Symbol)
    getproperty(Ptr{foo_struct}(x), f)
end

function Base.setproperty!(x::Ptr{__JL_foo_struct}, f::Symbol, v)
    setproperty!(Ptr{foo_struct}(x), f, v)
end

Base.unsafe_convert(::Type{Ptr{__JL_foo_struct}}, x::Ref) = Base.unsafe_convert(Ptr{__JL_foo_struct}, Base.unsafe_convert(Ptr{foo_struct}, x))

Base.unsafe_convert(::Type{Ptr{__JL_foo_struct}}, x::Ptr) = Ptr{__JL_foo_struct}(x)

const foo = Ptr{__JL_foo_struct}

struct foo_struct
    bar::foo
end

After:

mutable struct __JL_foo_struct
end

function Base.unsafe_load(x::Ptr{__JL_foo_struct})
    unsafe_load(Ptr{foo_struct}(x))
end

function Base.getproperty(x::Ptr{__JL_foo_struct}, f::Symbol)
    getproperty(Ptr{foo_struct}(x), f)
end

function Base.setproperty!(x::Ptr{__JL_foo_struct}, f::Symbol, v)
    setproperty!(Ptr{foo_struct}(x), f, v)
end

const foo = Ptr{__JL_foo_struct}

struct foo_struct
    bar::foo
end
Base.unsafe_convert(::Type{Ptr{__JL_foo_struct}}, x::Ref{foo_struct}) = Base.unsafe_convert(Ptr{__JL_foo_struct}, Base.unsafe_convert(Ptr{foo_struct}, x))

Base.unsafe_convert(::Type{Ptr{__JL_foo_struct}}, x::Ptr{foo_struct}) = Ptr{__JL_foo_struct}(x)

I'm not sure why there's no newline after the struct definition :thinking:

JamesWrigley commented 7 months ago

I'll try fixing the docs, but I have no idea what's going on with the Windows CI :eyes: AFAICT it seems unrelated?

JamesWrigley commented 7 months ago

Well, that was a bit of a rabbit hole :upside_down_face: TL;DR:

JamesWrigley commented 7 months ago

Hmph, now I know why the windows tests are failing... 7zip was recently moved to a different directory, which is used by BinDeps.jl, which is used by CMake.jl, which is used by our tests: https://github.com/JuliaLang/julia/pull/48931 That was backported to 1.9, which is why the Julia 1 tests are failing too and not just nightly. Relevant issue: https://github.com/JuliaPackaging/CMake.jl/issues/23

JamesWrigley commented 7 months ago

Ok, got there in the end :tada: Main fix was switching to CMake_jll in 31876f0, and then some other minor fixes in 3afee34 and 61f1c48.

Gnimuc commented 7 months ago

Thanks for fixing this! I'll give it a test and update the test-suite: https://github.com/Gnimuc/GeneratorScripts