call destructors before Object dealloc

[galchinsky]

It obviously reduces amount of memory leaks. The only inherited from Object class that uses destructor is ValueHolder. ~ValueHolder() hasn't been called without this patch.

[jckarter]

The only change necessary should be to make Pointer invoke ~Object rather than dealloc.

[galchinsky]

I tried it first. This makes double delete happen. Virtual methods in destructors are not really virtual and destructors are called in reverse order. That's why calls virtual ~ANode() { ANode::dealloc(); }) + virtual ~Object() { Object::dealloc(); }) cause this:

ANodeAllocator->Deallocate(this);
::operator delete(this); //boom

[jckarter]

Of course, you're right. Looks good then.

[galchinsky]

ANode uses custom allocator, it can't be written so simply. But after writting custom delete it looks more concise.

[galchinsky]

Isn't this the issue why destructors in evaluator are not called? I see the FIXME line

ValueHolder::~ValueHolder()
{
    // FIXME: call clay 'destroy'
    free(this->buf);
}

[jckarter]

@galchinsky, @stepancheg is right that delete this is exactly equivalent to calling the destructor then operator delete. ANode handles its custom deallocator by overriding dealloc, which is necessary because operator delete is not dynamically dispatched.

I'm not sure that the ValueHolder destructor is the right place to invoke evaluator destructors. It would be more consistent to invoke them on scope exit, as is done in runtime codegen.

[galchinsky]

I looked through stackoverflow before patching and found the reference to §12.5.7 of the standard.

"Since member allocation and deallocation functions are static they cannot be virtual. [Note: however, when the cast-expression of a delete-expression refers to an object of class type, because the deallocation function actually called is looked up in the scope of the class that is the dynamic type of the object, if the destructor is virtual, the effect is the same."

operator delete is dinamically dispatched when called from virtual destructor . I checked it using printf.

[jckarter]

I didn't know that, thanks! Guess i still have some lingering brain damage from pre-standard c++ implementations.

[galchinsky]

Mmm. Pull? Anyone?