Skip to content

cl: class ctor/dtor; cl Config: defaultNameLookup; wrapInlineFunc: write c func prototype - #720

Merged
xushiwei merged 5 commits into
goplus:devfrom
xushiwei:q
Sep 16, 2026
Merged

xushiwei merged 5 commits into
goplus:devfrom
xushiwei:q

Conversation

@xushiwei

Copy link
Copy Markdown
Member

No description provided.

@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: constructor/destructor naming + optional Config defaults

The change is small and well-scoped. Making addObject(name, decl) take the name explicitly cleanly separates naming policy (ctor→XGo_Ctor, dtor→XGo_Dtor, method→spelling) from overload bookkeeping, and the nil-safe Config / default NameLookup make the public NewPackage API more robust. I traced the new names through getPubNamecPubName: since XGo_Ctor/XGo_Dtor start with an uppercase X, they pass through unchanged, and the new ctor_dtor fixture correctly shows overload suffixes for the two constructors (__0/__1) while the single destructor gets none. Good end-to-end coverage.

No blocking issues from the performance or security passes: the default NameLookup returning ("", true) accepts all mangling names by design (linear in declaration count, no repeated work), and there is no new runtime trust boundary — inputs remain build-time headers/symbol names.

A few minor points are left inline. One pre-existing note not from this diff: the Fset comment (cl/compile.go) still says "Load will use a new fileset, but preserve Fset's value" — there is no Load function (the entry point is NewPackage), and conf.Fset is passed straight to gogen and never written back, so "preserve Fset's value" doesn't match the code. Worth fixing while touching these doc comments, though it predates this PR.

Comment thread cl/class.go
case lc.CursorCXXMethod, lc.CursorConstructor, lc.CursorDestructor:
obj := cls.addObject(decl)
var name string
switch decl.Kind {

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.

Minor readability: this inner switch decl.Kind lists default first, which is legal but unconventional. Since the outer switch already constrains the kind to method/constructor/destructor, default here really means "CXXMethod" — consider putting default last and labeling it, e.g.:

switch decl.Kind {
case lc.CursorConstructor:
	name = ctorName
case lc.CursorDestructor:
	name = dtorName
default: // lc.CursorCXXMethod
	name = clang.String(decl)
}

The "XGo_Ctor"/"XGo_Dtor" literals also effectively form part of the generated-code contract; promoting them to named constants would give a single source of truth.

Comment thread cl/compile_test.go
Comment thread cl/compile.go Outdated
@xushiwei xushiwei changed the title cl: class ctor/dtor; cl Config: defaultNameLookup cl: class ctor/dtor; cl Config: defaultNameLookup; cl wrapInlineFunc: write c func prototype Sep 16, 2026
@xushiwei xushiwei changed the title cl: class ctor/dtor; cl Config: defaultNameLookup; cl wrapInlineFunc: write c func prototype cl: class ctor/dtor; cl Config: defaultNameLookup; wrapInlineFunc: write c func prototype Sep 16, 2026
@xushiwei
xushiwei merged commit 826ae20 into goplus:dev Sep 16, 2026
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.

1 participant