From c3f9beaadc3044885b39ce4c15ba5e21183b8034 Mon Sep 17 00:00:00 2001 From: Peter Dimov Date: Sun, 12 May 2019 00:38:21 +0300 Subject: [PATCH 1/4] Strong guarantee on emplace --- include/boost/variant2/variant.hpp | 85 +++--------------------------- test/variant_valueless.cpp | 52 ++++++------------ 2 files changed, 24 insertions(+), 113 deletions(-) diff --git a/include/boost/variant2/variant.hpp b/include/boost/variant2/variant.hpp index f7fac51..7834ef7 100644 --- a/include/boost/variant2/variant.hpp +++ b/include/boost/variant2/variant.hpp @@ -598,11 +598,8 @@ template using resolve_overload_index = mp11::mp_find using can_be_valueless = mp11::mp_any..., std::is_nothrow_default_constructible...>; -template using valueless_index = mp11::mp_if, monostate>, mp11::mp_find, monostate>, mp11::mp_find_if, std::is_nothrow_default_constructible>>; - -template struct variant_base_impl; // trivially destructible, single buffered -template using variant_base = variant_base_impl...>::value, mp11::mp_any...>, can_be_valueless>::value, T...>; +template struct variant_base_impl; +template using variant_base = variant_base_impl...>::value, mp11::mp_all...>::value, T...>; struct none {}; @@ -649,7 +646,7 @@ template struct variant_base_impl return st1_.get( mp11::mp_size_t() ); } - template BOOST_CXX14_CONSTEXPR void emplace_impl( mp11::mp_true, mp11::mp_bool, A&&... a ) + template BOOST_CXX14_CONSTEXPR void emplace_impl( mp11::mp_true, A&&... a ) { static_assert( std::is_nothrow_constructible::value, "Logic error: U must be nothrow constructible from A&&..." ); @@ -657,7 +654,7 @@ template struct variant_base_impl ix_ = J; } - template BOOST_CXX14_CONSTEXPR void emplace_impl( mp11::mp_false, mp11::mp_true, A&&... a ) + template BOOST_CXX14_CONSTEXPR void emplace_impl( mp11::mp_false, A&&... a ) { static_assert( std::is_nothrow_move_constructible::value, "Logic error: U must be nothrow move constructible" ); @@ -667,37 +664,12 @@ template struct variant_base_impl ix_ = J; } - template void emplace_impl( mp11::mp_false, mp11::mp_false, A&&... a ) - { - static_assert( can_be_valueless::value, "Logic error: T... must have a fallback type" ); - - std::size_t const K = valueless_index::value; - - static_assert( K < sizeof...(T), "Logic error: T... must have a fallback index" ); - - try - { - st1_.emplace( mp11::mp_size_t(), std::forward(a)... ); - ix_ = J; - } - catch( ... ) - { - st1_.emplace( mp11::mp_size_t() ); - ix_ = K+1; - - throw; - } - } - template BOOST_CXX14_CONSTEXPR void emplace( A&&... a ) { std::size_t const J = I+1; using U = mp11::mp_at_c, I>; - constexpr bool B1 = can_be_valueless::value; - constexpr bool B2 = mp11::mp_all, detail::is_trivially_move_assignable...>::value; - - this->emplace_impl( std::is_nothrow_constructible(), mp11::mp_bool(), std::forward(a)... ); + this->emplace_impl( std::is_nothrow_constructible(), std::forward(a)... ); } }; @@ -836,42 +808,12 @@ template struct variant_base_impl return st1_.get( mp11::mp_size_t() ); } - template void emplace_impl( mp11::mp_int<0>, A&&... a ) + template void emplace( A&&... a ) { - static_assert( std::is_nothrow_constructible::value, "Logic error: U must be nothrow constructible from A&&..." ); + size_t const J = I+1; - _destroy(); + using U = mp11::mp_at_c, I>; - st1_.emplace( mp11::mp_size_t(), std::forward(a)... ); - ix_ = J; - } - - template void emplace_impl( mp11::mp_int<1>, A&&... a ) - { - static_assert( can_be_valueless::value, "Logic error: T... must have a fallback type" ); - - std::size_t const K = valueless_index::value; - - static_assert( K < sizeof...(T), "Logic error: T... must have a fallback index" ); - - _destroy(); - - try - { - st1_.emplace( mp11::mp_size_t(), std::forward(a)... ); - ix_ = J; - } - catch( ... ) - { - st1_.emplace( mp11::mp_size_t() ); - ix_ = K+1; - - throw; - } - } - - template void emplace_impl( mp11::mp_int<2>, A&&... a ) - { static_assert( std::is_nothrow_move_constructible::value, "Logic error: U must be nothrow move constructible" ); U tmp( std::forward(a)... ); @@ -881,17 +823,6 @@ template struct variant_base_impl st1_.emplace( mp11::mp_size_t(), std::move(tmp) ); ix_ = J; } - - template void emplace( A&&... a ) - { - size_t const J = I+1; - - using U = mp11::mp_at_c, I>; - - int const D = std::is_nothrow_constructible::value? 0: ( can_be_valueless::value? 1: 2 ); - - this->emplace_impl( mp11::mp_int(), std::forward(a)... ); - } }; // not trivially destructible, double buffered diff --git a/test/variant_valueless.cpp b/test/variant_valueless.cpp index 176c674..d5d18d7 100644 --- a/test/variant_valueless.cpp +++ b/test/variant_valueless.cpp @@ -115,8 +115,8 @@ int main() } catch( std::exception const& ) { - // basic guarantee; X1 is nothrow default-constructible - BOOST_TEST_EQ( v.index(), 1 ); + // strong guarantee + BOOST_TEST_EQ( v.index(), 0 ); } } @@ -132,8 +132,8 @@ int main() } catch( std::exception const& ) { - // basic guarantee; X1 is nothrow default-constructible - BOOST_TEST_EQ( v.index(), 0 ); + // strong guarantee + BOOST_TEST_EQ( v.index(), 1 ); } } @@ -149,8 +149,8 @@ int main() } catch( std::exception const& ) { - // basic guarantee; monostate - BOOST_TEST_EQ( v.index(), 2 ); + // strong guarantee + BOOST_TEST_EQ( v.index(), 0 ); } } @@ -166,8 +166,8 @@ int main() } catch( std::exception const& ) { - // basic guarantee; monostate - BOOST_TEST_EQ( v.index(), 2 ); + // strong guarantee + BOOST_TEST_EQ( v.index(), 1 ); } } @@ -183,8 +183,8 @@ int main() } catch( std::exception const& ) { - // basic guarantee; X1 is nothrow default-constructible - BOOST_TEST_EQ( v.index(), 2 ); + // strong guarantee + BOOST_TEST_EQ( v.index(), 0 ); } } @@ -200,8 +200,8 @@ int main() } catch( std::exception const& ) { - // basic guarantee; monostate - BOOST_TEST_EQ( v.index(), 3 ); + // strong guarantee + BOOST_TEST_EQ( v.index(), 0 ); } } @@ -234,18 +234,8 @@ int main() } catch( std::exception const& ) { - // X3 is not v2d::trivially_move_assignable on libstdc++ 4.x - - if( v2d::is_trivially_move_assignable::value ) - { - // all trivially destructible and move-assignable, no change - BOOST_TEST_EQ( v.index(), 0 ); - } - else - { - // basic guarantee; X1 is nothrow default-constructible - BOOST_TEST_EQ( v.index(), 1 ); - } + // strong guarantee + BOOST_TEST_EQ( v.index(), 0 ); } } @@ -261,18 +251,8 @@ int main() } catch( std::exception const& ) { - // X3 is not v2d::trivially_move_assignable on libstdc++ 4.x - - if( v2d::is_trivially_move_assignable::value ) - { - // all trivially destructible and move-assignable, no change - BOOST_TEST_EQ( v.index(), 0 ); - } - else - { - // basic guarantee; monostate - BOOST_TEST_EQ( v.index(), 2 ); - } + // strong guarantee + BOOST_TEST_EQ( v.index(), 0 ); } } From 7f7c74522b321458a2348c9ba67bc3eb7632e834 Mon Sep 17 00:00:00 2001 From: Peter Dimov Date: Sun, 12 May 2019 01:46:55 +0300 Subject: [PATCH 2/4] Strong guarantee on assignment --- include/boost/variant2/variant.hpp | 35 +++++------------------------- 1 file changed, 6 insertions(+), 29 deletions(-) diff --git a/include/boost/variant2/variant.hpp b/include/boost/variant2/variant.hpp index 7834ef7..4bf0577 100644 --- a/include/boost/variant2/variant.hpp +++ b/include/boost/variant2/variant.hpp @@ -1168,14 +1168,7 @@ private: template void operator()( I i ) const { - if( this_->index() == i ) - { - this_->_get_impl( i ) = r._get_impl( i ); - } - else - { - this_->variant_base::template emplace( r._get_impl( i ) ); - } + this_->variant_base::template emplace( r._get_impl( i ) ); } }; @@ -1186,7 +1179,7 @@ public: class E3 = mp11::mp_if..., std::is_copy_assignable...>, E1> > BOOST_CXX14_CONSTEXPR variant& operator=( variant const & r ) - noexcept( mp11::mp_all..., std::is_nothrow_copy_assignable...>::value ) + noexcept( mp11::mp_all...>::value ) { mp11::mp_with_index( r.index(), L3{ this, r } ); return *this; @@ -1210,14 +1203,7 @@ private: template void operator()( I i ) const { - if( this_->index() == i ) - { - this_->_get_impl( i ) = std::move( r._get_impl( i ) ); - } - else - { - this_->variant_base::template emplace( std::move( r._get_impl( i ) ) ); - } + this_->variant_base::template emplace( std::move( r._get_impl( i ) ) ); } }; @@ -1228,7 +1214,7 @@ public: class E3 = mp11::mp_if..., std::is_move_assignable...>, E1> > variant& operator=( variant && r ) - noexcept( mp11::mp_all..., std::is_nothrow_move_assignable...>::value ) + noexcept( mp11::mp_all...>::value ) { mp11::mp_with_index( r.index(), L4{ this, r } ); return *this; @@ -1240,19 +1226,10 @@ public: class E2 = typename std::enable_if::value && std::is_constructible::value>::type > BOOST_CXX14_CONSTEXPR variant& operator=( U&& u ) - noexcept( std::is_nothrow_assignable::value && std::is_nothrow_constructible::value ) + noexcept( std::is_nothrow_constructible::value ) { std::size_t const I = detail::resolve_overload_index::value; - - if( index() == I ) - { - _get_impl( mp11::mp_size_t() ) = std::forward(u); - } - else - { - this->template emplace( std::forward(u) ); - } - + this->template emplace( std::forward(u) ); return *this; } From 8691721a9c9ff279165b0ca0aaa1749a3b45e5fa Mon Sep 17 00:00:00 2001 From: Peter Dimov Date: Sun, 12 May 2019 02:02:49 +0300 Subject: [PATCH 3/4] Update reference --- doc/variant2/reference.adoc | 52 +++++++++++-------------------------- 1 file changed, 15 insertions(+), 37 deletions(-) diff --git a/doc/variant2/reference.adoc b/doc/variant2/reference.adoc index 0ad3538..a24fa73 100644 --- a/doc/variant2/reference.adoc +++ b/doc/variant2/reference.adoc @@ -390,18 +390,14 @@ Effects: :: ``` constexpr variant& operator=( const variant& r ) - noexcept( mp_all..., - std::is_nothrow_copy_assignable...>::value ); + noexcept( mp_all...>::value ); ``` [none] * {blank} + Let `j` be `r.index()`. -Effects: :: -- If `index() == j`, assigns the value contained in `r` to the value - contained in `*this`. -- Otherwise, equivalent to `emplace(get(r))`. +Effects: :: `emplace(get(r))`. Returns: :: `*this`. Ensures: :: `index() == r.index()`. Remarks: :: This operator does not participate in overload resolution unless @@ -410,18 +406,14 @@ Remarks: :: This operator does not participate in overload resolution unless ``` constexpr variant& operator=( variant&& r ) - noexcept( mp_all..., - std::is_nothrow_move_assignable...>::value ); + noexcept( mp_all...>::value ); ``` [none] * {blank} + Let `j` be `r.index()`. -Effects: :: -- If `index() == j`, assigns the value contained in `std::move(r)` to the - value contained in `*this`. -- Otherwise, equivalent to `emplace(get(std::move(r)))`. +Effects: :: `emplace(get(std::move(r)))`. Returns: :: `*this`. Ensures: :: `index() == r.index()`. Remarks: :: This operator does not participate in overload resolution unless @@ -440,18 +432,14 @@ Let `Tj` be a type that is determined as follows: build an imaginary function overload resolution for the expression `FUN(std::forward(u))` defines the alternative `Tj` which is the type of the contained value after construction. -Effects: :: -- If `index() == j`, assigns `std::forward(u)` to the value contained in - `*this`. -- Otherwise, equivalent to `emplace(std::forward(u))`. +Effects: :: `emplace(std::forward(u))`. Returns: :: `*this`. Ensures: :: `index() == j`. Remarks: :: - The expression inside `noexcept` is `std::is_nothrow_constructible_v - && std::is_nothrow_assignable_v`. + The expression inside `noexcept` is `std::is_nothrow_constructible_v`. This operator does not participate in overload resolution unless - `std::is_same_v, variant>` is `false`, - - `std::is_constructible_v && std::is_assignable_v` is + - `std::is_constructible_v && std::is_assignable_v` is `true`, and - the expression `FUN(std::forward(u))` (with `FUN` being the above-mentioned set of imaginary functions) is well-formed. @@ -470,7 +458,7 @@ Let `I` be the zero-based index of `U` in `T...`. Effects: :: Equivalent to: `return emplace(std::forward(a)...);` Remarks: :: This function shall not participate in overload resolution unless - `std::is_constructible_v` is `true` and `U` occurs exactly once + `std::is_constructible_v` is `true` and `U` occurs exactly once in `T...`. ``` @@ -485,7 +473,7 @@ Let `I` be the zero-based index of `U` in `T...`. Effects: :: Equivalent to: `return emplace(il, std::forward(a)...);` Remarks: :: This function shall not participate in overload resolution unless - `std::is_constructible_v&, A...>` is `true` + `std::is_constructible_v&, A&&...>` is `true` and `U` occurs exactly once in `T...`. @@ -505,16 +493,10 @@ Ensures: :: `index() == I`. Returns: :: A reference to the new contained value. Throws: :: Nothing unless the initialization of the new contained value throws. -Exception Safety: :: On exception: - - If the list of alternatives contains `monostate`, the contained value - is either unchanged, or `monostate{}`; - - Otherwise, if the list of alternatives contains types for which - `is_nothrow_default_constructible_v` is `true`, the contained value - is either unchanged, or `Tj{}`, where `Tj` is the first such alternative; - - Otherwise, the contained value is unchanged. +Exception Safety: :: Strong. On exception, the contained value is unchanged. Remarks: :: This function shall not participate in overload resolution unless - `std::is_constructible_v` is `true`. + `std::is_constructible_v` is `true`. ``` template @@ -532,16 +514,10 @@ Ensures: :: `index() == I`. Returns: :: A reference to the new contained value. Throws: :: Nothing unless the initialization of the new contained value throws. -Exception Safety: :: On exception: - - If the list of alternatives contains `monostate`, the contained value - is either unchanged, or `monostate{}`; - - Otherwise, if the list of alternatives contains types for which - `is_nothrow_default_constructible_v` is `true`, the contained value - is either unchanged, or `Tj{}`, where `Tj` is the first such alternative; - - Otherwise, the contained value is unchanged. +Exception Safety: :: Strong. On exception, the contained value is unchanged. Remarks: :: This function shall not participate in overload resolution unless - `std::is_constructible_v&, A...>` is `true`. + `std::is_constructible_v&, A&&...>` is `true`. #### Value Status @@ -553,6 +529,8 @@ constexpr bool valueless_by_exception() const noexcept; + Returns: :: `false`. +NOTE: This function is provided purely for compatibility with `std::variant`. + ``` constexpr size_t index() const noexcept; ``` From 6390b5ed20eb5b12d153392c6c5bf3ed20ec1190 Mon Sep 17 00:00:00 2001 From: Peter Dimov Date: Sun, 12 May 2019 02:02:58 +0300 Subject: [PATCH 4/4] Update html --- doc/html/variant2.html | 105 ++++++++++------------------------------- 1 file changed, 26 insertions(+), 79 deletions(-) diff --git a/doc/html/variant2.html b/doc/html/variant2.html index 918dab2..9019e21 100644 --- a/doc/html/variant2.html +++ b/doc/html/variant2.html @@ -1437,8 +1437,7 @@ there is exactly one occurrence of U in T…​ 
constexpr variant& operator=( const variant& r )
-  noexcept( mp_all<std::is_nothrow_copy_constructible<T>...,
-    std::is_nothrow_copy_assignable<T>...>::value );
+ noexcept( mp_all<std::is_nothrow_copy_constructible<T>...>::value );
@@ -1452,17 +1451,7 @@ there is exactly one occurrence of U in T…​
Effects:
-
-
    -
  • -

    If index() == j, assigns the value contained in r to the value -contained in *this.

    -
    • -
  • -

    Otherwise, equivalent to emplace<j>(get<j>(r)).

    -
    • -
