cl: template class with virtual methods - #872
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #872 +/- ##
==========================================
+ Coverage 85.12% 85.33% +0.21%
==========================================
Files 22 22
Lines 1802 1821 +19
==========================================
+ Hits 1534 1554 +20
+ Misses 268 267 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Review: template type-parameter support in vtable generation
Solid, well-scoped change. genVtable now threads the *classCtx through so a template class's type parameters flow into both the generated _xgo_vtable_X[...] struct and the XGo_vptr() receiver, and the out.go fixture confirms the intended output. Build passes.
One latent correctness issue is flagged inline. The remaining notes below are minor (naming/docs) and non-blocking.
Minor — naming & docs (cl/vtable.go)
cloneTypes(L148) doesn't clone — it repacks[]*types.TypeParaminto[]types.Typefortypes.Instantiate. A name liketypeParamsAsArgs/toTypeArgswould read truer. Same "clone" framing applies to the block generally.cloneTypeName(L165) reusesobj.Type()for the newTypeName's type, whereasnewTemplateParamsinclass.go(L90) passesnilwhen building the same shape. Worth aligning the two so the vtable's type params are guaranteed identical to the class's; if theobj.Type()binding is intentional, a one-line comment would help.- The three new helpers (
cloneTypes,cloneTypeParams,cloneTypeName, L148–167) lack doc comments in an otherwise thoroughly-documented file. The doc comments ongenVtable(L88) andgenVptrAccessor(L169) also still describe only the non-generic*_xgo_vtable_Xform; a brief note about the template ([T...]) path would keep them accurate.
No security or performance concerns: the unsafe.Pointer cast in classCtx.scope() and the deliberate panic invariants are pre-existing and untouched; new work scales with the small type-parameter count at codegen time.
No description provided.