Can this technique for creating a container of het

Question

This blog post describes a technique for creating a container of heterogeneous pointers. The basic trick is to create a trivial base class (i.e. no explicit function declarations, no data members, nothing) and a templated derived class for storing std::function<> objects with arbitrary signatures, then make the container hold unique_ptrs to objects of the base class. The code is also available on GitHub.

I don't think this code can be made robust; std::function<> can be created from a lambda, which might include a capture, which might include a by-value copy of a nontrivial object whose destructor must be called. When the Func_t type is deleted by unique_ptr upon removal from the map, only its (trivial) destructor will be called, so the std::function<> objects never get properly deleted.

I've replaced the use-case code from the example on GitHub with a "non-trivial type" that is then captured by value inside a lambda and added to the container. In the code below, the parts copied from the example are noted in comments; everything else is mine. There's probably a simpler demonstration of the problem, but I'm struggling a bit to even get a valid compile out of this thing.

#include <map>
#include <memory>
#include <functional>
#include <typeindex>
#include <iostream>

// COPIED FROM https://plus.google.com/+WisolCh/posts/eDZMGb7PN6T
namespace {

  // The base type that is stored in the collection.
  struct Func_t {};
  // The map that stores the callbacks.
  using callbacks_t = std::map<std::type_index, std::unique_ptr<Func_t>>;
  callbacks_t callbacks;

  // The derived type that represents a callback.
  template<typename ...A>
    struct Cb_t : public Func_t {
      using cb = std::function<void(A...)>;
      cb callback;
      Cb_t(cb p_callback) : callback(p_callback) {}
    };

  // Wrapper function to call the callback stored at the given index with the
  // passed argument.
  template<typename ...A>
    void call(std::type_index index, A&& ... args)
    {
      using func_t = Cb_t<A...>;
      using cb_t = std::function<void(A...)>;
      const Func_t& base = *callbacks[index];
      const cb_t& fun = static_cast<const func_t&>(base).callback;
      fun(std::forward<A>(args)...);
    }

} // end anonymous namespace

// END COPIED CODE

class NontrivialType
{
  public:
    NontrivialType(void)
    {
      std::cout << "NontrivialType{void}" << std::endl;
    }

    NontrivialType(const NontrivialType&)
    {
      std::cout << "NontrivialType{const NontrivialType&}" << std::endl;
    }

    NontrivialType(NontrivialType&&)
    {
      std::cout << "NontrivialType{NontrivialType&&}" << std::endl;
    }


    ~NontrivialType(void)
    {
      std::cout << "Calling the destructor for a NontrivialType!" << std::endl;
    }

    void printSomething(void) const
    {
      std::cout << "Calling NontrivialType::printSomething()!" << std::endl;
    }
};

// COPIED WITH MODIFICATIONS
int main()
{
  // Define our functions.
  using func1 = Cb_t<>;

  NontrivialType nt;
  std::unique_ptr<func1> f1 = std::make_unique<func1>(
      [nt](void)
      {
        nt.printSomething();
      }
  );

  // Add to the map.
  std::type_index index1(typeid(f1));
  callbacks.insert(callbacks_t::value_type(index1, std::move(f1)));

  // Call the callbacks.
  call(index1);

  return 0;
}

This produces the following output (using G++ 5.1 with no optimization):

NontrivialType{void}
NontrivialType{const NontrivialType&}
NontrivialType{NontrivialType&&}
NontrivialType{NontrivialType&&}
NontrivialType{const NontrivialType&}
Calling the destructor for a NontrivialType!
Calling the destructor for a NontrivialType!
Calling the destructor for a NontrivialType!
Calling NontrivialType::printSomething()!
Calling the destructor for a NontrivialType!

I count five constructor calls and four destructor calls. I think that indicates that my analysis is correct--the container cannot properly destroy the instance it owns.

Is this approach salvageable? When I add a virtual =default destructor to Func_t, I see a matching number of ctor/dtor calls:

NontrivialType{void}
NontrivialType{const NontrivialType&}
NontrivialType{NontrivialType&&}
NontrivialType{NontrivialType&&}
NontrivialType{const NontrivialType&}
Calling the destructor for a NontrivialType!
Calling the destructor for a NontrivialType!
Calling the destructor for a NontrivialType!
Calling NontrivialType::printSomething()!
Calling the destructor for a NontrivialType!
Calling the destructor for a NontrivialType!

... so I think this change might be sufficient. Is it?

(Note: the correctness--or lack thereof--of this approach is independent of whether the idea of a container of heterogeneous functions is a good idea. In a few very specific cases, there may be some merit, for instance, when designing an interpreter; e.g., a Python class may be thought of as just such a container of heterogeneous functions plus a container of heterogeneous data types. But in general, my decision to ask this question does not indicate that I think this is likely to be a good idea in very many cases.)

Answer 1

This is basically someone trying to implement type erasure and getting it horribly wrong.

Yes, you need a virtual destructor. The dynamic type of the thing being deleted is obviously not Func_t, so it's plainly UB if the destructor isn't virtual.

The whole design is completely broken, anyway.

The point of type erasure is that you have a bunch of different types that share a common characteristic (e.g. "can be called with an int and get a double back"), and you want to make them into a single type that has that characteristic (e.g., std::function<double(int)>). By its nature, type erasure is a one-way street: once you have erased the type, you can't get it back without knowing what it is.

What does erasing something down to an empty class mean? Nothing, other than "it's a thing". It's a std::add_pointer_t<std::common_type_t<std::enable_if_t<true>, std::void_t<int>>> (more commonly known as void*), obfuscated in template clothing.

There are plenty of other problems with the design. Because the type was erased into nothingness, it had to recover the original type in order to do anything useful with it. But you can't recover the original type without knowing what it is, so it ends up using the type of arguments passed to Call to infer the type of the thing stored in the map. That is ridiculously error-prone, because A..., which represents the types and value categories of the arguments passed to Call, is highly unlikely to match exactly the parameter types of std::function's template argument. For instance, if you have a std::function<void(int)> stored in there, and you tried to call it with a int x = 0; Call(/* ... */ , x);, it's undefined behavior. Go figure.

To make it worse, any mismatch is hidden behind a static_cast that causes undefined behavior, making it harder to find and fix. There's also the curious design that requires the user to pass a type_index, when you "know" what the type is anyway, but it's just a sideshow when compared to all the other problems with this code.