3

So gcc 4.9.0 has implemented a static_assertion that the type is not void to be correctly conforming to the standard. Which is great, all for standards conformance.

I have a variant type that stored the data under a std::unique_ptr<void> which now doesn't work. The easy fix is to just change it to a std::shared_ptr<void> and it compiles straight away. The better fix is to provide the deleter functor.

Is the following a safe way of fixing up the unique_ptr? Will this exhibit undefined behaviour with certain polymorphic types?

#include <memory>
#include <iostream>

template<typename T>
void Deleter(void * p) {
  delete reinterpret_cast<T*>(p);
}

int main() {
  const std::unique_ptr<void, void(*)(void*)> v(new int(199), Deleter<int>);
  std::cout << *reinterpret_cast<int*>(v.get()) << std::endl;
  return 0;
}
Community
  • 1
  • 1
Matt Clarkson
  • 14,106
  • 10
  • 57
  • 85
  • 1
    Wouldn't it be easier (in the long run anyway) and safer to use e.g. [Boost variant](http://www.boost.org/doc/libs/1_55_0/doc/html/variant.html) or [Boost any](http://www.boost.org/doc/libs/1_55_0/doc/html/any.html)? – Some programmer dude May 06 '14 at 12:15
  • why not use a proper variant type (like [`boost::variant`](http://www.boost.org/doc/libs/release/doc/html/variant.html) or your own implementation) ? `void*` should be used as little as possible imo. – Sander De Dycker May 06 '14 at 12:16
  • I do have my own variant type, that's what I've implemented. I would usually use a boost variant but my variant needs to be serializable. The type identification numbers that are derived form `MurmurHash3` and serialization functions are stripped. This makes the variant marshallable across processes/devices which is needed for my current use case. – Matt Clarkson May 06 '14 at 12:24
  • @MattClarkson: using `reinterpret_cast` here is asking for troubles, I advise against using this with polymorphic types... – Matthieu M. May 06 '14 at 12:45
  • @MatthieuM., thanks, have changed to use `static_cast` as per Jonathan's advice. – Matt Clarkson May 06 '14 at 13:00
  • @MattClarkson: I would point out that you are still not out of the woods, even with `static_cast`, the only valid transformation with `void*` is `T*` -> `void*` -> `T*`, if you cast back to anything else than the original type, you have trouble brewing. – Matthieu M. May 06 '14 at 15:05
  • I am casting back to the original type, guaranteed. The `Deleter` is instantiated with the same type as the incoming type. – Matt Clarkson May 06 '14 at 15:19

1 Answers1

9

Will this exhibit undefined behaviour with certain polymorphic types?

If you use unique_ptr<void, void(*)(void*)>(new X(...), Deleter<X>) that will be correct for any X, because the deleter uses the correct dynamic type of the object.

If you used unique_ptr<void, void(*)(void*)>(new Derived(...), Deleter<Base>) that will have undefined behaviour (for both polymorphic and non-polymorphic types) if Base does not have a virtual destructor, or if the Base sub-object is not at the same address as the Derived object that contains it.

However, you should use static_cast not reinterpret_cast in both places. You should prefer to use the weakest cast that will work for a given situation, reinterpret_cast is a sledgehammer.

To make the code less error-prone I would write a helper function so you only ever name the type once, something like:

template<typename T, typename... Args>
  std::unique_ptr<void, void(*)(void*)>
  make_unique_void_ptr(Args&&... args)
  {
    using Ptr = std::unique_ptr<void, void(*)(void*)>;
    return Ptr{ new T(std::forward<Args>(args)...), Deleter<T> };
  }
Jonathan Wakely
  • 166,810
  • 27
  • 341
  • 521
  • I am using your first example. Thanks for a great answer (as always). – Matt Clarkson May 06 '14 at 12:21
  • 1
    I believe the second case is always Undefined Behavior, even if `Base` has a virtual destructor, simply because you cannot expect that `Derived*` -> `void*` -> `Base*` behave in a sane way since there is no guarantee that `(void*)derived == (void*)base` in general (even though with gcc if this is the first base it will work). – Matthieu M. May 06 '14 at 12:43
  • 1
    @MatthieuM. good point, but that doesn't mean it's _always_ UB, only when `(void*)derived != (void*)base`. – Jonathan Wakely May 06 '14 at 15:23
  • That UB could be avoided by doing `unique_ptr(static_cast(new Derived(...)), Deleter)`, then it would only have UB if `~Base()` is non-virtual. But at that point you might as well just use `Deleter` and avoid the problem entirely :) – Jonathan Wakely May 06 '14 at 15:27
  • @JonathanWakely: I believe that unless the conversion is `T* -> void* -> T*` the Standard regards the cycle as UB. In practice, it so happens that it only breaks if `(void*)derived != (void*)base`, but I would loathe relying on it as it's implementation dependent (ABI). – Matthieu M. May 06 '14 at 16:22
  • I don't think there's any UB if the addresses are the same. [expr.static.cast]/13: "If the original pointer value represents the address A of a byte in memory and A satisfies the alignment requirement of T, then the resulting pointer value represents the same address as the original pointer value, that is, A. The result of any other such pointer conversion is unspecified." – Jonathan Wakely May 06 '14 at 17:02
  • Also [basic.compound]/3 "If an object of type `T` is located at an address `A`, a pointer of type _cv_ `T*` whose value is the address `A` is said to point to that object, regardless of how the value was obtained." – Jonathan Wakely May 08 '14 at 19:22