From 6fb57d062af091e239bda997e74328e6f2dd8d10 Mon Sep 17 00:00:00 2001 From: Gustavo Niemeyer Date: Mon, 9 Sep 2013 17:32:36 -0300 Subject: [PATCH] Methods with multiple arguments and at most one return value work. --- all_test.go | 28 +++++++++++++++------------- bridge.go | 19 ++++++++++++++++--- cpp/capi.h | 7 +++++-- cpp/govalue.cpp | 23 +++++++++++++++-------- datatype.go | 11 +++++------ 5 files changed, 56 insertions(+), 32 deletions(-) diff --git a/all_test.go b/all_test.go index c6c7b358..e93353ac 100644 --- a/all_test.go +++ b/all_test.go @@ -77,9 +77,10 @@ func (ts *testStruct) StringMethod() string { return ts.StringValue } -func (ts *testStruct) PresetString() string { - ts.StringValue = "" - return ts.StringValue +func (ts *testStruct) ChangeString(new string) (old string) { + old = ts.StringValue + ts.StringValue = new + return } func intIs64() bool { @@ -370,7 +371,11 @@ func (s *S) TestMethodCall(c *C) { data := ` import QtQuick 2.0 - Item { Component.onCompleted: console.log('string is', value.stringMethod()); } + Item { + Component.onCompleted: { + console.log("string was", value.changeString("")); + } + } ` component, err := s.engine.Load(qml.String("file.qml", data)) @@ -379,13 +384,10 @@ func (s *S) TestMethodCall(c *C) { inst := component.Create(s.context) inst.Destroy() - c.Assert(c.GetTestLog(), Matches, "(?s).*string is .*") + c.Assert(c.GetTestLog(), Matches, "(?s).*string was .*") + c.Assert(value.StringValue, Equals, "") } -// TODO presetString is a weird test method, but allows moving forward without -// methods fully implemented. Change it to something more reasonable once -// methods work properly. - func (s *S) TestConnectQmlSignalToGoMethod(c *C) { value := &testStruct{StringValue: ""} s.context.SetVar("value", value) @@ -394,10 +396,10 @@ func (s *S) TestConnectQmlSignalToGoMethod(c *C) { import QtQuick 2.0 Item { id: item - signal testSignal() + signal testSignal(string s) Component.onCompleted: { - item.testSignal.connect(value.presetString) - item.testSignal() + item.testSignal.connect(value.changeString) + item.testSignal("") } } ` @@ -408,5 +410,5 @@ func (s *S) TestConnectQmlSignalToGoMethod(c *C) { inst := component.Create(s.context) inst.Destroy() - c.Assert(value.StringValue, Equals, "") + c.Assert(value.StringValue, Equals, "") } diff --git a/bridge.go b/bridge.go index 4bd7aadd..fe0abd70 100644 --- a/bridge.go +++ b/bridge.go @@ -274,8 +274,10 @@ func convertAndSet(to, from reflect.Value) { to.Set(from.Convert(to.Type())) } +var dataValueSize = uintptr(unsafe.Sizeof(C.DataValue{})) + //export hookGoValueCallMethod -func hookGoValueCallMethod(enginep unsafe.Pointer, foldp unsafe.Pointer, reflectIndex C.int, resultdv *C.DataValue) { +func hookGoValueCallMethod(enginep unsafe.Pointer, foldp unsafe.Pointer, reflectIndex C.int, args *C.DataValue) { fold := ensureEngine(enginep, foldp) v := reflect.ValueOf(fold.gvalue) @@ -283,13 +285,24 @@ func hookGoValueCallMethod(enginep unsafe.Pointer, foldp unsafe.Pointer, reflect method := v.Method(int(reflectIndex)) + // TODO Ensure methods with more parameters than this are not registered. + var params [C.MaximumParamCount-1]reflect.Value + + numIn := uintptr(method.Type().NumIn()) + for i := uintptr(0); i < numIn; i++ { + // TODO Type checking to avoid explosions (or catch the explosion) + paramdv := (*C.DataValue)(unsafe.Pointer(uintptr(unsafe.Pointer(args)) + (i+1) * dataValueSize)) + params[i] = reflect.ValueOf(unpackDataValue(paramdv)) + } + + result := method.Call(params[:numIn]) + // TODO Unhardcode this. - result := method.Call(nil) if len(result) != 1 || result[0].Type() != typeString { panic("result must be a string for now") } - packDataValue(result[0].Interface(), resultdv, fold.engine, jsOwner) + packDataValue(result[0].Interface(), args, fold.engine, jsOwner) } func ensureEngine(enginep unsafe.Pointer, foldp unsafe.Pointer) *valueFold { diff --git a/cpp/capi.h b/cpp/capi.h index 973be95b..fc5783ed 100644 --- a/cpp/capi.h +++ b/cpp/capi.h @@ -8,6 +8,9 @@ extern "C" { #endif +// It's surprising that this constant is privately defined within qmetaobject.cpp. +enum { MaximumParamCount = 11 }; // Up to 10 arguments + 1 return value + typedef void QApplication_; typedef void QMetaObject_; typedef void QObject_; @@ -55,8 +58,8 @@ typedef struct { int addrOffset; char *methodSignature; char *resultSignature; - int argsIn; - int argsOut; + int numIn; + int numOut; } GoMemberInfo; typedef struct { diff --git a/cpp/govalue.cpp b/cpp/govalue.cpp index 43adf4a4..d5a413a5 100644 --- a/cpp/govalue.cpp +++ b/cpp/govalue.cpp @@ -48,14 +48,13 @@ int GoValueMetaObject::metaCall(QMetaObject::Call c, int idx, void **a) case QMetaObject::ReadProperty: case QMetaObject::WriteProperty: { - // TODO cache propertyOffset() + // TODO Cache propertyOffset, methodOffset, and qmlEngine results? if (idx < propertyOffset()) { return value->qt_metacall(c, idx, a); } GoMemberInfo *memberInfo = valuePriv->typeInfo->fields; for (int i = 0; i < valuePriv->typeInfo->fieldsLen; i++) { if (memberInfo->metaIndex == idx) { - // TODO Cache the qmlEngine call result? if (c == QMetaObject::ReadProperty) { DataValue result; hookGoValueReadField(qmlEngine(value), valuePriv->addr, memberInfo->reflectIndex, &result); @@ -77,18 +76,21 @@ int GoValueMetaObject::metaCall(QMetaObject::Call c, int idx, void **a) } case QMetaObject::InvokeMetaMethod: { - // TODO cache methodOffset() if (idx < methodOffset()) { return value->qt_metacall(c, idx, a); } GoMemberInfo *memberInfo = valuePriv->typeInfo->methods; for (int i = 0; i < valuePriv->typeInfo->methodsLen; i++) { if (memberInfo->metaIndex == idx) { - // TODO Cache the qmlEngine call result? - DataValue result; - hookGoValueCallMethod(qmlEngine(value), valuePriv->addr, memberInfo->reflectIndex, &result); - QVariant *out = reinterpret_cast(a[0]); - unpackDataValue(&result, out); + // args[0] is the result if any. + DataValue args[MaximumParamCount]; + for (int i = 1; i < memberInfo->numIn+1; i++) { + packDataValue(reinterpret_cast(a[i]), &args[i]); + } + hookGoValueCallMethod(qmlEngine(value), valuePriv->addr, memberInfo->reflectIndex, args); + if (memberInfo->numOut > 0) { + unpackDataValue(&args[0], reinterpret_cast(a[0])); + } return -1; } memberInfo++; @@ -125,6 +127,11 @@ GoAddr *GoValue::addr() void GoValue::activate(int fieldReflectIndex) { Q_D(GoValue); + // TODO This is actually broken. reflectIndex may contain holes. + // Can still use it, but must first find the metaIndex for the corresponding + // reflectIndex, compute the relative index, and then do what follows with the + // relative index. + // Go fields have an absolute index of (propertyOffset + fieldReflectIndex), // while Go methods have (methodOffset + fieldCount + methodReflectIndex), // because properties are added first, and each property gets its own signal diff --git a/datatype.go b/datatype.go index 4dd78971..c160776a 100644 --- a/datatype.go +++ b/datatype.go @@ -231,8 +231,9 @@ func typeInfo(v interface{}) *C.GoTypeInfo { memberInfo.resultSignature = C.CString(result) // TODO Sort out methods with a variable number of arguments. // TODO Sort out methods with more than one result. - memberInfo.argsIn = C.int(method.Type.NumIn()-1) - memberInfo.argsOut = C.int(method.Type.NumOut()) + // It's called while bound, so drop the receiver. + memberInfo.numIn = C.int(method.Type.NumIn()-1) + memberInfo.numOut = C.int(method.Type.NumOut()) membersi += 1 mnamesi += uintptr(len(method.Name)) + 1 } @@ -265,9 +266,7 @@ func methodQtSignature(method reflect.Method) (signature, result string) { buf.WriteByte('(') n := method.Type.NumIn() for i := 1; i < n; i++ { - // TODO What would be the benefits of using more typing information here? - // Connections would be checked at connect time; what else? - if i > 0 { + if i > 1 { buf.WriteByte(',') } buf.WriteString("QVariant") @@ -275,7 +274,7 @@ func methodQtSignature(method reflect.Method) (signature, result string) { buf.WriteByte(')') signature = buf.String() - // TODO Implement zero and multiple returns. + // TODO Implement multiple returns. if method.Type.NumOut() > 1 { panic("unfinished implementation: methods can only have zero or one return value") }