From 4de9f80487974567b2cb304ce0fb4e045ba221b6 Mon Sep 17 00:00:00 2001
From: Peter Dimov <pdimov@pdimov.com>
Date: Wed, 14 Jun 2017 01:04:36 +0300
Subject: [PATCH] Require valueless to be first, add test

---
 include/boost/variant2/variant.hpp |  24 +--
 test/Jamfile                       |   1 +
 test/variant_valueless.cpp         | 235 +++++++++++++++++++++++++++++
 3 files changed, 249 insertions(+), 11 deletions(-)
 create mode 100644 test/variant_valueless.cpp

diff --git a/include/boost/variant2/variant.hpp b/include/boost/variant2/variant.hpp
index b20bdf8..cd59f0e 100644
--- a/include/boost/variant2/variant.hpp
+++ b/include/boost/variant2/variant.hpp
@@ -407,7 +407,7 @@ template<class T1, class... T> union variant_storage_impl<mp_true, T1, T...>
         rest_.emplace( mp_size_t<I-1>(), std::forward<A>(a)... );
     }
 
-    template<std::size_t I, class... A> constexpr void emplace_impl( mp_true, mp_size_t<I>, A&&... a ) noexcept
+    template<std::size_t I, class... A> constexpr void emplace_impl( mp_true, mp_size_t<I>, A&&... a )
     {
         *this = variant_storage_impl( mp_size_t<I>(), std::forward<A>(a)... );
     }
@@ -459,8 +459,10 @@ template<class U, class... T> using resolve_overload_index = mp_find<mp_list<T..
 
 // variant_base
 
+template<class... T> using can_be_valueless = std::is_same<mp_first<mp_list<T...>>, valueless>;
+
 template<bool is_trivially_destructible, bool is_single_buffered, class... T> struct variant_base_impl; // trivially destructible, single buffered
-template<class... T> using variant_base = variant_base_impl<mp_all<std::is_trivially_destructible<T>...>::value, mp_any<mp_all<std::is_nothrow_move_constructible<T>...>, std::is_same<T, valueless>...>::value, T...>;
+template<class... T> using variant_base = variant_base_impl<mp_all<std::is_trivially_destructible<T>...>::value, mp_any<mp_all<std::is_nothrow_move_constructible<T>...>, can_be_valueless<T...>>::value, T...>;
 
 struct none {};
 
