cmd: llcppdump - #696
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: Add VisitChildren / Cursor wrapper API
This is a clean, idiomatic addition to the high-level clang wrapper package. The new TranslationUnit.Cursor() method, the Cursor / ChildVisitResult type aliases, the Break / Continue / Recurse constants, and VisitChildren all match the existing conventions of the file.
Confirmed correct:
- The closure plumbing in
VisitChildrenis correct and memory-safe:c.ClosureData(fn)boxes the Go func into a stable pointer passed asClientData, and the C-ABI trampoline unboxes it viac.GoClosure[...]with the matching generic type and(cur, parent)order. Sinceclang_visitChildrenis synchronous andfnstays reachable for the whole call, there is no GC/use-after-free window. - The
Break/Continue/Recurseconstants correctly alias the underlyingclang.ChildVisit_*values. - The
go.modbump tolib v0.5.2is consistent with the use of theGoClosure/ClosureDatahelpers.
The pure-Go go build relocation failures for C symbols are expected for this LLGo binding project and are not a defect of this PR.
Findings below are minor / non-blocking.
| idx := clang.CreateIndex(0, 0) | ||
| defer idx.Dispose() | ||
|
|
||
| u := idx.ParseTranslationUnit(0, "") |
There was a problem hiding this comment.
[P2] Demo parses an empty filename, so it never exercises the new API
idx.ParseTranslationUnit(0, "") passes a non-NULL pointer to an empty C string with no command-line args. libclang treats a NULL source_filename as "the file is given in the args", but an empty-but-non-NULL string instead asks libclang to open a file literally named "", which fails and returns a NULL translation unit. As a result u.Cursor() yields a null cursor and dump() visits nothing, so this demo doesn't actually demonstrate VisitChildren.
Consider parsing a real source file (or taking a path from os.Args) and checking the returned unit before traversing. Since dump's visitor also just returns clang.Continue, printing something per node (e.g. cursor kind/spelling) would make it a meaningful "dump" demo.
|
|
||
| /** | ||
| * CreateIndex provides a shared context for creating translation units. | ||
| * Provides a shared context for creating translation units. |
There was a problem hiding this comment.
[P3] CreateIndex doc comment no longer starts with the identifier name
Changing CreateIndex provides a shared context... to Provides a shared context... means the doc comment for the exported CreateIndex function no longer begins with the identifier name, so go doc CreateIndex renders "Provides a shared context..." and loses the Go doc convention.
Note this is a style tradeoff: the rest of this file uses doxygen /** */ blocks copied from the clang headers that also don't start with the identifier name, so this change is arguably making CreateIndex consistent with the surrounding style. Non-blocking — flag only if Go-doc conformance is preferred over header-comment fidelity.
1 participant
No description provided.