[boost] [variant2] Review

Subject: [boost] [variant2] Review
From: Damian Jarek (damian.jarek93_at_[hidden])
Date: 2019-04-14 22:46:44

> - What is your evaluation of the design?
Variant2 is clearly a superior replacement for the original variant type in
boost - Boost.Variant and would be very useful for libraries in Boost and
outside of it that are stuck on C++11/14. I strongly believe that
triviality propagation should be supported, especially if Variant2 is to be
used as the implementation detail of a type like `expected`.
As for the hot topic of valuelessness - based on my experience of how
variant types are typically used, I think that an implementation should
loudly object at compile-time to any operation that could put it in a
valueless state - e.g. if emplacing can throw and the type does not have a
noexcept move constructor. However, I see that it could be quite
inconvenient for many users and I consider the fallback mechanism currently
in variant2 to be a good enough compromise.

> - What is your evaluation of the implementation?
I reviewed the implementation and it looks great. As a test, I replaced the
internal variant type in Boost.Beast(beast::detail::variant) with variant2
to compare it against the private lean variant and against std::variant. In
terms of compilation time, I've observed no significant difference between
std::variant and variant2 when compiling the Boost.Beast test suite. Both
were about 10% slower to compile than beast::detail::variant, which can be
attributed to the additional header material which must be parsed by the
compiler. In terms of runtime performance, I have not observed a
significant difference between the 3 variants in terms of requests/second
in a test project I use to optimize Boost.Beast performance. Note that
Boost.Beast HTTP serialization makes heavy use of a variant to enable
sending an HTTP message in as few write calls as possible. As far as I
know, all types stored in the variant were "well behaved", so it did not
test the double-buffering cases.

> - What is your evaluation of the documentation?
The documentation is adequate, however, it could use a few examples for
users unfamiliar with the concept of sum types.

> - What is your evaluation of the potential usefulness of the library?
It is a useful replacement for Boost.Variant in C++11/14 libraries both in
Boost and outside of Boost.

> - How much effort did you put into your evaluation? A glance? A quick
>   reading? In-depth study?
About 6 hours, most of which where spent on the compilation and runtime
benchmarks based on Boost.Beast.

> - Are you knowledgeable about the problem domain?
I have implemented and used a few variant implementations in embedded code.
I have also reimplemented the private and lean beast::detail::variant type
on top of Boost.mp11.

Conclusion:
I recommend that Variant2 is CONDITIONALLY ACCEPTED if:
- documentation is extended with usage examples
- triviality propagation is implemented