Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle struct values (non-pointer) #67

Open
quarnster opened this issue May 25, 2014 · 6 comments
Open

Handle struct values (non-pointer) #67

quarnster opened this issue May 25, 2014 · 6 comments

Comments

@quarnster
Copy link
Contributor

Occurs in both v0 and v1:

package qml_test

import (
    "gopkg.in/qml.v1"
    "testing"
)

func init() {
    qml.SetupTesting()
}

func TestBlah(t *testing.T) {
    //qml.Init(nil)
    type Blah struct {
        A int
    }

    f := func() error {
        e := qml.NewEngine()
        defer e.Destroy()
        e.Context().SetVar("blah", Blah{})
        c, err := e.LoadString("blah.qml", `
import QtQuick 2.0
Item {
    Component.onCompleted: {
        blah.a = 10;
    }
}
`)
        if err != nil {
            return err
        }
        w := c.CreateWindow(nil)
        defer w.Destroy()
        return nil
    }
    err := f()
    if err != nil {
        t.Error(err)
    }
}

Yes I understand that it goes away if I do e.Context().SetVar("blah", &Blah{}), but that is besides the point.

Real use case is a go function that returns a small struct type by value. This value I then want to modify qml side to pass on this modified data to another go function.

@niemeyer
Copy link
Contributor

If I understand what you're trying to achieve, this won't work, for the same reason this does not work:

http://play.golang.org/p/jMTwpE5bNo

Does that make sense?

If so, I'll change the topic of this issue as that error message is really bad and should be fixed.

@quarnster
Copy link
Contributor Author

My point is that this works: http://play.golang.org/p/sbPAvbv6cf

@niemeyer
Copy link
Contributor

The original point was about modifying the returned value, which does not work. But yeah, we can allocate a new value and copy it. Somewhat wasteful, but maybe it's worth to make such methods work.

@niemeyer niemeyer changed the title panic: cannot use int as a int (setting data in non-pointer struct) Handle struct values (non-pointer) May 26, 2014
@niemeyer
Copy link
Contributor

Yeah, let's do that. Thanks for bringing this up.

@quarnster
Copy link
Contributor Author

Making Go-QML "convert" return values into set-able types (if they weren't already) makes the QML script side behave more as the same code would work if it was written in Go, so IMO it's worth it.

There's already unavoidable overhead in every QML/Go function call boundary and any app where this boundary has a noticeable performance impact would do better in trying to get rid of the QML/Go boundary crossing in the first place.

The memory allocation could potentially be partially removed by using reflect.NewAt and keeping a small piece of memory for QML function stack, but I doubt the complexity of that is actually worth the effort :)

IIRC I also saw a panic-call when passing a "non-hashable" struct as a function argument or something when skimming the code. Would it be possible to use a similar approach there?

@niemeyer
Copy link
Contributor

Yeah, the unhashable note is about the same issue. If we copy the value and get an address for it, both problems will go away.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants