genbindings: improvements for miqt_string handling

cmd/genbindings/emitcabi.go

@@ -75,7 +75,7 @@ func (p CppParameter) RenderTypeCabi() string {
 
 func emitReturnTypeCabi(p CppParameter) string {
 	if p.ParameterType == "QString" {
-		return "void" // Will be handled separately
+		return "struct miqt_string*"
 
 	} else if _, ok := p.QListOf(); ok {
 		return "void" // Will be handled separately
@@ -142,9 +142,7 @@ func emitParametersCabi(m CppMethod, selfType string) string {
 
 	for _, p := range m.Parameters {
 		if p.ParameterType == "QString" {
-			// The Go code has called this with two arguments: char* and len
+			tmp = append(tmp, "struct miqt_string* "+p.ParameterName)
-			// Declare that we take two parameters
-			tmp = append(tmp, "const char* "+p.ParameterName+", size_t "+p.ParameterName+"_Strlen")
 
 		} else if t, ok := p.QListOf(); ok {
 
@@ -188,11 +186,7 @@ func emitParametersCabi(m CppMethod, selfType string) string {
 	// Qt C++: memory is in QString RAII
 	// CABI: memory is moved into C.malloc/C.free
 	// Go: converted to native Go string
-	if m.ReturnType.ParameterType == "QString" {
+	if t, ok := m.ReturnType.QListOf(); ok {
-		// Normally we would use size_t for a strlen, but Go calls C.GoStringN which takes a C.int
-		tmp = append(tmp, "char** _out, int* _out_Strlen")
-
-	} else if t, ok := m.ReturnType.QListOf(); ok {
 		// +1 pointer indirection since it's a heap array
 		// +1 pointer indirection for mutating remote parameter
 		// = 3 for char*, 2 for most types
@@ -222,7 +216,7 @@ func emitParametersCABI2CppForwarding(params []CppParameter) (preamble string, f
 		if p.ParameterType == "QString" {
 			// The CABI has accepted two parameters - need to convert to one real QString
 			// Create it on the stack
-			preamble += "\tQString " + p.ParameterName + "_QString = QString::fromUtf8(" + p.ParameterName + ", " + p.ParameterName + "_Strlen);\n"
+			preamble += "\tQString " + p.ParameterName + "_QString = QString::fromUtf8(&" + p.ParameterName + "->data, " + p.ParameterName + "->len);\n"
 			tmp = append(tmp, p.ParameterName+"_QString")
 
 		} else if listType, ok := p.QListOf(); ok {
@@ -327,6 +321,8 @@ func emitAssignCppToCabi(assignExpression string, p CppParameter, rvalue string)
 
 	shouldReturn = shouldReturn[len(indent):]
 
+	namePrefix := p.ParameterName // TODO make unique, strip out [0], etc
+
 	if p.ParameterType == "void" && !p.Pointer {
 		shouldReturn = ""
 
@@ -337,21 +333,17 @@ func emitAssignCppToCabi(assignExpression string, p CppParameter, rvalue string)
 			// These are rare, and probably expected to be lightweight references
 			// But, a copy is the best we can project it as
 			// Un-pointer-ify
-			shouldReturn = "QString* ret = "
+			shouldReturn = ifv(p.Const, "const ", "") + "QString* " + namePrefix + "_ret = "
 			afterCall = indent + "// Convert QString pointer from UTF-16 in C++ RAII memory to UTF-8 in manually-managed C memory\n"
-			afterCall += indent + "QByteArray b = ret->toUtf8();\n"
+			afterCall += indent + "QByteArray " + namePrefix + "_b = " + namePrefix + "_ret->toUtf8();\n"
 
 		} else {
-			shouldReturn = "QString ret = "
+			shouldReturn = ifv(p.Const, "const ", "") + "QString " + p.ParameterName + "_ret = "
 			afterCall = indent + "// Convert QString from UTF-16 in C++ RAII memory to UTF-8 in manually-managed C memory\n"
-			afterCall += indent + "QByteArray b = ret.toUtf8();\n"
+			afterCall += indent + "QByteArray " + namePrefix + "_b = " + namePrefix + "_ret.toUtf8();\n"
 		}
-		if p.Const {
+
-			shouldReturn = "const " + shouldReturn
+		afterCall += indent + assignExpression + "miqt_strdup(" + namePrefix + "_b.data(), " + namePrefix + "_b.length());\n"
-		}
-		afterCall += indent + "*_out = static_cast<char*>(malloc(b.length()));\n"
-		afterCall += indent + "memcpy(*_out, b.data(), b.length());\n"
-		afterCall += indent + "*_out_Strlen = b.length();\n"
 
 	} else if t, ok := p.QListOf(); ok {
 
@@ -376,37 +368,37 @@ func emitAssignCppToCabi(assignExpression string, p CppParameter, rvalue string)
 
 		} else if !t.QtClassType() || (t.QtClassType() && t.Pointer) { // QList<int>, QList<QFoo*>
 
-			shouldReturn = p.RenderTypeQtCpp() + " ret = "
+			shouldReturn = p.RenderTypeQtCpp() + " " + namePrefix + "_ret = "
 			afterCall += indent + "// Convert QList<> from C++ memory to manually-managed C memory\n"
-			afterCall += indent + "" + t.RenderTypeCabi() + "* __out = static_cast<" + t.RenderTypeCabi() + "*>(malloc(sizeof(" + t.RenderTypeCabi() + ") * ret.length()));\n"
+			afterCall += indent + "" + t.RenderTypeCabi() + "* __out = static_cast<" + t.RenderTypeCabi() + "*>(malloc(sizeof(" + t.RenderTypeCabi() + ") * " + namePrefix + "_ret.length()));\n"
-			afterCall += indent + "for (size_t i = 0, e = ret.length(); i < e; ++i) {\n"
+			afterCall += indent + "for (size_t i = 0, e = " + namePrefix + "_ret.length(); i < e; ++i) {\n"
 			if t.Const {
 				nonConst := t // copy
 				nonConst.Const = false
-				afterCall += indent + "\t__out[i] = const_cast<" + t.RenderTypeCabi() + ">(ret[i]);\n"
+				afterCall += indent + "\t__out[i] = const_cast<" + t.RenderTypeCabi() + ">(" + namePrefix + "_ret[i]);\n"
 			} else {
-				afterCall += indent + "\t__out[i] = ret[i];\n"
+				afterCall += indent + "\t__out[i] = " + namePrefix + "_ret[i];\n"
 			}
 			afterCall += indent + "}\n"
 			afterCall += indent + "*_out = __out;\n"
-			afterCall += indent + "*_out_len = ret.length();\n"
+			afterCall += indent + "*_out_len = " + namePrefix + "_ret.length();\n"
 
 		} else { // QList<QFoo>
 
-			shouldReturn = p.RenderTypeQtCpp() + " ret = "
+			shouldReturn = p.RenderTypeQtCpp() + " " + namePrefix + "_ret = "
 			afterCall += indent + "// Convert QList<> from C++ memory to manually-managed C memory of copy-constructed pointers\n"
-			afterCall += indent + "" + t.RenderTypeCabi() + "** __out = static_cast<" + t.RenderTypeCabi() + "**>(malloc(sizeof(" + t.RenderTypeCabi() + "**) * ret.length()));\n"
+			afterCall += indent + "" + t.RenderTypeCabi() + "** __out = static_cast<" + t.RenderTypeCabi() + "**>(malloc(sizeof(" + t.RenderTypeCabi() + "**) * " + namePrefix + "_ret.length()));\n"
-			afterCall += indent + "for (size_t i = 0, e = ret.length(); i < e; ++i) {\n"
+			afterCall += indent + "for (size_t i = 0, e = " + namePrefix + "_ret.length(); i < e; ++i) {\n"
-			afterCall += indent + "\t__out[i] = new " + t.ParameterType + "(ret[i]);\n"
+			afterCall += indent + "\t__out[i] = new " + t.ParameterType + "(" + namePrefix + "_ret[i]);\n"
 			afterCall += indent + "}\n"
 			afterCall += indent + "*_out = __out;\n"
-			afterCall += indent + "*_out_len = ret.length();\n"
+			afterCall += indent + "*_out_len = " + namePrefix + "_ret.length();\n"
 
 		}
 
 	} else if p.QtClassType() && p.ByRef {
 		// It's a pointer in disguise, just needs one cast
-		shouldReturn = p.RenderTypeQtCpp() + " ret = "
+		shouldReturn = p.RenderTypeQtCpp() + " " + namePrefix + "_ret = "
 		afterCall += indent + "// Cast returned reference into pointer\n"
 		if p.Const {
 			nonConst := p // copy
@@ -414,28 +406,28 @@ func emitAssignCppToCabi(assignExpression string, p CppParameter, rvalue string)
 			nonConst.ByRef = false
 			nonConst.Pointer = true
 			nonConst.PointerCount = 1
-			afterCall += indent + "" + assignExpression + "const_cast<" + nonConst.RenderTypeQtCpp() + ">(&ret);\n"
+			afterCall += indent + "" + assignExpression + "const_cast<" + nonConst.RenderTypeQtCpp() + ">(&" + namePrefix + "_ret);\n"
 		} else {
-			afterCall += indent + "" + assignExpression + "&ret;\n"
+			afterCall += indent + "" + assignExpression + "&" + namePrefix + "_ret;\n"
 		}
 
 	} else if p.QtClassType() && !p.Pointer {
-		shouldReturn = p.ParameterType + " ret = "
+		shouldReturn = p.ParameterType + " " + namePrefix + "_ret = "
 		afterCall = indent + "// Copy-construct value returned type into heap-allocated copy\n"
-		afterCall += indent + "return static_cast<" + p.ParameterType + "*>(new " + p.ParameterType + "(ret));\n"
+		afterCall += indent + "return static_cast<" + p.ParameterType + "*>(new " + p.ParameterType + "(" + namePrefix + "_ret));\n"
 
 	} else if p.Const {
 		shouldReturn += "(" + emitReturnTypeCabi(p) + ") "
 
 	} else if p.IsFlagType() {
 		// Needs an explicit int cast
-		shouldReturn = p.RenderTypeQtCpp() + " ret = "
+		shouldReturn = p.RenderTypeQtCpp() + " " + namePrefix + "_ret = "
-		afterCall += indent + "" + assignExpression + "static_cast<int>(ret);\n"
+		afterCall += indent + "" + assignExpression + "static_cast<int>(" + namePrefix + "_ret);\n"
 
 	} else if p.IsEnum() {
 		// Needs an explicit uintptr cast
-		shouldReturn = p.RenderTypeQtCpp() + " ret = "
+		shouldReturn = p.RenderTypeQtCpp() + " " + namePrefix + "_ret = "
-		afterCall += indent + "" + assignExpression + "static_cast<uintptr_t>(ret);\n"
+		afterCall += indent + "" + assignExpression + "static_cast<uintptr_t>(" + namePrefix + "_ret);\n"
 
 	}
 
@@ -638,8 +630,8 @@ func emitBindingCpp(src *CppParsedHeader, filename string) (string, error) {
 		ret.WriteString(`#include <` + ref + ">\n")
 	}
 
-	ret.WriteString(`#include "` + filename + "\"\n\n")
+	ret.WriteString(`#include "` + filename + "\"\n")
-	ret.WriteString(`#include "gen_` + filename + "\"\n")
+	ret.WriteString(`#include "gen_` + filename + "\"\n\n")
 
 	for _, c := range src.Classes {
 

cmd/genbindings/emitgo.go

@@ -182,13 +182,12 @@ func (gfs *goFileState) emitParametersGo2CABIForwarding(m CppMethod) (preamble s
 			skipNext = false
 
 		} else if p.ParameterType == "QString" {
-			// Go: convert string -> char* and len
+			// Go: convert string -> miqt_string*
-			// CABI: convert char* and len -> real QString
+			// CABI: convert miqt_string* -> real QString
-			gfs.imports["unsafe"] = struct{}{}
 
-			preamble += p.ParameterName + "_Cstring := C.CString(" + p.ParameterName + ")\n"
+			preamble += p.ParameterName + "_ms := miqt_strdupg(" + p.ParameterName + ")\n"
-			preamble += "defer C.free(unsafe.Pointer(" + p.ParameterName + "_Cstring))\n"
+			preamble += "defer C.free(" + p.ParameterName + "_ms)\n"
-			tmp = append(tmp, p.ParameterName+"_Cstring, C.size_t(len("+p.ParameterName+"))") // Second parameter cast to size_t projected type
+			tmp = append(tmp, p.ParameterName+"_ms")
 
 		} else if listType, ok := p.QListOf(); ok {
 			// QList<T>
@@ -259,9 +258,7 @@ func (gfs *goFileState) emitParametersGo2CABIForwarding(m CppMethod) (preamble s
 		}
 	}
 
-	if m.ReturnType.ParameterType == "QString" {
+	if t, ok := m.ReturnType.QListOf(); ok {
-		tmp = append(tmp, "&_out, &_out_Strlen")
-	} else if t, ok := m.ReturnType.QListOf(); ok {
 
 		if t.ParameterType == "QString" {
 			// Combo
@@ -413,18 +410,17 @@ import "C"
 				// internal pointer
 				gfs.imports["unsafe"] = struct{}{}
 				returnTypeDecl = "unsafe.Pointer"
+
 				shouldReturn = "ret := "
 				afterword += "return (unsafe.Pointer)(ret)\n"
 
 			} else if m.ReturnType.ParameterType == "QString" {
-				shouldReturn = ""
 				returnTypeDecl = "string"
 				gfs.imports["unsafe"] = struct{}{}
 
-				preamble += "var _out *C.char = nil\n"
+				shouldReturn = "var ret_ms *C.struct_miqt_string = "
-				preamble += "var _out_Strlen C.int = 0\n" // I think size_t is "better" but GoStringN() requires C.int
+				afterword += "ret := C.GoStringN(&ret_ms.data, C.int(int64(ret_ms.len)))\n"
-				afterword += "ret := C.GoStringN(_out, _out_Strlen)\n"
+				afterword += "C.free(unsafe.Pointer(ret_ms))\n"
-				afterword += "C.free(unsafe.Pointer(_out))\n"
 				afterword += "return ret"
 
 			} else if t, ok := m.ReturnType.QListOf(); ok {

qt/binding.cpp

@@ -4,8 +4,8 @@
 #include "binding.h"
 
 struct miqt_string* miqt_strdup(const char* src, size_t len) {
-    struct miqtstring* ret = static_cast<struct miqtstring*>( malloc(len + sizeof(size_t)) );
+    struct miqt_string* ret = (struct miqt_string*)( malloc(len + sizeof(size_t)) );
-    ret->len = strlen(expect);
+    ret->len = len;
-    memcpy(&ret->data, expect, ret->len);
+    memcpy(&ret->data, src, len);
     return ret;
 }

qt/binding.h

@@ -1,5 +1,5 @@
-#ifndef GEN_QABSTRACTANIMATION_H
+#ifndef MIQT_BINDING_H
-#define GEN_QABSTRACTANIMATION_H
+#define MIQT_BINDING_H
 
 #ifdef __cplusplus
 extern "C" {
@@ -11,20 +11,20 @@ void miqt_exec_callback(void* cb, int argc, void* argv);
 
 struct miqt_string {
     size_t len;
-    char data; // Data continues after this element.
+    char data; // Data continues after this element, all in the same allocation
-}
+};
 
 struct miqt_array {
     size_t len;
-    char data; // Data continues after this element.
+    void* data; // Separate, second allocation
-}
+};
 
 // miqt_strdup allocates a miqt_string and copies C data into it.
 // The function is defined in C++.
 struct miqt_string* miqt_strdup(const char* src, size_t len);
 
 #ifdef __cplusplus
-extern "C" {
+}
 #endif
 
 #endif

qt/binding2.go

@@ -0,0 +1,21 @@
+package qt
+
+// SPDX-License-Identifier: MIT
+
+/*
+
+#include "binding.h"
+
+struct miqt_string* miqt_strdupg(_GoString_ gs) {
+	return miqt_strdup(_GoStringPtr(gs), _GoStringLen(gs));
+}
+
+*/
+import "C"
+
+// miqt_strdupg will strdup a Go string into a miqt_string*.
+// It is typed as returning an unsafe.Pointer because Cgo types cannot be shared
+// across Go file boundaries.
+func miqt_strdupg(s string) unsafe.Pointer {
+	return C.miqt_strdupg(s)
+}