Open arkanoid87 opened 2 years ago
position of forward declarations:
$ rg "proc [A-Za-z]+\*?\([: a-zA-Z]+\)\$"
examples/connections/main.nim
8: proc delete*(self: Contact)
9: proc setup(self: Contact)
examples/contactapp/contactlist.nim
13: proc delete(self: ContactList)
14: proc setup(self: ContactList)
examples/slotsandproperties/contact.nim
7: proc delete*(self: Contact)
8: proc setup(self: Contact)
examples/abstractitemmodel/mylistmodel.nim
12: proc delete(self: MyListModel)
13: proc setup(self: MyListModel)
src/nimqml/private/constructors.nim
17:proc setup*(self: QAbstractItemModel)
18:proc delete*(self: QAbstractItemModel)
26:proc setup*(self: QAbstractListModel)
27:proc delete*(self: QAbstractListModel)
35:proc setup*(self: QAbstractTableModel)
36:proc delete*(self: QAbstractTableModel)
44:proc setup*(variant: QVariant)
53:proc delete*(variant: QVariant)
97:proc delete*(self: QUrl)
104:proc setup*(self: QQuickView)
105:proc delete*(self: QQuickView)
112:proc setup*(self: QQmlApplicationEngine)
113:proc delete*(self: QQmlApplicationEngine)
120:proc setup*(self: QModelIndex)
122:proc delete*(self: QModelIndex)
135:proc delete*(self: QMetaObjectConnection)
146:proc delete*(metaObject: QMetaObject)
185:proc setup*(self: QHashIntByteArray)
186:proc delete*(self: QHashIntByteArray)
193:proc setup*(self: QGuiApplication)
194:proc delete*(self: QGuiApplication)
examples/charts/mylistmodel.nim
15: proc delete(self: MyListModel)
16: proc setup(self: MyListModel)
Client users that use NimQml already created their delete
methods that are called from the base class finalizers. This delete
methods take a ref T
argument thus they're not callable from a destroy
destructors. It's not possible to switch to destructors without breaking existing client code. The problem is probably in the compiler. I would wait for a fix for https://github.com/nim-lang/Nim/issues/19402
do we really have to wait a fix in nim compiler if a working solution seems to exist just by avoiding forward declarations of finalizers?
--gc:arc
type Foo = ref object of RootObj
proc delete(self: Foo)
proc newFoo: Foo = new(result, delete)
proc delete(self: Foo) = echo("delete Foo")
if isMainModule:
let mem0 = getOccupiedMem()
proc test() = discard newFoo()
test()
GC_fullCollect()
echo (getOccupiedMem() - mem0)
0
--gc:arc
type Foo = ref object of RootObj
proc delete(self: Foo) = echo("delete Foo")
proc newFoo: Foo = new(result, delete)
if isMainModule:
let mem0 = getOccupiedMem()
proc test() = discard newFoo()
test()
GC_fullCollect()
echo (getOccupiedMem() - mem0)
delete foo
0
@arkanoid87 No i'm ok in a workaround. Maybe i'm wrong but i recall having some issue in removing the finalizers forward declarations. Compiler seemed buggy in there. I had some bikeshedding and for example i had to remove the forwad declaration for the QObject (see https://github.com/filcuc/nimqml/blob/4f237b485475cde9d84dd4dfbba276ff66575a15/src/nimqml/private/constructors.nim#L3) I can give a try by moving all the finalizers definitions in constructors.nim
I think this has to be tackled first
Compiles and runs with refc, doesn't compile with arc. Is this the source of forward declarations / bikeshedding?
type
Parent* = ref object of RootObj
Child* = ref object of Parent
proc setup*(self: Parent) =
echo "Parent setup"
proc delete*(self: Parent) =
echo "Parent delete"
proc newParent*(): Parent =
echo "Parent newParent"
new(result, delete)
result.setup
proc setup(self: Child) =
self.Parent.setup
echo "Child setup"
proc delete*(self: Child) =
echo "Child delete"
self.Parent.delete
proc newChild*(): Child =
echo "Child newChild"
new(result, delete)
result.setup
proc main = discard newChild()
if isMainModule:
main()
GC_fullCollect()
--gc:refc
Child newChild
Parent setup
Child setup
Child delete
Parent delete
--gc:arc
Error: cannot bind another '=destroy' to: Child:ObjectType; previous declaration was constructed here implicitly
good news, the nim issue has been tagged as showstopper
@xflywind suggested to add {.experimental: "codeReordering".}
on module to fix, this would also remove the need for forward declarations
https://github.com/nim-lang/Nim/issues/19402#issuecomment-1022921122
problem is that I've tried to add it on top of constructors.nim and qqmlapplicationengine.nim but this code:
--gc:arc --d:debug
import nimqml
proc main =
var app = newQApplication()
var engine = newQQmlApplicationEngine()
engine.load("qml/main.qml")
app.exec()
echo "main scope end"
when isMainModule:
main()
echo "goodbye"
GC_fullCollect()
doesn't print NimQml: QQmlApplicationEngine: delete
, so the problem has to be extended to multiple file context and extended
use of the include
feature
EDIT: I've also tried adding it on top of nimqml.nim but yet no cake
the feature is experimental for a reason i would say...we need a proper fix with forward declarations imho
before I try removing the forward declarations by lot of refactoring, would you consider the change of setup
and delete
from proc
to method
or using destructors instead of finalizers?
i don't get why you want those changes and why it metters here. Again this is a compiler problem not of NimQml. If ORC/ARC is broken is not our fault
because current nimqml code is silently leaking memory with ORC/ARC.
We can wait a compiler fix, or actually handle this.
I think it is possible to avoid forwarding declarations and replace the include
approach with an proper import
one that's more nim style
The leaks are due to the compiler generated code not again due to NimQml. Just use refc since it's the default GC even for Nim 1.6. The use of include has been necessary due to Nim not handling cyclic dependencies and cyclic imports. The NimQml codebase has changed quite a bit since the time i had to switch using includes. Feel free to try but i can assure you that the use includes was due to the compiler not being able to support cycling imports.
I would not accept:
var
argument instead of a refspotted the showstopper I was not expecting:
Error: type bound operation `delete` can be defined only in the same module with its type
This triggers a chain reaction that stops me from willing to refactor the code:
a module re-design without cyclic dependencies is probably possible by splitting the shared code between two modules into a third new module that the two imports, but the requirement to split nimqmltypes.nim
or move all the delete
s in the types modules makes it smell anyway
This is where a dependency graph tools would come handy
Still working on this, I want at least to learn something or end up with a possible improvement
thanks to the help received from nim community, I've learned that is possible to use delayed import to overcome the recursive module dependency issue, and that mostly worked apart from a couple of modules. This has enabled the possibility to return all type declarations to each module, and replace include with imports.
The show stopper is now this pattern that wants user to call parent destructor/finalizer
type
MyListModel* = ref object of QAbstractListModel
names*: seq[string]
proc delete(self: MyListModel)
proc setup(self: MyListModel)
proc newMyListModel*(): MyListModel =
new(result, delete)
result.names = @["John", "Max", "Paul", "Anna"]
result.setup
proc delete(self: MyListModel) =
self.QAbstractListModel.delete
proc setup(self: MyListModel) =
self.QAbstractListModel.setup
minized:
# --gc:refc OK
# --gc:arc Error: cannot bind another '=destroy' to: Child:ObjectType; previous declaration was constructed here implicitly: finalizer1.nim(11, 3)
type
Parent = ref object of RootObj
Child = ref object of Parent
proc delete(self: Parent) =
echo "delete parent"
proc delete(self: Child) =
self.Parent.delete()
proc newChild: Child =
new(result, delete) # error here
that I have yet to find a way to prevent breaking changes, even by splitting logic between ARC/ORC and refc at compile time. Here a full example on how to call contructor/destructor hierarchy in both cases
type
ParentObj = object of RootObj
Parent = ref ParentObj
ChildObj = object of ParentObj
Child = ref ChildObj
proc setup(self: Parent)
proc setup(self: Child)
when defined(gcDestructors):
proc `=destroy`(self: var ParentObj) = echo "destroy Parent"
proc `=destroy`(self: var ChildObj) =
echo "destroy Child"
self.ParentObj.`=destroy`()
proc newParent: Parent =
result = new(Parent)
result.setup()
proc newChild: Child =
result = new(Child)
result.Parent.setup()
result.setup()
else:
proc delete(self: Parent) = echo "delete parent"
proc delete(self: Child) =
echo "delete child"
self.Parent.delete()
proc newParent: Parent =
new(result, delete)
result.setup()
proc newChild: Child =
new(result, delete)
result.Parent.setup()
result.setup()
proc setup(self: Parent) = echo "setup Parent"
proc setup(self: Child) = echo "setup Child"
proc test =
discard newChild()
test()
GC_fullCollect()
In the meantime I suggest to use defined(gcDestructors)
to trigger compile error if ARC/ORC is used, as nimqml it will silently leak memory and trigger hard-to-find problems otherwise
we can close this one with recent nim versions
is nimqml tested for orc on nim 2.0?
Following https://github.com/filcuc/nimqml/issues/42
Using forward declaration of finalizer (
delete
) function causes bug that silently skips finalizer call with--gc:arc
Possible solutions:I see that the code does not follow
import
pattern but is just a single large module. I think it would make it easier to think about the code by converting theinclude
intoimport
+export
and handle the dependency treeI'm already doing some experiments on this
case n==0: destroy can be declared on base
non-ref
type and it would be called on ref object dealloc, it is possible to call parent destroy case n==1: finalizer is called on leaf (real) type only and seems not possible to easily call parent finalizer without additional code