-
+

emplace<j>(get<j>(r)).

Returns:
@@ -1486,8 +1475,7 @@ contained in *this.

constexpr variant& operator=( variant&& r )
-  noexcept( mp_all<std::is_nothrow_move_constructible<T>...,
-    std::is_nothrow_move_assignable<T>...>::value );
+ noexcept( mp_all<std::is_nothrow_move_constructible<T>...>::value );
@@ -1501,17 +1489,7 @@ contained in *this.

Effects:
-
-
    -
  • -

    If index() == j, assigns the value contained in std::move(r) to the -value contained in *this.

    -
    • -
  • -

    Otherwise, equivalent to emplace<j>(get<j>(std::move(r))).

    -
    • -
-
+

emplace<j>(get<j>(std::move(r))).

Returns:
@@ -1552,17 +1530,7 @@ alternative Tj which is the type of the contained value after const
Effects:
-
-
    -
  • -

    If index() == j, assigns std::forward<U>(u) to the value contained in -*this.

    -
    • -
  • -

    Otherwise, equivalent to emplace<j>(std::forward<U>(u)).

    -
    • -
-
+

emplace<j>(std::forward<U>(u)).

Returns:
@@ -1574,8 +1542,7 @@ alternative Tj which is the type of the contained value after const
Remarks:
-

