Skip to content

Conversation

@xgopilot
Copy link
Contributor

@xgopilot xgopilot bot commented Feb 11, 2026

Requested by @luoliwoshang

Closes #618

Summary

  • Fixed ParseComment() in _xtool/internal/parser/parser.go to keep /* */ block comments as single ast.Comment nodes instead of incorrectly splitting by newline
  • Fixed NewCommentGroupFromC() in cl/internal/convert/comments.go to reassemble legacy split block comments from existing llcppsigfetch builds into single Go ast.Comment nodes
  • Updated github.com/goplus/gogen from v1.19.7 to v1.20.2
  • Also fixed shadowed variables and a typo in ParseCommentGroup() named return parameter
  • Added comprehensive table-driven unit tests for both NewCommentGroupFromC and gogen integration

Root Cause

Go's ast.Comment specification requires each /* */ block comment to be a single Comment node. The old ParseComment() function split all comments by newline, creating multiple nodes for block comments. This violated the spec and produced invalid Go code with gogen v1.20.2's stricter printer.

Changes

File Change
_xtool/internal/parser/parser.go Fix ParseComment() to distinguish block vs line comments; clean up ParseCommentGroup()
cl/internal/convert/comments.go Fix NewCommentGroupFromC() to reassemble split block comments for backward compatibility
cl/internal/convert/comments_test.go Add table-driven tests for comment conversion and gogen integration
go.mod / go.sum Update gogen v1.19.7 → v1.20.2

Test Plan

  • All NewCommentGroupFromC unit tests pass (9 cases: nil, empty, single block, multi-line block, legacy 2-node split, legacy 3-node split, javadoc style, single line, multiple lines)
  • Gogen integration tests confirm valid Go code generation for all block comment variants
  • Unit tests pass: config, internal/name, internal/arg, internal/unmarshal
  • All cl/internal/convert tests pass (excluding TestFromTestdata which requires llcppsigfetch)
  • go build ./... passes
  • go vet ./... passes
  • Full TestFromTestdata tests (requires llcppsigfetch rebuild with LLGo - CI will validate)

Fix ParseComment() to keep block comments (/* ... */) as single
ast.Comment nodes instead of splitting them by newline. Fix
NewCommentGroupFromC() to reassemble legacy split block comments
into single Go ast.Comment nodes.

This resolves invalid Go code generation exposed by gogen v1.20.2's
stricter comment handling.

Closes #618

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: luoliwoshang <51194195+luoliwoshang@users.noreply.github.com>
@luoliwoshang
Copy link
Member

@xgopilot

ci: test (ubuntu-latest, 19, v0.12.0, 1.23) got fail,check this problem

panic: failed generated package demos: _llcppgtest/cjson,_llcppgtest/lua

see https://github.com/goplus/llcppg/actions/runs/21896344268/artifacts/5461453809

@xgopilot
Copy link
Contributor Author

xgopilot bot commented Feb 11, 2026

CI Failure Fix

Root Cause: Downloaded the CI artifact logs and found the actual error:

cjson/cJSON.go:33:50: misplaced compiler directive
cjson/cJSON.go:41:86: misplaced compiler directive
... (10+ occurrences for both cjson and lua)
cannot build SSA for package cjson

When a C function has a block comment doc (/* ... */), NewCommentGroupFromC correctly reassembles it as a single ast.Comment node. Then package.go:200 appends the //go:linkname directive to the same comment group. gogen v1.20.2's printer outputs them adjacently on the same line:

/* Create an iterator *///go:linkname Foo C.Foo
func Foo()

While go parser accepts this syntax, the Go/LLGo compiler rejects //go:linkname as a "misplaced compiler directive" because it must be on its own line.

Fix (f4bf522): Insert an empty // line comment between block comment docs and //go:linkname (or // llgo:link) directives. This forces the directive onto its own line:

/* Create an iterator */ //
//go:linkname Foo C.Foo
func Foo()

Changes:

File Change
cl/internal/convert/comments.go Added hasBlockComment() helper
cl/internal/convert/package.go Insert // separator before linkname when doc has block comments
cl/internal/convert/comments_test.go Added TestBlockCommentWithLinkname (3 cases: single-line, multi-line, javadoc)

