therecipe / qt

Qt binding for Go (Golang) with support for Windows / macOS / Linux / FreeBSD / Android / iOS / Sailfish OS / Raspberry Pi / AsteroidOS / Ubuntu Touch / JavaScript / WebAssembly
GNU Lesser General Public License v3.0
10.48k stars 748 forks source link

qtminimal code generation bug #220

Open aebruno opened 7 years ago

aebruno commented 7 years ago

qtminimal is picking up function names outside of the qt namespace. The following simple example generates different core/core-minimal* files when it seems like they should be identical. For example, this compiles ok:

package main

import (
    "github.com/therecipe/qt/core"
    "github.com/somepackage/test"
)

//go:generate qtmoc
type Person struct {
    core.QObject
}

func (p *Person) DoSomething() bool {
    return test.Tester() == nil
}

However, simply changing test.Tester() to test.Group() fails to compile:

package main

import (
    "github.com/therecipe/qt/core"
    "github.com/somepackage/test"
)

//go:generate qtmoc
type Person struct {
    core.QObject
}

func (p *Person) DoSomething() bool {
    return test.Group() == nil
}

After running qtminimal sailfish-emulator $PWD and inspecting the difference between core/core-minimal.* files shows that the latter added a bunch of code. Seems like qtminimal is picking up the test.Group() function as being part of QT core when it's not:

--- /tmp/bug/core-minimal.go
+++ ../../therecipe/qt/core/core-minimal.go
+
+func (ptr *QAbstractAnimation) Group() *QAnimationGroup {
+   if ptr.Pointer() != nil {
+       var tmpValue = NewQAnimationGroupFromPointer(C.QAbstractAnimation_Group(ptr.Pointer()))
+       if !qt.ExistsSignal(fmt.Sprint(tmpValue.Pointer()), "QObject::destroyed") {
+           tmpValue.ConnectDestroyed(func(*QObject) { tmpValue.SetPointer(nil) })
+       }
+       return tmpValue
+   }
+   return nil
+}
+
+type QAnimationGroup struct {
+   QAbstractAnimation
+}
+
+func (ptr *QAnimationGroup) DestroyQAnimationGroup() {
+   if ptr.Pointer() != nil {
+       C.QAnimationGroup_DestroyQAnimationGroup(ptr.Pointer())
+       qt.DisconnectAllSignals(fmt.Sprint(ptr.Pointer()))
+       ptr.SetPointer(nil)
+   }
+}
+
+func (ptr *QFileInfo) Group() string {
+   if ptr.Pointer() != nil {
+       return cGoUnpackString(C.QFileInfo_Group(ptr.Pointer()))
+   }
+   return ""
+}
+
+func (ptr *QSettings) Group() string {
+   if ptr.Pointer() != nil {
+       return cGoUnpackString(C.QSettings_Group(ptr.Pointer()))
+   }
+   return ""
+}
+

This feels like a bug or perhaps there should be some way to limit qtminimal to only searching/adding code under the github.com/therecipe/qt/* namespace?

therecipe commented 7 years ago

Hey

Yes, the problem is that qtminimal just simply scans for class and function names. This is the simplest method to filter out most of the unnecessary functions.

To make it more effective and only include the really necessary functions, there would be a better parser needed. One which is context aware and can report whether or not test is really a Qt class.

I will look around, maybe there is one I can use to replace the old one.

aebruno commented 7 years ago

Yeah having a more intelligent parser would be great but can imagine that's a non-trivial task :)

The immediate issue I'm running into is I need to use the test.Group() function and it's including the code from QAnimationGroup which is failing to compile. In the short term, I'm fine with the added unnecessary code as along as it compiles. Here's the error:

../../therecipe/qt/core/core-minimal.cpp: In function 'void* QAnimationGroup_NewQAnimationGroup(void*)':
../../therecipe/qt/core/core-minimal.cpp:991:60: error: cannot allocate an object of abstract type 'MyQAnimationGroup'
  return new MyQAnimationGroup(static_cast<QObject*>(parent));
                                                            ^
../../therecipe/qt/core/core-minimal.cpp:983:7: note:   because the following virtual functions are pure within 'MyQAnimationGroup':
 class MyQAnimationGroup: public QAnimationGroup
       ^
In file included from /srv/mer/targets/SailfishOS-i486/usr/include/qt5/QtCore/QAbstractAnimation:1:0,
                 from ../../therecipe/qt/core/core-minimal.cpp:9:
/srv/mer/targets/SailfishOS-i486/usr/include/qt5/QtCore/qabstractanimation.h:103:17: note:      virtual int QAbstractAnimation::duration() const
     virtual int duration() const = 0;
                 ^
/srv/mer/targets/SailfishOS-i486/usr/include/qt5/QtCore/qabstractanimation.h:124:18: note:      virtual void QAbstractAnimation::updateCurrentTime(int)
     virtual void updateCurrentTime(int currentTime) = 0;
                  ^

Any thoughts? I could try blacklisting QAnimationGroup but there doesn't seem to be an easy way to do this.

Thanks!

therecipe commented 7 years ago

Sorry, for the unimplemented pure virtual functions there is an simple fix / small trick.

You just need to make use of these functions (or it has to look so at least).

In your case you just need to include:

/*
  .Duration(
  .UpdateCurrentTime(
*/

somewhere in one of your *.go files.

This is another (related) issue, and should be fixed once I'm done with the changes to the code generator.

aebruno commented 7 years ago

Great! Thanks, I'll give this a try.

aebruno commented 7 years ago

The above trick worked great. Thanks.

aebruno commented 7 years ago

the problem is that qtminimal just simply scans for class and function names. This is the simplest method to filter out most of the unnecessary functions.

To make it more effective and only include the really necessary functions, there would be a better parser needed. One which is context aware and can report whether or not test is really a Qt class.

I will look around, maybe there is one I can use to replace the old one.

Curious if you had a chance to look into this at all?

therecipe commented 7 years ago

Curious if you had a chance to look into this at all?

No not really, I actually rewrote most of qtminimal but only to solve the issue with abstract classes (missing functions) and to cleanup the code a bit. But I didn't looked into a way to intelligently parse code yet.

I will do some experiments in the next few days and report back if I got something working or not.