The expression inside noexcept is std::is_nothrow_constructible_v<Tj, U> - && std::is_nothrow_assignable_v<Tj&, U>. +

The expression inside noexcept is std::is_nothrow_constructible_v<Tj, U&&>. This operator does not participate in overload resolution unless

    @@ -1583,7 +1550,7 @@ This operator does not participate in overload resolution unless

    std::is_same_v<std::remove_cvref_t<T>, variant> is false,

  • -

    std::is_constructible_v<Tj, U> && std::is_assignable_v<Tj&, U> is +

    std::is_constructible_v<Tj, U&&> && std::is_assignable_v<Tj&, U&&> is true, and

  • @@ -1623,7 +1590,7 @@ above-mentioned set of imaginary functions) is well-formed.

    Remarks:

    This function shall not participate in overload resolution unless -std::is_constructible_v<U, A…​> is true and U occurs exactly once +std::is_constructible_v<U, A&&…​> is true and U occurs exactly once in T…​.

@@ -1653,7 +1620,7 @@ in T…​.

Remarks:

This function shall not participate in overload resolution unless -std::is_constructible_v<U, std::initializer_list<V>&, A…​> is true +std::is_constructible_v<U, std::initializer_list<V>&, A&&…​> is true and U occurs exactly once in T…​.

