genbindings: use separate virtbase helper to get base pointers

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
+	slist := make([]string, 0, len(ctor.Parameters))
-
+	for _, p := range ctor.Parameters {
-	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 = 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
+					"\treturn new " + cppClassName + "(" + forwarding + ");\n",
-					"\t*outptr_" + cabiClassName(c.ClassName) + " = ret;\n", // Original class name
 			)
-			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

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 + ` {				
+			func UnsafeNew` + goClassName + `(h unsafe.Pointer) *` + goClassName + ` {
-				if h == nil {
+				return new` + goClassName + `( (*C.` + goClassName + `)(h) )
-					return nil
-				}
-				
-				return &` + goClassName + `{` + unsafeInit + `}
 			}
 			
 		`)
@@ -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(`
+			ret.WriteString(`				
-				C.` + goClassName + `_new` + maybeSuffix(i) + `(` + forwarding + `)
+				ret := new` + goClassName + `(C.` + goClassName + `_new` + maybeSuffix(i) + `(` + forwarding + `))
-				ret := new` + goClassName + `(` + strings.Join(outptrs, `, `) + `)
 				ret.isSubclass = true
 				return ret
 			}