From 5a7a46e28f8cd5ff20b149068301d76dd85d2cf0 Mon Sep 17 00:00:00 2001 From: mappu Date: Sat, 7 Dec 2024 17:15:42 +1300 Subject: [PATCH] genbindings: use separate virtbase helper to get base pointers --- cmd/genbindings/emitcabi.go | 70 ++++++++++++------------ cmd/genbindings/emitgo.go | 104 +++++++++++++----------------------- 2 files changed, 74 insertions(+), 100 deletions(-) diff --git a/cmd/genbindings/emitcabi.go b/cmd/genbindings/emitcabi.go index e718edcb..30bae65f 100644 --- a/cmd/genbindings/emitcabi.go +++ b/cmd/genbindings/emitcabi.go @@ -723,7 +723,17 @@ extern "C" { methodPrefixName := cabiClassName(c.ClassName) for i, ctor := range c.Ctors { - ret.WriteString(fmt.Sprintf("void %s_new%s(%s);\n", methodPrefixName, maybeSuffix(i), emitParametersCabiConstructor(&c, &ctor))) + ret.WriteString(fmt.Sprintf("%s* %s_new%s(%s);\n", methodPrefixName, methodPrefixName, maybeSuffix(i), emitParametersCabiConstructor(&c, &ctor))) + } + + if len(c.DirectInheritClassInfo()) > 0 { + ret.WriteString( + "void " + methodPrefixName + "_virtbase(" + methodPrefixName + "* src", + ) + for _, baseClass := range c.DirectInheritClassInfo() { + ret.WriteString(", " + cabiClassName(baseClass.Class.ClassName) + "** outptr_" + cabiClassName(baseClass.Class.ClassName)) + } + ret.WriteString(");\n") } for _, m := range c.Methods { @@ -765,25 +775,8 @@ func fullyQualifiedConstructor(className string) string { func emitParametersCabiConstructor(c *CppClass, ctor *CppMethod) string { - plist := slice_copy(ctor.Parameters) // semi-shallow copy - - plist = append(plist, CppParameter{ - ParameterName: cabiClassName("outptr_" + cabiClassName(c.ClassName)), - ParameterType: c.ClassName, - Pointer: true, - PointerCount: 2, - }) - for _, baseClass := range c.AllInherits() { - plist = append(plist, CppParameter{ - ParameterName: cabiClassName("outptr_" + cabiClassName(baseClass)), - ParameterType: baseClass, - Pointer: true, - PointerCount: 2, - }) - } - - slist := make([]string, 0, len(plist)) - for _, p := range plist { + slist := make([]string, 0, len(ctor.Parameters)) + for _, p := range ctor.Parameters { slist = append(slist, p.RenderTypeCabi()+" "+p.ParameterName) } @@ -969,34 +962,24 @@ func emitBindingCpp(src *CppParsedHeader, filename string) (string, error) { for i, ctor := range c.Ctors { - // The returned ctor needs to return a C++ pointer for not just the - // class itself, but also all of the inherited base classes - // That's because C++ virtual inheritance shifts the pointer; we - // need all the base pointers to call base methods from CGO - // Supply them all as out-parameters so we only need one roundtrip - preamble, forwarding := emitParametersCABI2CppForwarding(ctor.Parameters, "\t") ret.WriteString( - "void " + methodPrefixName + "_new" + maybeSuffix(i) + "(" + emitParametersCabiConstructor(&c, &ctor) + ") {\n", + cabiClassName(c.ClassName) + "* " + methodPrefixName + "_new" + maybeSuffix(i) + "(" + emitParametersCabiConstructor(&c, &ctor) + ") {\n", ) if ctor.LinuxOnly { ret.WriteString( "#ifndef Q_OS_LINUX\n" + - "\treturn;\n" + + "\treturn nullptr;\n" + "#else\n", ) } ret.WriteString( preamble + - "\t" + cppClassName + "* ret = new " + cppClassName + "(" + forwarding + ");\n" + // Subclass class name - "\t*outptr_" + cabiClassName(c.ClassName) + " = ret;\n", // Original class name + "\treturn new " + cppClassName + "(" + forwarding + ");\n", ) - for _, baseClass := range c.AllInherits() { - ret.WriteString("\t*outptr_" + cabiClassName(baseClass) + " = static_cast<" + baseClass + "*>(ret);\n") - } if ctor.LinuxOnly { ret.WriteString( @@ -1011,6 +994,27 @@ func emitBindingCpp(src *CppParsedHeader, filename string) (string, error) { } + // Add a helper method to retrieve base class pointers + // That's because C++ virtual inheritance shifts the pointer; we + // need the base pointers to call base methods from CGO + if len(c.DirectInheritClassInfo()) > 0 { + ret.WriteString( + "void " + methodPrefixName + "_virtbase(" + methodPrefixName + "* src", + ) + for _, baseClass := range c.DirectInheritClassInfo() { + ret.WriteString(", " + baseClass.Class.ClassName + "** outptr_" + cabiClassName(baseClass.Class.ClassName)) + } + ret.WriteString(") {\n") + for _, baseClass := range c.DirectInheritClassInfo() { + ret.WriteString("\t*outptr_" + cabiClassName(baseClass.Class.ClassName) + " = static_cast<" + baseClass.Class.ClassName + "*>(src);\n") + } + ret.WriteString( + "}\n" + + "\n", + ) + + } + for _, m := range c.Methods { // Protected virtual methods will be bound separately (the only diff --git a/cmd/genbindings/emitgo.go b/cmd/genbindings/emitgo.go index 09d335cc..5490e2aa 100644 --- a/cmd/genbindings/emitgo.go +++ b/cmd/genbindings/emitgo.go @@ -588,18 +588,14 @@ func (gfs *goFileState) emitCabiToGo(assignExpr string, rt CppParameter, rvalue gfs.imports[importPathForQtPackage(pkg.PackageName)] = struct{}{} } - // FIXME This needs to somehow figure out the real child pointers - extraConstructArgs := strings.Repeat(", nil", len(pkg.Class.AllInherits())) - // We can only reference the rvalue once, in case it is a complex // expression - var rvalue2 string if crossPackage == "" { - rvalue2 = "new" + cabiClassName(rt.ParameterType) + "(" + rvalue + extraConstructArgs + ")" + rvalue = "new" + cabiClassName(rt.ParameterType) + "(" + rvalue + ")" } else { gfs.imports["unsafe"] = struct{}{} - rvalue2 = crossPackage + "UnsafeNew" + cabiClassName(rt.ParameterType) + "(unsafe.Pointer(" + rvalue + ")" + extraConstructArgs + ")" + rvalue = crossPackage + "UnsafeNew" + cabiClassName(rt.ParameterType) + "(unsafe.Pointer(" + rvalue + "))" } if !(rt.Pointer || rt.ByRef) { @@ -608,7 +604,7 @@ func (gfs *goFileState) emitCabiToGo(assignExpr string, rt CppParameter, rvalue // To preserve Qt's approximate semantics, add a runtime // finalizer to automatically Delete once the type goes out // of Go scope - afterword += namePrefix + "_goptr := " + rvalue2 + "\n" + afterword += namePrefix + "_goptr := " + rvalue + "\n" afterword += namePrefix + "_goptr.GoGC() // Qt uses pass-by-value semantics for this type. Mimic with finalizer\n" // If this is a function return, we have converted value-returned Qt types to pointers @@ -623,7 +619,7 @@ func (gfs *goFileState) emitCabiToGo(assignExpr string, rt CppParameter, rvalue } else { // No need for temporary _goptr variable - afterword += assignExpr + "" + rvalue2 + "\n" + afterword += assignExpr + "" + rvalue + "\n" } return afterword @@ -788,57 +784,47 @@ import "C" gfs.imports["unsafe"] = struct{}{} localInit := "h: h" - unsafeInit := "h: (*C." + goClassName + ")(h)" - extraCArgs := "" - extraUnsafeArgs := "" - - // We require arguments for all inherits, but we only embed the direct inherits - // Any recursive inherits will be owned by the base - for _, base := range c.AllInherits() { - - extraCArgs += ", h_" + cabiClassName(base) + " *C." + cabiClassName(base) - extraUnsafeArgs += ", h_" + cabiClassName(base) + " unsafe.Pointer" - } - - for _, pkg := range c.DirectInheritClassInfo() { - ctorPrefix := "" - base := pkg.Class.ClassName - - constructRequiresParams := pkg.Class.AllInherits() - var ixxParams []string = make([]string, 0, len(constructRequiresParams)+1) - ixxParams = append(ixxParams, "h_"+cabiClassName(base)) - for _, grandchildInheritedClass := range constructRequiresParams { - ixxParams = append(ixxParams, "h_"+cabiClassName(grandchildInheritedClass)) - } - - if pkg.PackageName != gfs.currentPackageName { - ctorPrefix = path.Base(pkg.PackageName) + "." - - localInit += ",\n" + cabiClassName(base) + ": " + ctorPrefix + "UnsafeNew" + cabiClassName(base) + "(unsafe.Pointer(" + strings.Join(ixxParams, "), unsafe.Pointer(") + "))" - } else { - localInit += ",\n" + cabiClassName(base) + ": new" + cabiClassName(base) + "(" + strings.Join(ixxParams, ", ") + ")" - - } - - unsafeInit += ",\n" + cabiClassName(base) + ": " + ctorPrefix + "UnsafeNew" + cabiClassName(base) + "(" + strings.Join(ixxParams, ", ") + ")" - } ret.WriteString(` // new` + goClassName + ` constructs the type using only CGO pointers. - func new` + goClassName + `(h *C.` + goClassName + extraCArgs + `) *` + goClassName + ` { + func new` + goClassName + `(h *C.` + goClassName + `) *` + goClassName + ` { if h == nil { return nil } + `) + + if len(c.DirectInheritClassInfo()) > 0 { + xbaseParams := "" + for _, pkg := range c.DirectInheritClassInfo() { + + base := pkg.Class.ClassName + + // Make extra CGO call to get base pointers from C++ space + outptrVar := "outptr_" + cabiClassName(base) + ret.WriteString("var " + outptrVar + " *C." + cabiClassName(base) + " = nil\n") + xbaseParams += ", &" + outptrVar + + // Set up how we would pass the pointer to its own make function + if pkg.PackageName != gfs.currentPackageName { + localInit += ",\n" + cabiClassName(base) + ": " + path.Base(pkg.PackageName) + "." + "UnsafeNew" + cabiClassName(base) + "(unsafe.Pointer(" + outptrVar + "))" + } else { + localInit += ",\n" + cabiClassName(base) + ": new" + cabiClassName(base) + "(" + outptrVar + ")" + } + + } + + // Populate outptr pointers + ret.WriteString("C." + cabiClassName(c.ClassName) + "_virtbase(h" + xbaseParams + ")\n") + + } + + ret.WriteString(` return &` + goClassName + `{` + localInit + `} } // UnsafeNew` + goClassName + ` constructs the type using only unsafe pointers. - func UnsafeNew` + goClassName + `(h unsafe.Pointer` + extraUnsafeArgs + `) *` + goClassName + ` { - if h == nil { - return nil - } - - return &` + goClassName + `{` + unsafeInit + `} + func UnsafeNew` + goClassName + `(h unsafe.Pointer) *` + goClassName + ` { + return new` + goClassName + `( (*C.` + goClassName + `)(h) ) } `) @@ -865,26 +851,10 @@ import "C" ret.WriteString(preamble) - // Outptr management - - outptrs := make([]string, 0, len(c.AllInherits())+1) - ret.WriteString(`var outptr_` + cabiClassName(c.ClassName) + ` *C.` + goClassName + " = nil\n") - outptrs = append(outptrs, "outptr_"+cabiClassName(c.ClassName)) - for _, baseClass := range c.AllInherits() { - ret.WriteString(`var outptr_` + cabiClassName(baseClass) + ` *C.` + cabiClassName(baseClass) + " = nil\n") - outptrs = append(outptrs, "outptr_"+cabiClassName(baseClass)) - } - - if len(forwarding) > 0 { - forwarding += ", " - } - forwarding += "&" + strings.Join(outptrs, ", &") - // Call Cgo constructor - ret.WriteString(` - C.` + goClassName + `_new` + maybeSuffix(i) + `(` + forwarding + `) - ret := new` + goClassName + `(` + strings.Join(outptrs, `, `) + `) + ret.WriteString(` + ret := new` + goClassName + `(C.` + goClassName + `_new` + maybeSuffix(i) + `(` + forwarding + `)) ret.isSubclass = true return ret }