Skip to content

fix(tool): preserve Python header documentation - #928

Merged
xushiwei merged 5 commits into
mainfrom
copilot/fix-tool-testpython-bugfix-keep-doc
Oct 5, 2026
Merged

xushiwei merged 5 commits into
mainfrom
copilot/fix-tool-testpython-bugfix-keep-doc

Conversation

Copilot AI commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

TestPython generated bindings without comments from declarations in Python headers. Clang requires -fparse-all-comments to expose these ordinary C comments.

  • Generation: Add KeepDoc support to tool configuration; when enabled, parse headers with -fparse-all-comments.
  • Python fixture: Enable KeepDoc and update generated goldens with the extracted comments.
{
  "KeepDoc": true
}

Copilot AI linked an issue Oct 5, 2026 that may be closed by this pull request
Copilot AI and others added 3 commits October 5, 2026 10:50
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 AI changed the title [WIP] Fix TestPython bug to extract documentation from .h file fix(tool): preserve Python header documentation Oct 5, 2026
Copilot AI requested a review from xushiwei October 5, 2026 10:54
@xushiwei

xushiwei commented Oct 5, 2026

Copy link
Copy Markdown
Member

@copilot The tool package does not offer an option to disable doc extraction, so there is no need to add the KeepDoc configuration item.

@xushiwei
xushiwei marked this pull request as ready for review October 5, 2026 10:54

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread tool/gen.go Outdated
Comment thread tool/config.go Outdated
Co-authored-by: xushiwei <396972+xushiwei@users.noreply.github.com>

Copilot AI commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

@copilot The tool package does not offer an option to disable doc extraction, so there is no need to add the KeepDoc configuration item.

Removed KeepDoc and made -fparse-all-comments unconditional in tool parsing. The Python fixture and llgo test ./tool/... pass. Commit: d673211.

@xushiwei
xushiwei merged commit 5dfd4cb into main Oct 5, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(tool): TestPython bugfix - keep doc

2 participants