Skip to content

cl: redeclarable struct/class; type: reference type (T&); emitClass = newType + initClassType; nextAnonName (merge enum/struct/class/union) - #869

Merged
xushiwei merged 4 commits into
goplus:mainfrom
xushiwei:q
Sep 29, 2026

Conversation

@xushiwei

Copy link
Copy Markdown
Member

No description provided.

@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.43478% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.48%. Comparing base (73aed46) to head (20a5b8f).
⚠️ Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
cl/typedef.go 33.33% 4 Missing ⚠️
cl/type.go 0.00% 3 Missing ⚠️
cl/class.go 93.10% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #869      +/-   ##
==========================================
- Coverage   85.80%   85.48%   -0.33%     
==========================================
  Files          22       22              
  Lines        1691     1715      +24     
==========================================
+ Hits         1451     1466      +15     
- Misses        240      249       +9     
Flag Coverage Δ
llgo-tests 85.48% <80.43%> (-0.33%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@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: template/forward-declaration handling

Overall this is a focused, well-structured change. Skipping templated classes, deferring declaration-only classes via uninited, adding LValueReference -> pointer, and propagating featIgnored in loadTypedef all look correct for the common path, and the added comments on the existing noop/TODO branches are accurate.

The main thing worth a closer look before merge is the uninited map lifecycle (see inline): it is keyed by Go type name and emitClass is not guarded against being invoked more than once for the same name within a translation unit, so repeated forward declarations could leave a stale entry uninitialized or re-register an already-defined type. The remaining notes are minor documentation/maintainability items.

Comment thread cl/type.go
Comment thread cl/ctx.go Outdated
Comment thread tool/gen_test.go Outdated

@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 (follow-up): uninited lifecycle finding

Follow-up to add the primary finding, which was omitted from the first review submission. See the inline comment.

Comment thread cl/class.go Outdated
@xushiwei xushiwei changed the title cl: redeclarable struct/class; type: reference type (T&); tool test: TestLLVM_DenseMapInfo; llcppg-system.cfg add MacroPrefix, FuncPrefix cl: redeclarable struct/class; type: reference type (T&); emitClass = newType + initClassType; nextAnonName (merge enum/struct/class/union) Sep 29, 2026
@xushiwei
xushiwei merged commit f985852 into goplus:main Sep 29, 2026
2 of 4 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.

1 participant