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

Forward declaration of struct defined in dependency causes duplicate code generation #489

Open Octogonapus opened 2 months ago

Octogonapus commented 2 months ago

I have found that if the JLL I'm generating bindings for contains a forward declaration to a struct which is defined in one of its dependencies (which I include as system headers), a struct definition is still generated even though it should be skipped due to being in the system headers. Using Clang.jl v0.18.3 with generate_isystem_symbols = false.

I'm generating bindings for aws-c-http. This struct is forward declared here in aws-c-http. aws-c-http includes the following file from aws-c-io. This struct is forward declared here in aws-c-io. This struct is defined here in aws-c-io.

For now I can work around this using output_ignorelist.

The relevant part of my code is:

for target in JLLEnvs.JLL_ENV_TRIPLES
    if should_skip_target(target)
        continue
    end
    options = load_options(joinpath(@__DIR__, "generator.toml"))
    options["general"]["output_file_path"] = joinpath(@__DIR__, "..", "lib", "$target.jl")
    options["general"]["callback_documentation"] = get_docs

    args = get_default_args(target)
    inc = JLLEnvs.get_pkg_include_dir(aws_c_common_jll, target)
    push!(args, "-isystem$inc")
    inc = JLLEnvs.get_pkg_include_dir(aws_c_io_jll, target)
    push!(args, "-isystem$inc")
    inc = JLLEnvs.get_pkg_include_dir(aws_c_compression_jll, target)
    push!(args, "-isystem$inc")
    inc = JLLEnvs.get_pkg_include_dir(aws_c_cal_jll, target)
    push!(args, "-isystem$inc")

    header_dirs = []
    inc = JLLEnvs.get_pkg_include_dir(aws_c_http_jll, target)
    push!(args, "-I$inc")
    push!(header_dirs, inc)

    headers = String[]
    for header_dir in header_dirs
        for (root, dirs, files) in walkdir(header_dir)
            for file in files
                if endswith(file, ".h")
                    push!(headers, joinpath(root, file))
                end
            end
        end
    end
    unique!(headers)

    ctx = create_context(headers, args, options)
    build!(ctx)
end

I've done some debugging and found that aws_client_bootstrap is not in dag.sys, so it can't be ignored by generate_isystem_symbols. Also this struct is always parsed as a forward declaration:

┌ Info: collect_top_level_nodes!
│   ty = Clang.Generators.StructForwardDecl()
└   id = :aws_client_bootstrap
# ... two more times

Which is confusing because no code should be generated for those given:

skip_check(dag::ExprDAG, node::ExprNode{StructForwardDecl}) = true

So I must have missed something. That's as far as I got tonight.

Gnimuc commented 2 months ago

https://github.com/JuliaInterop/Clang.jl/blob/18f793b7f12ce2936f8a3da12e1b4c5b5a17db6b/src/generator/passes.jl#L185-L211

It was added from the FindOpaques pass.

The definition of an opaque type could be in the source file or its dependent "system" headers.

I'm wondering whether there is an efficient way to check that the definition of an opaque type is in one of the system headers.

Gnimuc commented 2 months ago

https://github.com/JuliaInterop/Clang.jl/blob/18f793b7f12ce2936f8a3da12e1b4c5b5a17db6b/src/generator/codegen.jl#L700-L718

We run this pass because we want to generate strong-typed code if opaque_as_mutable_struct is on.

Gnimuc commented 2 months ago

A rewriter can also be used to workaround such issues:

build!(ctx, BUILDSTAGE_NO_PRINTING)

function rewrite!(dag::ExprDAG)
    replace!(get_nodes(dag)) do node
        if node.id in # [exported symbol of xxx_jll]
            return ExprNode(node.id, Generators.Skip(), node.cursor, Expr[], node.adj)
        end
        return node
    end
end

rewrite!(ctx.dag)

build!(ctx, BUILDSTAGE_PRINTING_ONLY)
Octogonapus commented 2 months ago

I don't fully understand why this is not a bug, but I guess replacing those nodes works.

Gnimuc commented 2 months ago

It's a bug. I just can't come up with an elegant way to fix it...