@@ -517,10 +519,10 @@ template<class... T> struct variant_base_impl<true, true, T...>
 
     template<std::size_t J, class U, class... A> void emplace_impl( mp_false, mp_false, A&&... a )
     {
-        std::size_t const K = mp_find<mp_list<T...>, valueless>::value;
-
-        if( K < sizeof...(T) ) // have valueless
+        if( can_be_valueless<T...>::value ) // T0 == valueless
         {
+            std::size_t const K = 0;
+
             try
             {
                 st1_.emplace( mp_size_t<J>(), std::forward<A>(a)... );
@@ -550,7 +552,7 @@ template<class... T> struct variant_base_impl<true, true, T...>
         std::size_t const J = I+1;
         using U = mp_at_c<variant<T...>, I>;
 
-        this->emplace_impl<J, U>( std::is_nothrow_constructible<U, A...>(), mp_all<variant2::detail::is_trivially_move_constructible<U>, variant2::detail::is_trivially_move_assignable<T>...>(), std::forward<A>(a)... );
+        this->emplace_impl<J, U>( std::is_nothrow_constructible<U, A&&...>(), mp_all<variant2::detail::is_trivially_move_constructible<U>, variant2::detail::is_trivially_move_assignable<T>...>(), std::forward<A>(a)... );
     }
 };
 
@@ -670,8 +672,6 @@ template<class... T> struct variant_base_impl<false, true, T...>
     {
         size_t const J = I+1;
 
-        std::size_t const K = mp_find<mp_list<T...>, valueless>::value;
-
         using U = mp_at_c<variant<T...>, I>;
 
         if( std::is_nothrow_constructible<U, A...>::value )
@@ -681,8 +681,10 @@ template<class... T> struct variant_base_impl<false, true, T...>
             st1_.emplace( mp_size_t<J>(), std::forward<A>(a)... );
             ix_ = J;
         }
-        else if( K < sizeof...(T) ) // have valueless
+        else if( can_be_valueless<T...>::value ) // T0 == valueless
         {
+            std::size_t const K = 0;
+
             _destroy();
 
             try
@@ -692,8 +694,8 @@ template<class... T> struct variant_base_impl<false, true, T...>
             }
             catch( ... )
             {
-                st1_.emplace( mp_size_t<K>() );
-                ix_ = K;
+                st1_.emplace( mp_size_t<K+1>() );
+                ix_ = K+1;
 
                 throw;
             }
diff --git a/test/Jamfile b/test/Jamfile
index 39973c9..9d082bf 100644
--- a/test/Jamfile
+++ b/test/Jamfile
@@ -63,3 +63,4 @@ run variant_visit.cpp : : : $(REQ) ;
 run variant_lt_gt.cpp : : : $(REQ) ;
 run variant_convert_construct.cpp : : : $(REQ) ;
 run variant_subset.cpp : : : $(REQ) ;
+run variant_valueless.cpp : : : $(REQ) ;
diff --git a/test/variant_valueless.cpp b/test/variant_valueless.cpp
new file mode 100644
index 0000000..77bb166
--- /dev/null
+++ b/test/variant_valueless.cpp
@@ -0,0 +1,235 @@
+
+// Copyright 2017 Peter Dimov.
+//
+// Distributed under the Boost Software License, Version 1.0.
+//
+// See accompanying file LICENSE_1_0.txt or copy at
+// http://www.boost.org/LICENSE_1_0.txt
+
+#include <boost/variant2/variant.hpp>
+#include <boost/core/lightweight_test.hpp>
+#include <type_traits>
+#include <utility>
+#include <string>
+#include <stdexcept>
+
+using namespace boost::variant2;
+
+#define STATIC_ASSERT(...) static_assert(__VA_ARGS__, #__VA_ARGS__)
+
+enum E1
+{
+    e1
+};
+
+enum E2
+{
+    e2
+};
+
+struct X1
+{
+    X1() = default;
+    X1( E2 ) { throw std::runtime_error( "X1(E2)" ); }
+};
+
+STATIC_ASSERT( std::is_nothrow_copy_constructible<X1>::value );
+STATIC_ASSERT( std::is_nothrow_move_constructible<X1>::value );
+STATIC_ASSERT( !std::is_nothrow_constructible<X1, E2>::value );
+STATIC_ASSERT( std::is_trivially_destructible<X1>::value );
+
+struct X2
+{
+    X2() = default;
+    ~X2();
+    X2( E2 ) { throw std::runtime_error( "X1(E2)" ); }
+};
+
+X2::~X2() {}
+
+STATIC_ASSERT( std::is_nothrow_copy_constructible<X2>::value );
+STATIC_ASSERT( std::is_nothrow_move_constructible<X2>::value );
+STATIC_ASSERT( !std::is_nothrow_constructible<X2, E2>::value );
+STATIC_ASSERT( !std::is_trivially_destructible<X2>::value );
+
+struct X3
+{
+    X3() = default;
+    X3( X3 const& ) {}
+    X3( X3&& ) {}
+};
+
+STATIC_ASSERT( !std::is_nothrow_copy_constructible<X3>::value );
+STATIC_ASSERT( !std::is_nothrow_move_constructible<X3>::value );
+STATIC_ASSERT( std::is_trivially_destructible<X3>::value );
+
+int main()
+{
+    {
+        variant<monostate, E1, X1> v;
+
+        v = e1;
+
+        BOOST_TEST_EQ( v.index(), 1 );
+        BOOST_TEST_EQ( get<1>(v), e1 );
+
+        try
+        {
+            v = e2;
+            BOOST_ERROR( "`v = e2;` failed to throw" );
+        }
+        catch( std::exception const& )
+        {
+            BOOST_TEST_EQ( v.index(), 1 );
+            BOOST_TEST_EQ( get<1>(v), e1 );
+        }
+    }
+
+    {
+        variant<X3, E1, X1> v;
+
+        v = e1;
+
+        BOOST_TEST_EQ( v.index(), 1 );
+        BOOST_TEST_EQ( get<1>(v), e1 );
+
+        try
+        {
+            v = e2;
+            BOOST_ERROR( "`v = e2;` failed to throw" );
+        }
+        catch( std::exception const& )
+        {
+            BOOST_TEST_EQ( v.index(), 1 );
+            BOOST_TEST_EQ( get<1>(v), e1 );
+        }
+    }
+
+    {
+        variant<valueless, E1, X1> v;
+
+        v = e1;
+
+        BOOST_TEST_EQ( v.index(), 1 );
+        BOOST_TEST_EQ( get<1>(v), e1 );
+
+        try
+        {
+            v = e2;
+            BOOST_ERROR( "`v = e2;` failed to throw" );
+        }
+        catch( std::exception const& )
+        {
+            // all types are trivial, so we get a constexpr path
+            // and v.index() stays 1
+
+            BOOST_TEST( v.index() == 0 || v.index() == 1 );
+
+            if( v.index() == 1 )
+            {
+                BOOST_TEST_EQ( get<1>(v), e1 );
+            }
+        }
+    }
+
+    {
+        variant<monostate, E1, X1, valueless> v;
+
+        v = e1;
+
+        BOOST_TEST_EQ( v.index(), 1 );
+        BOOST_TEST_EQ( get<1>(v), e1 );
+
+        try
+        {
+            v = e2;
+            BOOST_ERROR( "`v = e2` failed to throw" );
+        }
+        catch( std::exception const& )
+        {
+            BOOST_TEST_EQ( v.index(), 1 );
+            BOOST_TEST_EQ( get<1>(v), e1 );
+        }
+    }
+
+    {
+        variant<monostate, E1, X2> v;
+
+        v = e1;
+
+        BOOST_TEST_EQ( v.index(), 1 );
+        BOOST_TEST_EQ( get<1>(v), e1 );
+
+        try
+        {
+            v = e2;
+            BOOST_ERROR( "`v = e2;` failed to throw" );
+        }
+        catch( std::exception const& )
+        {
+            BOOST_TEST_EQ( v.index(), 1 );
+            BOOST_TEST_EQ( get<1>(v), e1 );
+        }
+    }
+
+    {
+        variant<X3, E1, X2> v;
+
+        v = e1;
+
+        BOOST_TEST_EQ( v.index(), 1 );
+        BOOST_TEST_EQ( get<1>(v), e1 );
+
+        try
+        {
+            v = e2;
+            BOOST_ERROR( "`v = e2;` failed to throw" );
+        }
+        catch( std::exception const& )
+        {
+            BOOST_TEST_EQ( v.index(), 1 );
+            BOOST_TEST_EQ( get<1>(v), e1 );
+        }
+    }
+
+    {
+        variant<valueless, E1, X2> v;
+
+        v = e1;
+
+        BOOST_TEST_EQ( v.index(), 1 );
+        BOOST_TEST_EQ( get<1>(v), e1 );
+
+        try
+        {
+            v = e2;
+            BOOST_ERROR( "`v = e2;` failed to throw" );
+        }
+        catch( std::exception const& )
+        {
+            BOOST_TEST_EQ( v.index(), 0 );
+        }
+    }
+
+    {
+        variant<monostate, E1, X2, valueless> v;
+
+        v = e1;
+
+        BOOST_TEST_EQ( v.index(), 1 );
+        BOOST_TEST_EQ( get<1>(v), e1 );
+
+        try
+        {
+            v = e2;
+            BOOST_ERROR( "`v = e2;` failed to throw" );
+        }
+        catch( std::exception const& )
+        {
+            BOOST_TEST_EQ( v.index(), 1 );
+            BOOST_TEST_EQ( get<1>(v), e1 );
+        }
+    }
+
+    return boost::report_errors();
+}