Add support for C++17 std::variant

[sdebionne]

Partially fixes #114 and drops support for MSVC6.

[sdebionne]

@robertramey Robert, do you know how I could set std=c++17 only for a specific test with Boost.Build? Some compilers in the pipeline do not support C++17, is there a way to detect the feature and not run the test with these older compilers?

[robertramey]

the normal way to do this is to include a line in the Jamfile which looks like the following:

 [ test-bsl-run_files test_array : A : :  [ requires cxx11_hdr_array ] ] # BOOST_NO_CXX11_HDR_ARRAY

This will enable the test for all compilers which support std::array. This depends in boost config defining BOOST_NO_CXX11_HDR_ARRAY. So in your case you'll need to use BOOST_NO_CXX11_HDR_VARIANT. But I see that boost config doesn't include this yet. So you'll have to bug John Maddock to get this included. They you'll be good to go. BTW - since you're on this, you might want to look at boost variant2 - in development by peter dimov

[sdebionne]

AFAIU, variant2 is a drop in replacement of std::variant for C+11 so it should work out of the box (beside the namespace), are you suggesting to add support for this as well?

I try to add a property cxxstd=17 in the test requirements of the variant test, but that gives error: 'cxxstd=17' is not a valid property specification. What would be the right way to switch to C++17 for a single test?

[sdebionne]

@robertramey The only test (test_polymorphic_helper_polymorphic_binary_archive) that do not pass actually fails on the develop branch too. Any idea what is wrong?

[robertramey]

@robertramey The only test (test_polymorphic_helper_polymorphic_binary_archive) that do not pass actually fails on the develop branch to. Any idea what is wrong?

Notice that it only happens with gcc on windows (mingw?), with polymorphic_binary_archive, on static linking. The executable seems to run but then ... the expected *.test file isn't found. I'm guessing that this is indicates some obscure issue with the jam file in the test directory. I haven't been able to find it. The only real way to would be to test on a local machine with that configuration - which I don't have.

So I haven't worried about this failure. If you wanted to spend some time on this obscure circumstance, that would be great. But you can ignore it as well.

[p12tic]

@robertramey @sdebionne : Is there any way I could help to land this PR? Is it that test failure that prevents merging or something else? std::variant support would be really useful for us. Thanks!

[p12tic]

This will clash with a class with the same name on boost::variant's variant.hpp

[p12tic]

We could use std::move to allow non-copy-constructible types. The serialization support for boost::variant has the same issue though.

[sdebionne]

@p12tic Thanks for the review, I think all your points are valid, I'll fix that ASAP.

Regarding the merge, I won't have time to investigate the (unrelated) pipeline failure now, but I think @robertramey was OK to merge as is.

Boost.Variant2 is landing in 1.71. Since it has the same interface than the std, I wonder if we could factorize both serializations here.

As a side note, I have an std::optional serialization to, but will open a separate PR not to delay this one.

[mateuszzz88]

I think you should add serialization of std::monostate - it is equivalent of boost::blank

namespace boost {
namespace serialization {

template<class Archive>
void serialize(Archive &ar, std::monostate &, const unsigned int /*version*/)
{}

}
}

I see no reason to put in separate file, variant.hpp seems ok (monostate is in the same header <variant>).

[sdebionne]

@pdimov Do you see any way I could support Boost.Variant2 without duplicating the code too much?

[pdimov]

If you remove the std:: from std::visit and std::get, the same code should also work for variant2. Instead of variant_size you can use mp_size which should work on both (but you'll need to remove qualifiers and references if present.) Also, what I said above about mp11::mp_with_index.

[robertramey]

Instead of variant_size you can use mp_size which should work on both (but you'll need to remove qualifiers and references if present.) Also, what I said above about mp11::mp_with_index.

Hmmm - this would bump the Compiler requirement from C++03 to C++11. But then it would only do so for those using std::variant or boost::variant2. So I'd have to think about this.

[robertramey]

Whoops. I misspoke and I apologize. I got the notice of a commit and didn't realize it was for your PR and not the boost develop repo. Chalk it up to some GitHub confusion.

[kilasuelika]

I have test std_variant, it works perfectly.

[code-affinity]

When I try to compile this header in a context in which <boost/serialization/variant.hpp> has already been included, there is a name conflict between the variant_impl struct that is declared in this header, and the variant_impl struct that is declared in <boost/serialization/variant.hpp>

I resolved the problem by changing the three occurrences of variant_impl to stdvariant_impl.

[Febbe]

@robertramey whats missing here? May you overtake this PR when it does not fulfill your expectations?