fix(tool): preserve Python header documentation - #928
Conversation
Co-authored-by: xushiwei <396972+xushiwei@users.noreply.github.com>
Co-authored-by: xushiwei <396972+xushiwei@users.noreply.github.com>
Co-authored-by: xushiwei <396972+xushiwei@users.noreply.github.com>
|
@copilot The |
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: fix(tool): preserve Python header documentation
The change is small, well-scoped, and backward compatible. Adding an opt-in KeepDoc config that threads -fparse-all-comments into clang is the right mechanism for surfacing ordinary C header comments, and refactoring ParseSources into a public wrapper + private parseSources(..., keepDoc) preserves the existing exported signature — good call. The fixture llcppg.cfg plus regenerated goldens give regression coverage.
One substantive question and a couple of doc nits below. No blocking issues.
Design note worth confirming (not necessarily a bug): cfg.KeepDoc is threaded only into the clang parse flag; NewPackage never sets cl.Config.DontKeepDoc, so cl's internal keepDoc is always true in the tool path (comment emission is always on). In contrast, the test harness ties the two together via DontKeepDoc: !conf.KeepDoc (cl/compile_test.go:111). So in the tool path, KeepDoc=false still emits Doxygen-style doc comments and only suppresses ordinary comment parsing — which may well be intended, but the asymmetry with the cltest path is easy to trip over. See the inline note on tool/gen.go.
Co-authored-by: xushiwei <396972+xushiwei@users.noreply.github.com>
Removed |
PR goplus#928 enabled `-fparse-all-comments` unconditionally so that Python header documentation is preserved. That also surfaced formatting bugs in `toLineComments` for C/C++ comments, which produced garbled output and made TestClang fail. The marker stripping only handled a single, well-formed block. It left artifacts when: - a block opened with more than two asterisks ("/***" -> a stray "// *" line), as seen on clang_isPreprocessing / clang_isUnexposed; - a block closed with extra asterisks or was a banner of asterisks ("**/", "/*****...*****/"); - a raw comment concatenated several blocks (e.g. "/* Declarations */\n/** ... */", which libclang reports when a plain comment immediately precedes a doc comment on the same declaration), leaving interior "*/" and "/**" markers in the output. Rewrite the stripping to clean each physical line independently via a new `cleanCommentLine` helper that removes leading/trailing block markers (slash + run of "*"), line-comment markers ("//", "///", "//!") and a single Doxygen "*" decoration, wherever they appear. This cleans every case without changing any already-correctly-formatted comment (verified exhaustively against the clang-c and Python fixture headers). Update the two affected clang-c golden files (the stray "// *" lines from the "/***" openers) and add unit tests for `toLineComments`. The darwin-only gates in tool/gen_test.go are kept: the clang-c goldens remain platform-divergent (Apple Blocks typedefs and libclang-version comment association differ off-Apple), so enabling non-macOS would need separate goldens; the formatting fix itself is platform-independent.
TestPythongenerated bindings without comments from declarations in Python headers. Clang requires-fparse-all-commentsto expose these ordinary C comments.KeepDocsupport to tool configuration; when enabled, parse headers with-fparse-all-comments.KeepDocand update generated goldens with the extracted comments.{ "KeepDoc": true }