[v3] Implement LWG 2857, {variant,optional,any}::emplace should return the constructed value.

Submitted by ville on March 15, 2017, 10:21 p.m.

Details

Message ID CAFk2RUYVZWBm40tzeQRTx=YOoJGMavf4AtWrYQm29yxfn9Gjjw@mail.gmail.com
State New
Headers show

Commit Message

ville March 15, 2017, 10:21 p.m.
Tested on Linux-x64.

2017-03-16  Ville Voutilainen  <ville.voutilainen@gmail.com>

    Implement LWG 2857, {variant,optional,any}::emplace should
    return the constructed value.
    * include/std/any (any_cast(any*)): Forward-declare.
    (emplace(_Args&&...)): Change the return type and
    return a reference to the constructed value.
    (emplace(initializer_list<_Up>, _Args&&...)): Likewise.
    * include/std/optional (emplace(_Args&&...)): Likewise.
    (emplace(initializer_list<_Up>, _Args&&...)): Likewise.
    * include/std/variant (emplace<_Tp>(_Args&&...)): Likewise.
    (emplace<_Tp>(initializer_list<_Up>, _Args&&...)): Likewise.
    (emplace<_Np>(_Args&&...)): Likewise.
    (emplace<_Np>(initializer_list<_Up>, _Args&&...)): Likewise.
    * testsuite/20_util/any/assign/emplace.cc: Add tests for
    checking the return value of emplace.
    * testsuite/20_util/any/misc/any_cast_neg.cc: Adjust.
    * testsuite/20_util/optional/assignment/6.cc: Add tests for
    checking the return value of emplace.
    * testsuite/20_util/variant/run.cc: Likewise.

Comments

Jonathan Wakely March 15, 2017, 10:31 p.m.
On 16/03/17 00:21 +0200, Ville Voutilainen wrote:
>Tested on Linux-x64.
>
>2017-03-16  Ville Voutilainen  <ville.voutilainen@gmail.com>
>
>    Implement LWG 2857, {variant,optional,any}::emplace should
>    return the constructed value.
>    * include/std/any (any_cast(any*)): Forward-declare.
>    (emplace(_Args&&...)): Change the return type and
>    return a reference to the constructed value.
>    (emplace(initializer_list<_Up>, _Args&&...)): Likewise.
>    * include/std/optional (emplace(_Args&&...)): Likewise.
>    (emplace(initializer_list<_Up>, _Args&&...)): Likewise.
>    * include/std/variant (emplace<_Tp>(_Args&&...)): Likewise.
>    (emplace<_Tp>(initializer_list<_Up>, _Args&&...)): Likewise.
>    (emplace<_Np>(_Args&&...)): Likewise.
>    (emplace<_Np>(initializer_list<_Up>, _Args&&...)): Likewise.
>    * testsuite/20_util/any/assign/emplace.cc: Add tests for
>    checking the return value of emplace.
>    * testsuite/20_util/any/misc/any_cast_neg.cc: Adjust.
>    * testsuite/20_util/optional/assignment/6.cc: Add tests for
>    checking the return value of emplace.
>    * testsuite/20_util/variant/run.cc: Likewise.

>diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any
>index e807617..fff13d9 100644
>--- a/libstdc++-v3/include/std/any
>+++ b/libstdc++-v3/include/std/any
>@@ -74,6 +74,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>    *  An @c any object's state is either empty or it stores a contained object
>    *  of CopyConstructible type.
>    */
>+  class any;
>+  template<typename _ValueType>
>+    inline _ValueType* any_cast(any* __any) noexcept;
>+
>   class any
>   {
>     // Holds either pointer to a heap object or the contained object itself.
>@@ -268,18 +272,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>     /// Emplace with an object created from @p __args as the contained object.
>     template <typename _ValueType, typename... _Args>
>-      typename __any_constructible<void,
>+      typename __any_constructible<_Decay<_ValueType>&,
> 				   _Decay<_ValueType>, _Args&&...>::type
>       emplace(_Args&&... __args)
>       {
> 	__do_emplace<_Decay<_ValueType>>
> 	  (std::forward<_Args>(__args)...);
>+	return *(std::any_cast<_Decay<_ValueType>>(this));

Can we avoid the branch in any_cast to check the stored type?
We know it's the right type, because we just stored it.

Patch hide | download patch | download mbox

diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any
index e807617..fff13d9 100644
--- a/libstdc++-v3/include/std/any
+++ b/libstdc++-v3/include/std/any
@@ -74,6 +74,10 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
    *  An @c any object's state is either empty or it stores a contained object
    *  of CopyConstructible type.
    */
+  class any;
+  template<typename _ValueType>
+    inline _ValueType* any_cast(any* __any) noexcept;
+
   class any
   {
     // Holds either pointer to a heap object or the contained object itself.
@@ -268,18 +272,19 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     /// Emplace with an object created from @p __args as the contained object.
     template <typename _ValueType, typename... _Args>
-      typename __any_constructible<void,
+      typename __any_constructible<_Decay<_ValueType>&,
 				   _Decay<_ValueType>, _Args&&...>::type
       emplace(_Args&&... __args)
       {
 	__do_emplace<_Decay<_ValueType>>
 	  (std::forward<_Args>(__args)...);
+	return *(std::any_cast<_Decay<_ValueType>>(this));
       }
 
     /// Emplace with an object created from @p __il and @p __args as
     /// the contained object.
     template <typename _ValueType, typename _Up, typename... _Args>
-      typename __any_constructible<void,
+      typename __any_constructible<_Decay<_ValueType>&,
 				   _Decay<_ValueType>,
 				   initializer_list<_Up>,
 				   _Args&&...>::type
@@ -287,6 +292,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       {
 	__do_emplace<_Decay<_ValueType>, _Up>
 	  (__il, std::forward<_Args>(__args)...);
+	return *(std::any_cast<_Decay<_ValueType>>(this));
       }
 
     // modifiers
diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index 5e796ac..3f540ec 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -592,20 +592,22 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
         }
 
       template<typename... _Args>
-	enable_if_t<is_constructible<_Tp, _Args&&...>::value>
+	enable_if_t<is_constructible<_Tp, _Args&&...>::value, _Tp&>
 	emplace(_Args&&... __args)
 	{
 	  this->_M_reset();
 	  this->_M_construct(std::forward<_Args>(__args)...);
+	  return this->_M_get();
 	}
 
       template<typename _Up, typename... _Args>
 	enable_if_t<is_constructible<_Tp, initializer_list<_Up>&,
-				     _Args&&...>::value>
+				     _Args&&...>::value, _Tp&>
 	emplace(initializer_list<_Up> __il, _Args&&... __args)
 	{
 	  this->_M_reset();
 	  this->_M_construct(__il, std::forward<_Args>(__args)...);
+	  return this->_M_get();
 	}
 
       // Destructor is implicit, implemented in _Optional_base.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 46d7b92..58bf8c7 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -1007,25 +1007,33 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	}
 
       template<typename _Tp, typename... _Args>
-	enable_if_t<is_constructible_v<_Tp, _Args...> && __exactly_once<_Tp>>
+	enable_if_t<is_constructible_v<_Tp, _Args...> && __exactly_once<_Tp>,
+		    _Tp&>
 	emplace(_Args&&... __args)
 	{
-	  this->emplace<__index_of<_Tp>>(std::forward<_Args>(__args)...);
+	  auto& ret =
+	    this->emplace<__index_of<_Tp>>(std::forward<_Args>(__args)...);
 	  __glibcxx_assert(holds_alternative<_Tp>(*this));
+	  return ret;
 	}
 
       template<typename _Tp, typename _Up, typename... _Args>
 	enable_if_t<is_constructible_v<_Tp, initializer_list<_Up>&, _Args...>
-		    && __exactly_once<_Tp>>
+		    && __exactly_once<_Tp>,
+		    _Tp&>
 	emplace(initializer_list<_Up> __il, _Args&&... __args)
 	{
-	  this->emplace<__index_of<_Tp>>(__il, std::forward<_Args>(__args)...);
+	  auto& ret =
+	    this->emplace<__index_of<_Tp>>(__il,
+					   std::forward<_Args>(__args)...);
 	  __glibcxx_assert(holds_alternative<_Tp>(*this));
+	  return ret;
 	}
 
       template<size_t _Np, typename... _Args>
 	enable_if_t<is_constructible_v<variant_alternative_t<_Np, variant>,
-				       _Args...>>
+				       _Args...>,
+		    variant_alternative_t<_Np, variant>&>
 	emplace(_Args&&... __args)
 	{
 	  static_assert(_Np < sizeof...(_Types),
@@ -1042,11 +1050,13 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      __throw_exception_again;
 	    }
 	  __glibcxx_assert(index() == _Np);
+	  return std::get<_Np>(*this);
 	}
 
       template<size_t _Np, typename _Up, typename... _Args>
 	enable_if_t<is_constructible_v<variant_alternative_t<_Np, variant>,
-				       initializer_list<_Up>&, _Args...>>
+				       initializer_list<_Up>&, _Args...>,
+		    variant_alternative_t<_Np, variant>&>
 	emplace(initializer_list<_Up> __il, _Args&&... __args)
 	{
 	  static_assert(_Np < sizeof...(_Types),
@@ -1063,6 +1073,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      __throw_exception_again;
 	    }
 	  __glibcxx_assert(index() == _Np);
+	  return std::get<_Np>(*this);
 	}
 
       constexpr bool valueless_by_exception() const noexcept
diff --git a/libstdc++-v3/testsuite/20_util/any/assign/emplace.cc b/libstdc++-v3/testsuite/20_util/any/assign/emplace.cc
index 07cbdde..119104b 100644
--- a/libstdc++-v3/testsuite/20_util/any/assign/emplace.cc
+++ b/libstdc++-v3/testsuite/20_util/any/assign/emplace.cc
@@ -72,4 +72,8 @@  int main()
   std::any o10;
   o10.emplace<char*>(nullptr);
   VERIFY(o9.type() == o10.type());
+  std::any o11;
+  VERIFY(&o11.emplace<int>(42) == &std::any_cast<int&>(o11));
+  VERIFY(&o11.emplace<std::vector<int>>({1,2,3}) ==
+	 &std::any_cast<std::vector<int>&>(o11));
 }
diff --git a/libstdc++-v3/testsuite/20_util/any/misc/any_cast_neg.cc b/libstdc++-v3/testsuite/20_util/any/misc/any_cast_neg.cc
index 61f0bc4..2d2b3d3 100644
--- a/libstdc++-v3/testsuite/20_util/any/misc/any_cast_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/any/misc/any_cast_neg.cc
@@ -26,5 +26,5 @@  void test01()
   using std::any_cast;
 
   const any y(1);
-  any_cast<int&>(y); // { dg-error "invalid static_cast" "" { target { *-*-* } } 455 }
+  any_cast<int&>(y); // { dg-error "invalid static_cast" "" { target { *-*-* } } 461 }
 }
