Open alexnask opened 8 years ago
The proposed fix has the following advantages:
__destroy__
instead of free
free
function) EDIT: However, if a class does not have naked generic values, this method would have a small overhead (one extra function call, since all free
would do is call the user defined __destroy__
function) but this is easily controlled by defining it as free
instead.
@thomasfanell
What do you think about auto-generating free
in addition to removing __onheap__
?
As I mentioned, it would be backwards compatible and save you the hussle of manually freeing your generic values.
Right, it seems we always have to free
(or in ooc-kean actually memfree
) the __onheap__
variable, otherwise we leak memory?
Yes, from what I've seen __onheap__
basically replaces the alloca
call with gc_malloc
, so you always need to free it (not only for Object
/class values), which was the LinkedList bug if I'm not mistaken.
Auto generating free
seems like a win-win scenario to me, since it could be extended in the future, while still remaining backwards compatible.
You can think of generic values as byte arrays for all intents and purposes, with additional typechecking, automatic casting and nicer assignment (auto-memcpy
).
This is why you always need to free them.
which was the LinkedList bug
Exactly right.
This sounds really nice, however I'm now thinking that having to manually free everything except a generic variable simplifies the use of the language for anyone. It might even be more confusing that calling free
results in a double free ("but I'm only freeing it once").
@marcusnaslund
Sure, but generics are supposed to be baked into the language and automatically managed.
For example, seeing the following:
MyCell: class <T> {
val: T
init: func (=val)
}
I think it is reasonable to assume that the generic value will be cleaned up when calling free
, since you never actually allocate val
manually.
EDIT:
To rephrase, how the generic values are handled is an implementation detail, so it is reasonable to assume the implementation takes care of cleaning up their memory (in my opinion).
What actually happens when you do free(val)
? The object that I passed obviously still lives (since I can get()
it from the Cell
or the LinkedListNode
and use it even after they have been freed).
@marcusnaslund
Generic values are place holders, anything you assign to them is (shallow) copied in.
Freeing the generic value actually frees the "place holder", or the shallow copy if you prefer to reason about it that way.
Your actual object still lives.
That makes sense. In that case the auto free sounds perfectly reasonable to me. @thomasfanell knows more about rock than I do, though.
Starting work on this.
It seems really easy to remove since __onheap__
is only used for generic initialization anyways.
Auto generating free
is a little bit more work but should be pretty trivial too.
Awesome.
Having a bit of an issue with multiple evaluation of __destroy__
since all methods are virtual in ooc.
The solution is probably to stop generating __destroy__
calls and rely on Object free to call it once.
That means that not all __destroy__
calls are executed, only the class at the top of the hierarchy, which is probably not ideal.
Probably going to try to find another solution.
EDIT: It seems you can devirtualize a call manually in rock source, yay!
This issue is discussed in #41
Basically, in code like this:
The following C code is generated:
The result is that the generic value is actually a dangling pointer.
To solve this, one can currently do:
And then manually free the generic value.
As discussed in that PR, I believe the best course of action would be to automatically assume
__onheap__
for "naked" (non pointer) generic values and autogenerate a free function that deletes them and calls the user defined__destroy__
function (if a free function is not user defined).