[review][variant2] Variant2 Review Starts April 1
The Boost formal review of Peter Dimov's Variant2 library will take place April 1 - 10, 2019. Please consider participating in this review. The success of Boost is partially a result of the quality review process which is conducted by the community. You are part of the Boost community. I will be grateful to receive a review based on whatever level of effort or time you can devote. Variant2 is a never-valueless C++11/14/17 implementation of std::variant.
From the README:
The class boost::variant2::variant<T...> is an almost conforming implementation of std::variant with the following differences: * A converting constructor from, e.g. variant<int, float> to variant<float, double, int> is provided as an extension; * The reverse operation, going from variant<float, double, int> to variant<int, float> is provided as the member function subset<U...>. (This operation can throw if the current state of the variant cannot be represented.) * variant<T...> is not trivial when all contained types are trivial. To avoid going into a valueless-by-exception state, this implementation falls back to using double storage unless * one of the alternatives is the type monostate, * one of the alternatives has a nonthrowing default constructor, or * all the contained types are nothrow move constructible. If the first two bullets don't hold, but the third does, the variant uses single storage, but emplace constructs a temporary and moves it into place if the construction of the object can throw. In case this is undesirable, one can force emplace into always constructing in- place by adding monostate as one of the alternatives. You can find the source code here: <https://github.com/pdimov/variant2> and the documentation here: <https://pdimov.github.io/variant2/doc/html/variant2.html> Please take an early look at the library and ask questions now. Peter Dimov is very active on the Cpplang Slack #boost channel; however, please consider using the Boost Dev or User Mail Lists so that the entire community might benefit from your conversation. Note: The repository contains an expected implementation also; however, that is not being considered in this review. Thank you for your participation! michael -- Michael Caisse Ciere Consulting ciere.com
Hi Michael, Thanks for the announcement ahead of time. I have found a ten-day period always to be too short for doing a review, so I appreciate any opportunity to start earlier. My question to you and peter is what exactly are we going to review. The referenced GitHub repository has at least two branches: master and develop. In case they differ, which is the one we are supposed to be reviewing. In the same vein, is the linked documentation exactly what we are going to review? Documentation is also subject to the review process. Peter mentioned he is going to change the docs so that they reflect how the never-valueless guarantee is implemented. Is that change going to be reflected in the documentation version that is going to be reviewed? Regards, &rzej; niedz., 24 mar 2019 o 22:12 Michael Caisse via Boost <boost@lists.boost.org> napisał(a):
The Boost formal review of Peter Dimov's Variant2 library will take place April 1 - 10, 2019.
Please consider participating in this review. The success of Boost is partially a result of the quality review process which is conducted by the community. You are part of the Boost community. I will be grateful to receive a review based on whatever level of effort or time you can devote.
Variant2 is a never-valueless C++11/14/17 implementation of std::variant.
From the README:
The class boost::variant2::variant<T...> is an almost conforming implementation of std::variant with the following differences:
* A converting constructor from, e.g. variant<int, float> to variant<float, double, int> is provided as an extension;
* The reverse operation, going from variant<float, double, int> to variant<int, float> is provided as the member function subset<U...>. (This operation can throw if the current state of the variant cannot be represented.)
* variant<T...> is not trivial when all contained types are trivial.
To avoid going into a valueless-by-exception state, this implementation falls back to using double storage unless
* one of the alternatives is the type monostate,
* one of the alternatives has a nonthrowing default constructor, or
* all the contained types are nothrow move constructible.
If the first two bullets don't hold, but the third does, the variant uses single storage, but emplace constructs a temporary and moves it into place if the construction of the object can throw. In case this is undesirable, one can force emplace into always constructing in- place by adding monostate as one of the alternatives.
You can find the source code here: <https://github.com/pdimov/variant2>
and the documentation here: <https://pdimov.github.io/variant2/doc/html/variant2.html>
Please take an early look at the library and ask questions now. Peter Dimov is very active on the Cpplang Slack #boost channel; however, please consider using the Boost Dev or User Mail Lists so that the entire community might benefit from your conversation.
Note: The repository contains an expected implementation also; however, that is not being considered in this review.
Thank you for your participation! michael
-- Michael Caisse Ciere Consulting ciere.com
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On 3/25/19 01:31, Andrzej Krzemienski via Boost wrote:> Hi Michael,
niedz., 24 mar 2019 o 22:12 Michael Caisse via Boost <boost@lists.boost.org> napisał(a):
The Boost formal review of Peter Dimov's Variant2 library will take place April 1 - 10, 2019.
<snip announcement>
My question to you and peter is what exactly are we going to review. The referenced GitHub repository has at least two branches: master and develop. In case they differ, which is the one we are supposed to be reviewing.
In the same vein, is the linked documentation exactly what we are going to review? Documentation is also subject to the review process. Peter mentioned he is going to change the docs so that they reflect how the never-valueless guarantee is implemented. Is that change going to be reflected in the documentation version that is going to be reviewed?
The mater branch will be stable for the review. The documentation comes from the master branch and will therefore be stable also. Peter is currently making changes to develop and will merge those to master for an April 1st review date. The official start-of-review announcement will contain additional details such as review criteria. Thank you for the questions and the early start on getting familiar with the library. michael -- Michael Caisse Ciere Consulting ciere.com
On 3/24/19 4:04 PM, Michael Caisse via Boost wrote:
The Boost formal review of Peter Dimov's Variant2 library will take place April 1 - 10, 2019.
[snip] The documents here: https://pdimov.github.io/variant2/doc/html/variant2.html#ref_visit suggest visit can take any number of arguments. However, the attached fails to compile when defined(ARGS5). The user docs should make clear that there is a limit to the number of arguments. -regards, Larry
Larry Evans wrote:
The documents here:
https://pdimov.github.io/variant2/doc/html/variant2.html#ref_visit
suggest visit can take any number of arguments. However, the attached fails to compile when defined(ARGS5). The user docs should make clear that there is a limit to the number of arguments.
The error has nothing to do with number of arguments. The function object needs to be callable with any combination of types; the first variant, for instance, may hold a string, and therefore, the function object must be able to accept a string as its first argument.
On 4/6/19 8:44 AM, Peter Dimov via Boost wrote:
Larry Evans wrote:
The documents here:
https://pdimov.github.io/variant2/doc/html/variant2.html#ref_visit
suggest visit can take any number of arguments. However, the attached fails to compile when defined(ARGS5). The user docs should make clear that there is a limit to the number of arguments.
The error has nothing to do with number of arguments. The function object needs to be callable with any combination of types; the first variant, for instance, may hold a string, and therefore, the function object must be able to accept a string as its first argument.
I only drew that conclusion because the number of args seems the only difference between the test that compiles OK and the one that doesn't. The code was copy&pasted from: https://github.com/pdimov/variant2/blob/develop/test/variant_visit.cpp#L105 and the function object there only accepts `int` as it's first argument. So, does that compile because any of the alternative types can be converted to `int`? Hmmm. Changed the all types of the lamba args to auto and it still fails to compile :( Why is that? It would illustrate better how to use `visit` if you provided some examples other than those in the test directory. I'm also having a hard time understanding how the implementation at https://github.com/pdimov/variant2/blob/develop/include/boost/variant2/varia... works. Maybe some in source comments would help there. -regards, Larry
AMDG On 4/6/19 9:01 AM, Larry Evans via Boost wrote:
<snip> I only drew that conclusion because the number of args seems the only difference between the test that compiles OK and the one that doesn't. The code was copy&pasted from:
https://github.com/pdimov/variant2/blob/develop/test/variant_visit.cpp#L105
and the function object there only accepts `int` as it's first argument. So, does that compile because any of the alternative types can be converted to `int`?
Yes.
Hmmm. Changed the all types of the lamba args to auto and it still fails to compile :( Why is that?
The error is different, though. It fails because the function body doesn't compile with every combination of types. In Christ, Steven Watanabe
Larry Evans wrote:
I only drew that conclusion because the number of args seems the only difference between the test that compiles OK and the one that doesn't. The code was copy&pasted from:
https://github.com/pdimov/variant2/blob/develop/test/variant_visit.cpp#L105
and the function object there only accepts `int` as it's first argument. So, does that compile because any of the alternative types can be converted to `int`?
Yes.
Hmmm. Changed the all types of the lamba args to auto and it still fails to compile :( Why is that?
If we're talking about this program: #include <boost/variant2/variant.hpp> #include <string> using namespace boost::variant2; int main() { variant<int, float, double, char, std::string> v1( 1 ); variant<int, float, double, char, std::string> const v2( 3.14f ); variant<int, float, double, char, std::string> v3( 6.28 ); variant<int, float, double, char, std::string> const v4( 'A' ); variant<int, float, double, char, std::string> const v5( "xxx" ); visit( []( auto x1, auto x2, auto x3, auto x4, auto x5 ){}, v1, v2, v3, v4, v5 ); } it compiles with g++/clang++ -std=c++14, and doesn't with -std=c++11. It also doesn't compile on VS2017 but compiles on VS2019. That's because the unlimited argument implementation is disabled on C++11 or VS2017 here: https://github.com/pdimov/variant2/blob/develop/include/boost/variant2/varia... and, you're right, the workaround only supports up to four arguments.
On 4/6/19 10:51 AM, Peter Dimov via Boost wrote:
Larry Evans wrote:
I only drew that conclusion because the number of args seems the only difference between the test that compiles OK and the one that doesn't. The code was copy&pasted from:
https://github.com/pdimov/variant2/blob/develop/test/variant_visit.cpp#L105
and the function object there only accepts `int` as it's first argument. So, does that compile because any of the alternative types can be converted to `int`?
Yes.
Hmmm. Changed the all types of the lamba args to auto and it still fails to compile :( Why is that?
If we're talking about this program:
#include <boost/variant2/variant.hpp> #include <string>
using namespace boost::variant2;
int main() { variant<int, float, double, char, std::string> v1( 1 ); variant<int, float, double, char, std::string> const v2( 3.14f ); variant<int, float, double, char, std::string> v3( 6.28 ); variant<int, float, double, char, std::string> const v4( 'A' ); variant<int, float, double, char, std::string> const v5( "xxx" );
visit( []( auto x1, auto x2, auto x3, auto x4, auto x5 ){}, v1, v2, v3, v4, v5 ); }
it compiles with g++/clang++ -std=c++14, and doesn't with -std=c++11. It also doesn't compile on VS2017 but compiles on VS2019. That's because the unlimited argument implementation is disabled on C++11 or VS2017 here:
https://github.com/pdimov/variant2/blob/develop/include/boost/variant2/varia...
and, you're right, the workaround only supports up to four arguments.
Thanks for clearing that up. However, I did some more exploring, and with the attached: when only defined(UNSIGNED_I4), it compiles within a few secs. when only defined(UNSIGNED_I5), it times several seconds consuming 100% of one of my cpu's, but eventually compiles. OTOH, when onely UNSIGNED_I5) is defined, it never terminates: --{--cut here-- -*- mode: compilation; default-directory: "~/prog_dev/boost/releases/ro/boost_1_69_0/sandbox/pdimov/variant2/review/" -*- Compilation started at Sat Apr 6 12:13:45 make -k run /usr/bin/clang++ -c -O0 -gdwarf-2 -std=c++17 -ftemplate-backtrace-limit=0 -fdiagnostics-show-template-tree -fno-elide-type -fmacro-backtrace-limit=0 -I../variant2/include -I/home/evansl/prog_dev/boost/releases/ro/boost_1_69_0 -ftemplate-depth=200 visit_unsigned_i.cpp -MMD -o /tmp/build/clangxx6_0_pkg/boost/releases/ro/boost_1_69_0/sandbox/pdimov/variant2/review/visit_unsigned_i.o /home/evansl/prog_dev/root.imk:182: recipe for target '/tmp/build/clangxx6_0_pkg/boost/releases/ro/boost_1_69_0/sandbox/pdimov/variant2/review/visit_unsigned_i.o' failed make: *** [/tmp/build/clangxx6_0_pkg/boost/releases/ro/boost_1_69_0/sandbox/pdimov/variant2/review/visit_unsigned_i.o] Interrupt Compilation interrupt at Sat Apr 6 12:14:33 --}--cut here-- Have you any idea why? -regards, Larry
On 4/6/19 12:20 PM, Larry Evans via Boost wrote: [snip]
However, I did some more exploring, and with the attached:
when only defined(UNSIGNED_I4), it compiles within a few secs. when only defined(UNSIGNED_I5), it times several seconds consuming 100% of one of my cpu's, but eventually compiles. OTOH, when onely UNSIGNED_I5) is defined, it never terminates: [snip] The above should read:
OTOH, when only UNSIGNED_I6 is defined, it never terminates (waited more than 30secs before killing the process).
Larry Evans wrote:
Thanks for clearing that up.
However, I did some more exploring, and with the attached:
when only defined(UNSIGNED_I4), it compiles within a few secs. when only defined(UNSIGNED_I5), it times several seconds consuming 100% of one of my cpu's, but eventually compiles. OTOH, when onely UNSIGNED_I5) is defined, it never terminates:
4^4 = 256. 5^5 = 3125. 6^6 = 46656.
On 4/6/19 12:43 PM, Peter Dimov via Boost wrote:
Larry Evans wrote:
Thanks for clearing that up.
However, I did some more exploring, and with the attached:
when only defined(UNSIGNED_I4), it compiles within a few secs. when only defined(UNSIGNED_I5), it times several seconds consuming 100% of one of my cpu's, but eventually compiles. OTOH, when onely UNSIGNED_I5) is defined, it never terminates:
4^4 = 256. 5^5 = 3125. 6^6 = 46656.
Ah! Thanks. That information would be useful in the user documentation to give future users a heads up about the compile time complexity.
On 4/6/19 10:01 AM, Larry Evans via Boost wrote: [snip]
It would illustrate better how to use `visit` if you provided some examples other than those in the test directory.
Such as the example bracketed by the `#ifdef UNSIGNED_I4` in the attached. [snip]
participants (5)
-
Andrzej Krzemienski -
Larry Evans -
Michael Caisse -
Peter Dimov -
Steven Watanabe