Clone of Boost Variant

As part of learning C++, with special emphasis on C++11, I wanted to implement the equivalent of Boost's Variant (located here). My code is available at variant.hpp, with the current version given below.

How can std::aligned_storage be used portably? My current solution makes probably non-portable use of static_cast, though if it is portable, that information would be very valuable. The particular code is similar to *static_cast(static_cast(&value)), for value of type typename std::aligned_storage::type (where ... is not meant to indicate variadic templates).

I make some use of static_assert. In this particular use, would SFINAE be better? I understand SFINAE can be used to prune out overloads from the set of viable functions, but where I use static_assert I assume there would be only one viable function, though I would find valuable any examples of cases where there is more than one viable function.

I made much use of std::forward. Is it possible to get by with fewer uses?

I made use of std::enable_if on one of the constructor overloads to ensure that it would only be used when a move is intended (see variant(U&& value, typename detail::variant::enable_if_elem::type = nullptr, typename detail::variant::enable_if_movable::type = nullptr)). Without both of the enable_ifs, this contructor would be used when the copy constructor variant(variant const&) is instead intended, even though the former results in an eventual compiler error. Is there a better way to force this behavior? One solution I tried was including variant(variant&) as an overload that just delgates to variant(variant const& rhs) - it would be selected over variant(U&&), while variant(U&&) is preferred over variant(variant const&) by the overload rules. What is the general best practice when using T&& for some newly introduced T when move semantics, instead of a universal reference, are intended?

I still need to add multivisitors, though I am havin

There's a lot here, so I'm going to split my review into pieces. I want to start by just focusing on the metafunction section. Metafunctions may be short, but they're very powerful and important to get right - but in terms of correctness and usefulness.

To start with:

template 
using is_movable = typename std::integral_constant
    ::value && !std::is_const::value>;

template 
using enable_if_movable = std::enable_if::value, U>;

The first one is simply wrong. You're using this metafunction to check if a type is move constructible (in move_construct)... but you're doing this by just checking if it's neither an lvalue reference nor const. You're not actually checking anything relating to move construction. Just because something is an rvalue reference does not mean that you can move from it. And just because something is an lvalue reference does not mean that you cannot. Consider two simple classes:

struct No {
    A(A&& ) = delete;
};

struct Yes { };

As the name suggests, No is not move constructible. Your metafunction says it is. Also, Yes& is move constructible but your metafunction says no.

The correct implementation would be to simply use the standard type trait std::is_move_constructible.

Secondly, the alias there is questionable. Typically, we'd use aliases to avoid having to write the typename ::type cruft. You're not doing that, and the resulting call isn't that much more concise. Compare:

typename enable_if_movable::type      // yours with alias
std::enable_if_t::value>  // just using enable_if without alias
std::enable_if_t::value> // just stds

I would prefer the last version personally. Note that I'm using the C++14 alias here. If you don't have a C++14 compiler, it is absolutely worth it to start your metafunction library with all of them. They are simply to write:

template 
using enable_if_t = typename std::enable_if::type;

Moving onto:

template 
struct elem;

There is no way that anybody will know what elem does here. I didn't until I read the implementation. A much better name for this would be contains. But I'll get back to implementation in a moment.

First, let's start with:

template 
struct all;

all is super useful. So are its close relatives any and none. The way you wrote all is fine and works, but doesn't make it easier to write the other two. A good way of writing these out is to use @Columbo's bool_pack trick:

template  struct bool_pack;

template 
using all_same = std::is_same, bool_pack>;

That's just your helper. You can use that to implement all the rest easily:

template 
using all_of = all_same;

template 
using none_of = all_same;

template 
using any_of = std::integral_constant::value>;

And once we have that, we can reimplement contains as a one-liner:

template 
using contains = any_of::value...>;

Similarly to before, I don't see the value in enable_if_elem. And common_result_of should take the type, not just yield the metafunction:

template 
using common_result_of =
    std::common_type_t::...>;

Although it's more readabile to just stick that in your variant itself:

// no need to use anything in detail::, unless you need to 
// write your own aliases for common_type_t and result_of_t
template 
using result_of = std::common_type_t...>;

Now onto the usage. Throughout, you use these metafunctions in the return type:

template 
typename enable_if_movable::type operator()(T&& value);

Or as a dummy pointer:

template 
variant(U const& value,
        typename detail::variant::enable_if_elem::type* = nullptr)

But in both cases, I find it a lot easier to parse complex template expressions if you put the SFINAE logic as an unnamed final template parameter:

template ::value>
          >
void operator()(T&& value);

template ::value>
          >
variant(U const& value);

The consistency helps understanding too. The dummy pointer argument is a confusing hack leftover from C++03. There's no need for it anymore. Especially when you need two dummy pointers:

template ::value &&
                                       std::is_move_constructible::value>
           >
 variant(U&& value);

Side note on this guy. It doesn't actually do what you want. This isn't an arbitrary rvalue reference - it's a forwarding reference. In fact, we can even combine the two constructors here in one go:

template ,
          typename = std::enable_if_t::value &&
                                      std::is_constructible::value>
          >
variant(U&& value)
: which_{elem_index::value}
{
    new (&storage_) V(std::forward(value));  
}

Note that this also solves another problem with your code - namely that you never check if a class is copy constructible. What if you wanted to stick something like unique_ptr in your variant. Move constructible, but not copy constructible - but you never checked that in your code. Important - o

StackExchange Code Review Q#43230, answer score: 2