
I. DESIGN --------- The main design decision of Variant2 is the never-empty guarantee, which is relevant when replacing the value of a variant. This is done without overhead if the alternative types are "well-behaving"; if they are not, then Variant2 falls back to using double storage. This is a major difference to: * std::variant which enters an invalid (valuless_by_exception) state. While that may be appropriate for some use cases, there are others where it is not. For instance, the variant may be used for a state machine where the alternative types are function object types to be invoked when events arrive. Having a valueless state is the wrong choice for this use case. * Boost.Variant which uses a temporary heap object instead. Avoiding this internal heap allocation is sufficient grounds for another never-empty variant type. In time critical applications heap allocation not only introduces indeterministic timing, but may also incur priority inversion due to thread synchronization within the heap allocator. Variant2 has several ways to ensure the never-empty guarantee: V1. If all alternative types have non-throwing assignment, the variant cannot fail during re-assignment. V2. If an explicit null state is available, then this is chosen on re-assignment error. This is similar to std::variant, except is is opt-in. V3. If one of the alternative types has a non-throwing default constructor, then this is chosen on re-assignment failure. V4. Otherwise, the variant will remain it its old state (due to double storage) if re-assignment fails. Criterion V3 is possibly surprising because the variant may change to another type not anticipated by the user. This should be opt-in instead (e.g. using a tag.) I am missing a function to get the index of a type to work with the index() function. Something similar to holds_alternative() but that returns the index instead of a boolean. This useful in switch statements, when we want to avoid visitors for some reason: variant<int, float> var = 1; switch (var.index()) { case index_of<int>(var): /* Do int stuff */ break; case index_of<float>(var): /* Do float stuff */ break; } It would also be beneficial for performance reasons to add accessors that have a narrow-contract, similar to relaxed_get() in Boost.Variant. The implementation is already there (detail::unsafe_get.) Better compiler error messages would be desirable. For instance, if we use an illegal visitor that has different return types, then the compiler error does not give us any hint about the different return types. II. IMPLEMENTATION ------------------ The implementation is high quality. Although it does lots of meta-programming, most of the style is simple and easy to follow, especially if you have a rudimentary knowledge of Boost.Mp11. In a few places, like the visit() return type, the meta-programming is more hardcore. variant<T...> defines _get_impl as a public "private" function. This can be made more safe by using detail::unsafe_get instead of _get_impl throughout the code. detail::unsafe_get will continue to call _get_impl, but the former should be made a friend of variant<T...> such that _get_impl can be made private. The other public "private" function, variant<T...>::_real_index(), does not seem to be used and can be removed. The emplace_impl(mp_false, mp_false, ...) contains two distinct cases that depend on a compile time condition. If this is split in two, then the two assert()s can be changed into static_assert()s. The variant_alternative and get functions have a couple of compiler workarounds. The code could be made clearer if the workaround is always used for all compilers. Or does is this related to C++11 constexpr? III. DOCUMENTATION ------------------ The documentation is mainly aimed at experienced users. There is no tutorial for people unfamiliar with variant types, nor is there a design rationale that explains why Variant2 is exists when we already have Boost.Variant and std::variant. The reference documentation is adequate, although a bit terse. IV. MISC -------- Having written my own variant class (that has a maximum size, and any type exceeding the limit is allocated on heap) puts me in a good position to assess Variant2. I was also the review manager of Boost.Mp11 upon which Variant2 is built. I have spent 5-6 hours reading documentation, reviewing the code, and writing small test programs. V. VERDICT ---------- Variant2 fills a hole not covered by Boost.Variant and std::variant. I vote to CONDITIONALLY ACCEPT Variant2 into Boost, provided that the following change be made: 1. The selection of an arbitrary non-throwing type in case of an re-assignment failure (criterion V3 in the DESIGN section above) should be made optional. By default it should use double storage in this case. I would furthermore like to see improvements to the documentation, as well as the index_of/safe_get functionality, but neither is a requirement for acceptance. I have not reviewed expected<T, E...>.