cl: class ctor/dtor; cl Config: defaultNameLookup; wrapInlineFunc: write c func prototype - #720
Conversation
There was a problem hiding this comment.
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 getPubName→cPubName: 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.
| case lc.CursorCXXMethod, lc.CursorConstructor, lc.CursorDestructor: | ||
| obj := cls.addObject(decl) | ||
| var name string | ||
| switch decl.Kind { |
There was a problem hiding this comment.
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.
No description provided.