Prevent forwarding from being "too liberal"

I've implemented a function for performing an "in-place object recycle" hack based on the idea of the deplorable just-plain-wrongheadedness assignment-by-placement-new example shown in GotW #23 and referred to in #28.

(Yes, yes, I know... but that hack has actually shown quite useful as a very noticeable optimization in an application which processes a lot of messages, and I guess it's safe enough if used correctly.)

The code compiles without warnings on clang++ 3.5 and "works fine", but it is somewhat too liberal in my opinion as it allows some implicit conversions that might happen accidentially and which may be undesirable:

#include <type_traits>

template<typename T>
typename std::enable_if<std::is_nothrow_move_constructible<T>::value, T&>::type
recycle(T& obj, T&& other)  noexcept
{ obj.~T(); new (&obj) T(other);  return obj; }

template<typename T, typename... A>
typename std::enable_if<std::is_nothrow_constructible<T>::value && std::is_nothrow_move_constructible<T>::value, T&>::type
recycle(T& obj, A&&... arg) noexcept
{ obj.~T(); new (&obj) T(arg...); return obj; }


int main()
{
    struct foo
    {
        foo() noexcept {}
        explicit foo(float) noexcept {}
    };

    foo b;
    recycle(b, foo());     // OK, calls #1, move-constructing from default temporary
    recycle(b, foo(1.0f)); // OK, as above, but non-default temporary
    recycle(b, 5.0f);      // OK, calls #2, forwarding and move-constructing
    recycle(b, 5.0);       // succeeds calling #2 (undesired implicit double --> float conversion)
    recycle(b, 5);         // succeeds calling #2 (undesired implicit int ---> float conversion)
    recycle(b, b);         // succeeds, but should probably fail a runtime check (undefined behavior)

    return 0;
}

The fact that some calls compile just fine when you wouldn't expect them to may be because the template parameter pack catches "everything", but I'm still surprised how it can work alltogether since foo's constructor is explicit, and either way the template must call it. I'm not sure how it can convert integers and doubles to floats there without clang++ emitting even as much as a warning.

This makes me wonder whether I am assuming something wrong about what the explicit keyword does on a constructor.

Self-assignment (which will invoke undefined behavior) is a non-issue for me since trying to recycle an object with itself shows some serious brain failure in the first place (so I've decided not to add a runtime check for that), but accidentially calling a function with an implicitly convertible parameter is something that could happen pretty easily, and something I'd like to catch with a compiler error if that's possible.

Basically what I might need is something like get_constructor_args<T> to put inside another enable_if with is_convertible. Or something... concepts?

Is there a way of achieving this kind of thing with C++11 (or C++14, if you will)?

Solution

template<class T, class...Args>
using safe_to_recycle = std::integral_constant<bool,
  std::is_nothrow_constructible<T, Args...>::value
  && std::is_nothrow_destroyable<T>::value
>;


template<typename T, typename... Args>
std::enable_if_t<safe_to_recycle<T,Args...>::value, T&>
recycle(T& obj, Args&&... args)  noexcept
{
  obj.~T();
  new (&obj) T{std::forward<Args>(args)...};
  return obj;
}

template<typename T, typename... Args>
std::enable_if_t<safe_to_recycle<T,Args...>::value, T&>
recycle_explicit(T& obj, Args&&... args)  noexcept
{
  obj.~T();
  new (&obj) T(std::forward<Args>(args)...);
  return obj;
}

template<typename T, typename U>
std::enable_if_t<
  std::is_same<std::decay_t<T>,std::decay_t<U>>::value &&
  safe_to_recycle<T,U>::value,
  T&
>
recycle(T& obj, U&& rhs)  noexcept
{
  if (&obj == &rhs) return obj;
  obj.~T();
  new (&obj) T{std::forward<U>(rhs)};
  return obj;
}

template<typename T, typename U>
std::enable_if_t<
  std::is_same<std::decay_t<T>,std::decay_t<U>>::value &&
  safe_to_recycle<T,U>::value,
  T&
>
recycle_explicit(T& obj, U&& rhs)  noexcept
{
  if (&obj == &rhs) return obj;
  obj.~T();
  new (&obj) T(std::forward<U>(rhs));
  return obj;
}

If you lack C++14, write your own enable_if_t and decay_t.

You can drop that run time check if you want. Just eliminate the last 2 recyles. The variardic will handle move assignment etc.

Note that an explicit constructor will not be called by recycle -- you have to recycle( obj, T(5.0f) ). And recycle_explicit will do narrowing conversions.