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

Documentation references are generated for symbols which are not documented #478

Closed Octogonapus closed 3 months ago

Octogonapus commented 3 months ago

Clang.jl generates doc refs which reference symbols that aren't documented. This causes cross reference errors when using Documenter.jl. Ideally, Clang.jl would understand when a symbol is documented, and link to it only if it is documented.

I wrote a test case you can use to check this:

@testset "generating docs from doxygen comments with a missing reference omits that reference" begin
    mktemp() do path, io
        # Generate the bindings
        options = Dict("general" => Dict{String, Any}("output_file_path" => path,
                                                      "extract_c_comment_style" => "doxygen"))
        args = get_default_args()
        push!(args, "-fparse-all-comments")
        ctx = create_context([joinpath(@__DIR__, "include/documentation_missing_ref.h")], args, options)
        build!(ctx)

        # Load into a temporary module to avoid polluting the global namespace
        m = Module()
        Base.include(m, path)
        println(read(path, String))

        # Do some sanity checks on the docstring
        docstring = string(@doc m.foo)
        docstring_has = occursin(docstring)
        @show docstring # TODO assert there is no doc ref to SOME_CONST
    end
end

which needs this file documentation_missing_ref.h:

#define SOME_CONST 1

/**
 * uses SOME_CONST.
 */
void foo() {
}

So that it's clear, the problem is that this code is generated:

"""
    foo()

uses [`SOME_CONST`](@ref).
"""
function foo()
    ccall((:foo, libxxx), Cvoid, ())
end

You can see it includes a reference to SOME_CONST, but that symbol is not documented.

I'm just not sure how to change the implementation to fix this. Maybe another dict is needed for format_inline to track which symbols have been documented? But then you will have an ordering problem.

JamesWrigley commented 3 months ago

I would suggest using the documentation callback feature to explicitly add docstrings, there's an example of using it for that in the Clang.jl generator: https://github.com/JuliaInterop/Clang.jl/blob/master/gen/generator.jl#L43

Octogonapus commented 3 months ago

I see, thanks.

Octogonapus commented 3 months ago

Actually it turns out there are some nodes which don't have code generated but are still cross-referenced. Here is an example node:

node = ExprNode{Clang.Generators.MacroFunctionLike, Clang.CLMacroDefinition}(:AWS_STATIC_STRING_FROM_LITERAL, Clang.Generators.MacroFunctionLike(), CLCursor (Clang.CLMacroDefinition) AWS_STATIC_STRING_FROM_LITERAL, Expr[], Expr[], Int64[])

But it's referenced by other generated docs:

"""
    aws_string_destroy_secure(str)

Zeroes out the data bytes of string and then deallocates the memory. Not safe to run on a string created with [`AWS_STATIC_STRING_FROM_LITERAL`](@ref).

### Prototype

void aws_string_destroy_secure(struct aws_string *str);

"""
function aws_string_destroy_secure(str)
    ccall((:aws_string_destroy_secure, libaws_c_common), Cvoid, (Ptr{aws_string},), str)
end

With macro_mode = "basic" there is no code generated. So I tried setting macro_mode = "aggressive" but I get an error:

[ Info: Emit Julia expressions...
ERROR: LoadError: ParseError:
# Error @ none:1:56
RETURN_ARG_COUNT((_1_),(_2_),(_3_),(_4_),(_5_),(count),...)
#                                                      └─┘ ── invalid identifier
Stacktrace:
 [1] #parse#3
   @ ./meta.jl:244 [inlined]
 [2] parse
   @ ./meta.jl:236 [inlined]
 [3] parse(str::String; filename::String, raise::Bool, depwarn::Bool)
   @ Base.Meta ./meta.jl:278
 [4] parse(str::String)
   @ Base.Meta ./meta.jl:276
 [5] macro_emit!(dag::ExprDAG, node::ExprNode{Clang.Generators.MacroFunctionLike, Clang.CLMacroDefinition}, options::Dict{String, Any})
   @ Clang.Generators ~/.julia/packages/Clang/kNxXb/src/generator/macro.jl:381
 [6] (::CodegenMacro)(dag::ExprDAG, options::Dict{String, Any})
   @ Clang.Generators ~/.julia/packages/Clang/kNxXb/src/generator/passes.jl:1198
 [7] build!(ctx::Context, stage::Clang.Generators.BuildStage)
   @ Clang.Generators ~/.julia/packages/Clang/kNxXb/src/generator/context.jl:176
 [8] top-level scope
   @ ~/Documents/code/LibAwsCommon.jl/gen/generator.jl:83
in expression starting at /home/salmon/Documents/code/LibAwsCommon.jl/gen/generator.jl:52

I don't really want to generate code for this macro, either. I just want to avoid the dangling cross-reference.

For context I am working on this PR https://github.com/JuliaServices/LibAwsCommon.jl/pull/8

Do you have any advice for this case?

JamesWrigley commented 3 months ago

I see :thinking: I think the simplest option would be to use latest master of Clang.jl which will call get_docs() on all nodes (like in the Clang.jl generator), and then do a replace() on the missing symbols in the doc argument that are referenced to turn e.g. [`AWS_STATIC_STRING_FROM_LITERAL`](@ref) into `AWS_STATIC_STRING_FROM_LITERAL`.

JamesWrigley commented 3 months ago

@Gnimuc, maybe it's worth make a new release? That would also add support for Julia 1.11. If so then I can make a PR to update the changelog beforehand.

Gnimuc commented 3 months ago

The latest release has some JLL-platform issues on Windows. I'm considering dropping 1.10- in Clang.jl-v0.18.

JamesWrigley commented 3 months ago

You mean all releases from 1.10 onwards, including 1.11? That would be kinda unfortunate :sweat_smile: What're the JLL issues?

Gnimuc commented 3 months ago

https://github.com/JuliaInterop/Clang.jl/blob/master/Artifacts.toml

This file is copied from BBB upstream and is somewhat outdated. The hash of certain Windows platform tarballs doesn't match anymore.

Gnimuc commented 3 months ago

however, the mismatch issue doesn't happen on CI... I did reproduce it on a clean Windows machine.

Gnimuc commented 3 months ago

anything else need to be done here?

Octogonapus commented 3 months ago

I will try doing https://github.com/JuliaInterop/Clang.jl/issues/478#issuecomment-2041042005

Octogonapus commented 3 months ago

That is working for me. Thanks!