Identical types support for >= C++11 (fixes #40)

[AnthonyVH]

This branch contains changes required to support multiple identical types (e.g. nonstd::variant<nonstd::monostate, int, int>), and accompanying tests. I've tried keeping the code style of my changes identical to what was already there.

Note that I couldn't get this working for C++98. The reason being that there's no support for non-type template parameter default arguments for function templates. I use a defaulted function reference template argument to disambiguate between otherwise identical functions, such as constructors for T1 and T2 for a type nonstd::variant<nonstd::monostate, int, int>.

For the regular constructors (as well as copy constructor) this lack of support for default template arguments can be worked around by disambiguating with a defaulted function argument instead. Unfortunately, this is not possible for the assignment operator, which can only have a single argument.

Hence the lack of C++98 support. Maybe there's another way to do this, but I couldn't think of any. Just in case there's a simple way to add this support, I've already written all required SFINAE helper structs in a C++98-compatible way.

I've also made a couple of changes/fixes to the ordering of some defines, as well as to the tests. It seems that by default std::variant was never pulled in, even if C++17 was available. That's now fixed, as well some test code that started failing as a result of this (see individual commits). The tests for all C++ versions now compile and pass.

If you don't like having the "multiple identical types" + "test fixes" changes in a single pull request, just let me know. Then I'll try and split them up properly.

Feel free to let me know if there's anything that I overlooked or that I should have done differently!

[AnthonyVH]

It seems like GCC isn't too happy with my code changes and neither are some clang versions. I did all tests locally using clang 10.0.0-4. I'll look into the issue and fix it.

[martinmoene]

Hi @AnthonyVH Thanks for your effort and the explanation of what you did, a welcome improvement, great work!

I'd like to have a second look tomorrow.

The only review result now are:

• indentation of 4 sp vs. 2 sp :)
• i might prefer the following split between C++98 and C++11 code for the variant constructor:

#if variant_CPP11_OR_GREATER
    {%- for n in range(NumParams) %}
    template < variant_index_tag_t( {{n}} ) = variant_index_tag( {{n}} )
        variant_REQUIRES_B(detail::typelist_type_is_unique< variant_types, {{n}} >::value) >
    variant( T{{n}} const & t{{n}} ) : type_index( {{n}} ) { new( ptr() ) T{{n}}( t{{n}} ); }
    {% endfor %}
#else
    {%- for n in range(NumParams) %}
    variant( T{{n}} const & t{{n}} ) : type_index( {{n}} ) { new( ptr() ) T{{n}}( t{{n}} ); }
    {% endfor %}
#endif

instead of:

    {%- for n in range(NumParams) %}
#if variant_CPP11_OR_GREATER
    template < variant_index_tag_t( {{n}} ) = variant_index_tag( {{n}} )
      variant_REQUIRES_B(detail::typelist_type_is_unique< variant_types, {{n}} >::value) >
#endif
    variant( T{{n}} const & t{{n}} ) : type_index( {{n}} ) { new( ptr() ) T{{n}}( t{{n}} ); }
    {% endfor %}

[martinmoene]

Hi @AnthonyVH Did not find any other thing to mention on my second look. Very happy with your work!

Shall I merge it as-is or do you first want to make further changes yourself?

[AnthonyVH]

If you can wait a little longer, I'll go over it tonight to make the white space and layout changes you mentioned.