diff --git a/libstdc++-v3/testsuite/20_util/optional/assignment/6.cc b/libstdc++-v3/testsuite/20_util/optional/assignment/6.cc
index 4d754c1..40a537a 100644
--- a/libstdc++-v3/testsuite/20_util/optional/assignment/6.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/assignment/6.cc
@@ -76,6 +76,11 @@  int main()
     o.emplace({ 'a' }, "");
     VERIFY( o && o->state == 2 );
   }
+  {
+    O o;
+    VERIFY(&o.emplace(0) == &*o);
+    VERIFY(&o.emplace({ 'a' }, "") == &*o);
+  }
 
   static_assert( !std::is_constructible<O, std::initializer_list<int>, int>(), "" );
 
diff --git a/libstdc++-v3/testsuite/20_util/variant/run.cc b/libstdc++-v3/testsuite/20_util/variant/run.cc
index db4529e..c6c2bc9 100644
--- a/libstdc++-v3/testsuite/20_util/variant/run.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/run.cc
@@ -189,6 +189,15 @@  void emplace()
     try { v.emplace<1>(AlwaysThrow{}); } catch (nullptr_t) { }
     VERIFY(v.valueless_by_exception());
   }
+  VERIFY(&v.emplace<0>(1) == &std::get<0>(v));
+  VERIFY(&v.emplace<int>(1) == &std::get<int>(v));
+  VERIFY(&v.emplace<1>("a") == &std::get<1>(v));
+  VERIFY(&v.emplace<string>("a") == &std::get<string>(v));
+  {
+    variant<vector<int>> v;
+    VERIFY(&v.emplace<0>({1,2,3}) == &std::get<0>(v));
+    VERIFY(&v.emplace<vector<int>>({1,2,3}) == &std::get<vector<int>>(v));
+  }
 }
 
 void test_get()