
Hello, I looked at the review, at the code and some time ago I tried to implement my own variant. In my opinion in variant2 the individual parts are more worth than the whole. Most implementation details would be of great use for implementing variants with a differnt trade-off. Hence the library ought to be split into several independend parts: * a typing switch * a variadic union * an extendable visitation mechanism * a variant view * related type traits * several implementation of a concrete variant 1. A typing switch The typing switch would be provide a litle bit more than (and can be implementated with help of) mp_with_index. The following pseudo code shows the implementation idea: template<size_t MaxIndex, class Function, class Cast, class Object> constexpr decltype(auto) typing_switch(size_t index, Function fun, Cast cast, Object obj) { return fun(cast(integral_constant<size_t, index>{}, obj)); } The object is a variant and the cast transfroms this special variant to a concrete value (with type). The function is provided by the variant user. The visit function (line 1732) would become something like this: template<class Variant> struct variant_cast { template<class Index> constexpr decltype (auto) operator()(Index, Variant& V) { return unsafe_get<Index::value, V>(); } }; template<class Fun, class Variant> constexpr decltype(auto) visit( F&& f, V1&& v1 ) { return typing_switch<variant_size<V1>>(v.index(), f, variant_cast<V1>, v1); } A simplified deserialisation can be implementated as follows: template<class Types> struct index_to_type { template<class Index> constexpr decltype (auto) operator()(Index, std::nullptr_t) { return mp_identity<mp_at<Types, Index>>{}; } }; template<class Variant, class Archive> Variant deserialize(Archive& ar) { size_t index; ar >> index; Variant f = [&ar](auto type) { decltype(type)::type concrete_element; ar >> concrete_element; return Variant{std::move(concrete_element)}; }; return typing_switch(index, f, index_to_type<Variant>{}, nullptr); } With help of this function an implementation of a visitation function is easy. 2. A varidic union Without doubt a type implementing a varidaic union is a building block for almost every variant implementation. It should be part of the public interface, well tested and with a minimal interface (get<i> and emplace<i>) 3. An extendable visitation mechanism This could be used to implement visitation on multiple different variants. It should use visit on one variant as customization point. 4. A variant view (and a mutable variant view) It could have the following form: template<class... T> class variant_view { void* value; size_t index; public: /* visit ... */ }; It is a little bit like folly/DiscriminatedPtr.h (https://github.com/facebook/folly/blob/master/folly/DiscriminatedPtr.h) but without platform specific code. It can be used for functions which take variants as a const references (or in the mutable form as variants which can change the value but not the actual type). It can be constructed from corresponding variants, values (which are part of T...) and other variant_views. As long as it is constructed from a variant implemented with the variadic union the process can be simple: all members of a union have the same address as the union and the index is equal. For special cases the process can be more complicated. For other variant_views the transformation can be implemented as a runtime lookup in an array and for simple values it is simple. A variant_view can for example be used for functions accepting const references like output and serialisation, or for referencing "polymorphic" objects like nodes in ASTs. 5. Related type traits Meaningfull type traits which can be used in one variant have a good chance be be used in a related one. If one variant want to static assert in case void is one of its types the logic is probably usefull for others. They should be collected. 6. Several implementation of a concrete variant This is the place for variant2. With this building blocks it should be simpler to implement a custom variant. Now lets think of a few use cases: A class contains 3 variants each containing only a few "small" types. Instead of using 3 variants it could pack all indices into one int and construct variant_views as needed. This can reduce the size of the class and saves memory bandwidth. Editing of variants would be of course more complicated. A variant containing a few trivial small types which are used in allmost all cases and one type (T_evil) which is used rarely, is big and can throw in certain situation. The soloution can be a normal variant with the exception of storing T_evil as a smart pointer and dereferences it when visited. A variant with allocator support. In case a user has a solution (or idea) for their variant which lives in shared memory a simple blueprint with generic components like varidic union, a typing switch and useable type traits may be of help. I think requirements for different variant types mentioned in the review could be implemented with some well defined basic building blocks. Beside these thougths I looked at the library. expected.hpp: I tried to compile the following file (containing only one non empty line): #include <boost/variant2/expected.hpp> and it does not compile. It should be a self-contained header. Therefore it should be not part of the library. variant.hpp: Personally (in contrast to other reviewers) I would prefer code more structured in different files. In an ideal world there is for every component a single header file (in clean C++ 17) with minimal dependencies which directes for compiler (and version) specific stuff to a different header. For the future it would result in source code written in newest standard without disturbing influence from ifdefs caused by old buggy compilers. The source has more than 1800 lines with only a few comments, short (1 char) names and very long lines (200 chars and more). For me it is not easy to read (after clang-format it is 2030 lines). Variant alone has more than 450 lines of code. With more shorter files the connection of (and dependencies among) the components would be more visble. I started at the begining of the file and searched for a testcases for monostate and I didn't found one. There are a few testcases inside variant_valueless.cpp which use monostate but none which test it directly. Next variant_size. There are testcases but most deal with variants of type 'variant<> const', 'variant<void>' and the like. Declaring variables of these types results in compile time errors. It is not wrong std::variant gave me the same result, but it is a kind of strange that most tests are implemented in terms of variants wich are ill-formed. On the other hand there are no tests which do not compile. These are two example which should not compile. The template detail::variant_alternative_impl looks like something which copies const, volatile and references. Is there a difference to copy_cv_ref at the end? Could it be used? At the end of the file the comparison is implemented. But operator!= is not implementated in terms of operator== and operator<= and others not in terms of operator<. In case there is a reason for it documentation would be nice. Summarized I think variant2 should NOT be part of boost. I think the focus should be more on reusable components and better documented code. I think most of the code could be very usefull with more descriptive names and test. Regards Jan Herrmann