@@ -1697,28 +1664,12 @@ value as if using the expression Ti(std::forward<A>(a)…​
Exception Safety:
-

On exception:

-
-
    -
  • -

    If the list of alternatives contains monostate, the contained value -is either unchanged, or monostate{};

    -
    • -
  • -

    Otherwise, if the list of alternatives contains types for which -is_nothrow_default_constructible_v is true, the contained value -is either unchanged, or Tj{}, where Tj is the first such alternative;

    -
    • -
  • -

    Otherwise, the contained value is unchanged.

    -
    • -
-
+

Strong. On exception, the contained value is unchanged.

Remarks:

This function shall not participate in overload resolution unless -std::is_constructible_v<Ti, A…​> is true.

+std::is_constructible_v<Ti, A&&…​> is true.

@@ -1761,28 +1712,12 @@ value as if using the expression Ti(il, std::forward<A>(a)…
Exception Safety:
-

On exception:

-
-
    -
  • -

    If the list of alternatives contains monostate, the contained value -is either unchanged, or monostate{};

    -
    • -
  • -

    Otherwise, if the list of alternatives contains types for which -is_nothrow_default_constructible_v is true, the contained value -is either unchanged, or Tj{}, where Tj is the first such alternative;

    -
    • -
  • -

    Otherwise, the contained value is unchanged.

    -
    • -
-
+

Strong. On exception, the contained value is unchanged.

Remarks:

This function shall not participate in overload resolution unless -std::is_constructible_v<Ti, std::initializer_list<V>&, A…​> is true.

+std::is_constructible_v<Ti, std::initializer_list<V>&, A&&…​> is true.

@@ -1809,6 +1744,18 @@ is either unchanged, or Tj{}, where Tj is the first su +
+ + + + + +
+
Note
+		 +This function is provided purely for compatibility with std::variant. +
+
@@ -2519,7 +2466,7 @@ the Boost Software License, Versi