Skip to content

Commit

Permalink
Methods with multiple arguments and at most one return value work.
Browse files Browse the repository at this point in the history
  • Loading branch information
niemeyer committed Sep 9, 2013
1 parent c4da46f commit 6fb57d0
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 32 deletions.
28 changes: 15 additions & 13 deletions all_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ func (ts *testStruct) StringMethod() string {
return ts.StringValue
}

func (ts *testStruct) PresetString() string {
ts.StringValue = "<preset value>"
return ts.StringValue
func (ts *testStruct) ChangeString(new string) (old string) {
old = ts.StringValue
ts.StringValue = new
return
}

func intIs64() bool {
Expand Down Expand Up @@ -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("<new content>"));
}
}
`

component, err := s.engine.Load(qml.String("file.qml", data))
Expand All @@ -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 <string content>.*")
c.Assert(c.GetTestLog(), Matches, "(?s).*string was <string content>.*")
c.Assert(value.StringValue, Equals, "<new content>")
}

// 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: "<string content>"}
s.context.SetVar("value", value)
Expand All @@ -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("<new content>")
}
}
`
Expand All @@ -408,5 +410,5 @@ func (s *S) TestConnectQmlSignalToGoMethod(c *C) {
inst := component.Create(s.context)
inst.Destroy()

c.Assert(value.StringValue, Equals, "<preset value>")
c.Assert(value.StringValue, Equals, "<new content>")
}
19 changes: 16 additions & 3 deletions bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,22 +274,35 @@ 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)

// TODO Must ensure that v is necessarily a pointer here.

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 {
Expand Down
7 changes: 5 additions & 2 deletions cpp/capi.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down Expand Up @@ -55,8 +58,8 @@ typedef struct {
int addrOffset;
char *methodSignature;
char *resultSignature;
int argsIn;
int argsOut;
int numIn;
int numOut;
} GoMemberInfo;

typedef struct {
Expand Down
23 changes: 15 additions & 8 deletions cpp/govalue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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<QVariant *>(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<QVariant *>(a[i]), &args[i]);
}
hookGoValueCallMethod(qmlEngine(value), valuePriv->addr, memberInfo->reflectIndex, args);
if (memberInfo->numOut > 0) {
unpackDataValue(&args[0], reinterpret_cast<QVariant *>(a[0]));
}
return -1;
}
memberInfo++;
Expand Down Expand Up @@ -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
Expand Down
11 changes: 5 additions & 6 deletions datatype.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -265,17 +266,15 @@ 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")
}
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")
}
Expand Down

0 comments on commit 6fb57d0

Please sign in to comment.