Generic values are allocated on the stack during construction

[alexnask]

This issue is discussed in #41

Basically, in code like this:

Foo: class <T> {
    val: T
    init: func (=val)
}

The following C code is generated:

void Foo___defaults___impl(FooClass* this) {
    types__Class___defaults___impl((types__Class*) this);
    this->val = Memory__alloca(this->T->size);
}

The result is that the generic value is actually a dangling pointer.
To solve this, one can currently do:

Foo: class <T> {
    val: __onheap__ T
    init: func (=val)
}

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).

[alexnask]

The proposed fix has the following advantages:

• Backwards compatible with existing codebases that define free functions themselves
• Easy transition for new code, defining __destroy__ instead of free
• Can easily be extended to additional memory management features (for example, if owned pointers were a thing, they could be automatically freed in the generated 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.

[alexnask]

@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.

[marcusnaslund]

Right, it seems we always have to free (or in ooc-kean actually memfree) the __onheap__ variable, otherwise we leak memory?

[alexnask]

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.

[alexnask]

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.

[marcusnaslund]

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").

[alexnask]

@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).

[marcusnaslund]

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).

[alexnask]

@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.

[marcusnaslund]

That makes sense. In that case the auto free sounds perfectly reasonable to me. @thomasfanell knows more about rock than I do, though.

[alexnask]

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.

[marcusnaslund]

Awesome.

[alexnask]

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!