Destructors and inheritance; risk of UB about sketchup-api-c-wrapper HOT 5 OPEN

Thank you for catching this. Changes committed. It is correct to be a virtual and public destructor, for polymorphism.

from sketchup-api-c-wrapper.

In this case, then maybe a protected non-virtual destructor is appropriate. Are there any usecases for creating any of these wrapper objects on the heap (using new)? They are essentially opaque pointers over the SU API refs (which is again opaque pointers).

Adding virtual to this introduce the vtable, eating more memory per object. If these objects are only ever used without being allocated by new - or to be more precise, without being deleted by a base pointer, then leaving it non-virtual but protected might be a more suitable option.

from sketchup-api-c-wrapper.

I have not found any use cases for creating wrapper objects on the heap using new. However, if I make the destructor protected non-virtual, then I have issues with Temporary Objects, which I do use.

For example, this bit of code fails to compile when I have protected Entity destructor:

Entity RubyAPI::entity_from_ruby(const RUBY_VALUE& ruby_entity) {
  SUEntityRef entity_ref = SU_INVALID;
  SU_RESULT res = SUEntityFromRuby(ruby_entity, &entity_ref);
  // ...Skip error checking....
  return Entity(entity_ref, true); // Fails to compile as a temporary object needs to have a non-protected destructor.
}

I don't know as much as yourself about C++ @thomthom, but from what I understand the additional memory cost for the vtable is not large, and the additional memory is per class and not per object ( https://stackoverflow.com/questions/1626290/c-virtual-function-table-memory-cost ). There is an additional pointer per instance to the vtable however. If that is the case, I don't think it is something to worry about too much?

If there are ways to get around the temporary objects issue but use protected destructors, I'm certainly open to avoiding virtual methods.

from sketchup-api-c-wrapper.

hm... I don't see why a temporary needs a non-protected dtor. I'll have to have a closer look.

from sketchup-api-c-wrapper.

Do you have an example of issues with temporary objects when the dtor is protected?

from sketchup-api-c-wrapper.