Verification:

  • All unit tests pass (config, internal/name, internal/arg, internal/unmarshal)
  • All comment tests pass (12 NewCommentGroupFromC + 4 gogen integration + 3 block+linkname)
  • go build ./... passes
  • go vet ./... passes
  • go fmt ./... clean

When a C function has a block comment doc (/* ... */), gogen v1.20.2
outputs the block comment and the following //go:linkname directive
on the same line (e.g., `*///go:linkname Foo C.Foo`). The Go/LLGo
compiler rejects this as "misplaced compiler directive" because
//go:linkname must be on its own line.

Fix: insert an empty // line comment between block comment docs and
//go:linkname (or // llgo:link) directives to ensure the directive
gets its own line in the generated output.

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: luoliwoshang <51194195+luoliwoshang@users.noreply.github.com>
@luoliwoshang
Copy link
Member

@xgopilot is the TestParserCMode and TestParserCppMode 's output change expect? if is expect update the expect result!

@xgopilot
Copy link
Contributor Author

xgopilot bot commented Feb 11, 2026

Updated Parser Test Expected Results

Yes, the TestParserCMode and TestParserCppMode output changes are expected due to the ParseComment() fix. I've updated the expect.json files in commit a30a996.

3 files updated:

File Changes
testdata/comment/expect.json Block comments → single nodes (no trailing \n); line comments → no trailing \n; consecutive block comments merged into single node
testdata/forwarddecl1/expect.json 2 block comments and 2 line comments → removed trailing \n
testdata/typeof/expect.json 1 line comment → removed trailing \n

Key format changes:

  • Block comments (/* ... */): Multi-line blocks that were split into multiple nodes are now a single node with the full text (no trailing \n)
  • Line comments (// ...): Trailing \n removed, separate nodes per line preserved
  • Consecutive block comments (e.g., /*! doc 1 */\n/*! doc 2 */): Merged into single node since clang provides them as one raw string

Verification: go build ./..., go vet ./..., and all unit tests pass.

Note: The actual TestParserCMode/TestParserCppMode tests require LLGo (llgo test) to run since they use clang bindings. CI will validate these expected outputs match the actual parser output.

Update expect.json files to match the new ParseComment() behavior:
- Block comments (/* ... */) are now kept as single ast.Comment nodes
  instead of being split by newline
- Line comments (// ...) no longer have trailing \n
- Consecutive block comments are merged into a single node

Files updated:
- testdata/comment/expect.json
- testdata/forwarddecl1/expect.json
- testdata/typeof/expect.json

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: luoliwoshang <51194195+luoliwoshang@users.noreply.github.com>
@luoliwoshang
Copy link
Member

@xgopilot The TestFromTestdata need update also

@xgopilot
Copy link
Contributor Author

xgopilot bot commented Feb 11, 2026

Updated TestFromTestdata expected results

Regenerated gogensig.expect files for 5 test cases affected by the new block comment handling:

File Change
gettext/gogensig.expect Block comment separator (//) added before //go:linkname
gpgerror/gogensig.expect Block comment separators added before // llgo:link and //go:linkname directives
issue507/gogensig.expect Single-line /** ... */ block comments now inline with type declarations (as formatted by go fmt)
keepcomment/gogensig.expect Block comments properly reassembled (/**\n...*/ instead of split nodes); separators added before compiler directives
xml2/gogensig.expect Block comment separators added before //go:linkname and // llgo:link directives

Verification:

  • go build ./... - pass
  • go vet ./... - pass
  • Unit tests (config, internal/name, internal/arg, internal/unmarshal) - pass
  • TestFromTestdata (all 27 subtests) - pass
  • All cl/internal/convert tests - pass

Commit: 3f8c2dd pushed to branch xgopilot/issue-618-1770793666.

Regenerate gogensig.expect files to match the new block comment
handling in NewCommentGroupFromC and gogen v1.20.2's printer output.

Affected test cases: gettext, gpgerror, issue507, keepcomment, xml2.

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: luoliwoshang <51194195+luoliwoshang@users.noreply.github.com>
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.

Bug: gogen v1.20.2 update exposes comment parsing issue causing test failures

2 participants