public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely@redhat.com>
To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org
Subject: [committed 3/3] libstdc++: Refactor emplace-like functions in std::variant
Date: Thu,  4 Nov 2021 09:42:09 +0000	[thread overview]
Message-ID: <20211104094209.1930691-3-jwakely@redhat.com> (raw)
In-Reply-To: <20211104094209.1930691-1-jwakely@redhat.com>

Tested powerpc64le-linux, pushed to trunk.


libstdc++-v3/ChangeLog:

	* include/std/variant (__detail::__variant::__emplace): New
	function template.
	(_Copy_assign_base::operator=): Reorder conditions to match
	bulleted list of effects in the standard. Use __emplace instead
	of _M_reset followed by _Construct.
	(_Move_assign_base::operator=): Likewise.
	(__construct_by_index): Remove.
	(variant::emplace): Use __emplace instead of _M_reset followed
	by __construct_by_index.
	(variant::swap): Hoist valueless cases out of visitor. Use
	__emplace to replace _M_reset followed by _Construct.
---
 libstdc++-v3/include/std/variant | 179 ++++++++++++++-----------------
 1 file changed, 82 insertions(+), 97 deletions(-)

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index dc3d032c543..c4c307b7bb2 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -590,6 +590,19 @@ namespace __variant
       __index_type _M_index;
     };
 
+  // Implementation of v.emplace<N>(args...).
+  template<size_t _Np, bool _Triv, typename... _Types, typename... _Args>
+    _GLIBCXX20_CONSTEXPR
+    inline void
+    __emplace(_Variant_storage<_Triv, _Types...>& __v, _Args&&... __args)
+    {
+      __v._M_reset();
+      auto* __addr = std::__addressof(__variant::__get_n<_Np>(__v._M_u));
+      std::_Construct(__addr, std::forward<_Args>(__args)...);
+      // Construction didn't throw, so can set the new index now:
+      __v._M_index = _Np;
+    }
+
   template<typename... _Types>
     using _Variant_storage_alias =
 	_Variant_storage<_Traits<_Types...>::_S_trivial_dtor, _Types...>;
@@ -655,6 +668,7 @@ namespace __variant
 	  }, __variant_cast<_Types...>(std::move(__rhs)));
 	this->_M_index = __rhs._M_index;
       }
+
       _Move_ctor_base(const _Move_ctor_base&) = default;
       _Move_ctor_base& operator=(const _Move_ctor_base&) = default;
       _Move_ctor_base& operator=(_Move_ctor_base&&) = default;
@@ -685,36 +699,24 @@ namespace __variant
 	__variant::__raw_idx_visit(
 	  [this](auto&& __rhs_mem, auto __rhs_index) mutable
 	  {
-	    if constexpr (__rhs_index != variant_npos)
+	    constexpr size_t __j = __rhs_index;
+	    if constexpr (__j == variant_npos)
+	      this->_M_reset(); // Make *this valueless.
+	    else if (this->_M_index == __j)
+	      __variant::__get<__j>(*this) = __rhs_mem;
+	    else
 	      {
-		if (this->_M_index == __rhs_index)
-		  __variant::__get<__rhs_index>(*this) = __rhs_mem;
+		using _Tj = typename _Nth_type<__j, _Types...>::type;
+		if constexpr (is_nothrow_copy_constructible_v<_Tj>
+			      || !is_nothrow_move_constructible_v<_Tj>)
+		  __variant::__emplace<__j>(*this, __rhs_mem);
 		else
 		  {
-		    using __rhs_type = __remove_cvref_t<decltype(__rhs_mem)>;
-		    if constexpr (is_nothrow_copy_constructible_v<__rhs_type>
-			|| !is_nothrow_move_constructible_v<__rhs_type>)
-		      {
-			// The standard says emplace<__rhs_index>(__rhs_mem)
-			// should be used here, but this is equivalent. Either
-			// copy construction doesn't throw, so we have strong
-			// exception safety guarantee, or both copy construction
-			// and move construction can throw, so emplace only
-			// gives basic exception safety anyway.
-			this->_M_reset();
-			std::_Construct(std::__addressof(this->_M_u),
-					in_place_index<__rhs_index>,
-					__rhs_mem);
-			this->_M_index = __rhs_index;
-		      }
-		    else
-		      __variant_cast<_Types...>(*this)
-			= variant<_Types...>(std::in_place_index<__rhs_index>,
-					     __rhs_mem);
+		    using _Variant = variant<_Types...>;
+		    _Variant& __self = __variant_cast<_Types...>(*this);
+		    __self = _Variant(in_place_index<__j>, __rhs_mem);
 		  }
 	      }
-	    else
-	      this->_M_reset();
 	  }, __variant_cast<_Types...>(__rhs));
 	return *this;
       }
@@ -749,13 +751,23 @@ namespace __variant
 	__variant::__raw_idx_visit(
 	  [this](auto&& __rhs_mem, auto __rhs_index) mutable
 	  {
-	    if constexpr (__rhs_index != variant_npos)
+	    constexpr size_t __j = __rhs_index;
+	    if constexpr (__j != variant_npos)
 	      {
-		if (this->_M_index == __rhs_index)
-		  __variant::__get<__rhs_index>(*this) = std::move(__rhs_mem);
+		if (this->_M_index == __j)
+		  __variant::__get<__j>(*this) = std::move(__rhs_mem);
 		else
-		  __variant_cast<_Types...>(*this)
-		    .template emplace<__rhs_index>(std::move(__rhs_mem));
+		  {
+		    using _Tj = typename _Nth_type<__j, _Types...>::type;
+		    if constexpr (is_nothrow_move_constructible_v<_Tj>)
+		      __variant::__emplace<__j>(*this, std::move(__rhs_mem));
+		    else
+		      {
+			using _Variant = variant<_Types...>;
+			_Variant& __self = __variant_cast<_Types...>(*this);
+			__self.template emplace<__j>(std::move(__rhs_mem));
+		      }
+		  }
 	      }
 	    else
 	      this->_M_reset();
@@ -1164,17 +1176,6 @@ namespace __variant
 	>;
     }
 
-  template<size_t _Np, typename _Variant, typename... _Args>
-    _GLIBCXX20_CONSTEXPR
-    inline void
-    __construct_by_index(_Variant& __v, _Args&&... __args)
-    {
-      std::_Construct(std::__addressof(__variant::__get<_Np>(__v)),
-		      std::forward<_Args>(__args)...);
-      // Construction didn't throw, so can set the new index now:
-      __v._M_index = _Np;
-    }
-
 } // namespace __variant
 } // namespace __detail
 
@@ -1415,11 +1416,6 @@ namespace __variant
 	friend _GLIBCXX20_CONSTEXPR decltype(auto)
 	__variant_cast(_Tp&&);
 
-      template<size_t _Np, typename _Variant, typename... _Args>
-	friend _GLIBCXX20_CONSTEXPR void
-	__detail::__variant::__construct_by_index(_Variant& __v,
-						  _Args&&... __args);
-
       static_assert(sizeof...(_Types) > 0,
 		    "variant must have at least one alternative");
       static_assert(!(std::is_reference_v<_Types> || ...),
@@ -1589,17 +1585,14 @@ namespace __variant
 	  // to avoid becoming valueless.
 	  if constexpr (is_nothrow_constructible_v<type, _Args...>)
 	    {
-	      this->_M_reset();
-	      __variant::__construct_by_index<_Np>(*this,
-		  std::forward<_Args>(__args)...);
+	      __variant::__emplace<_Np>(*this, std::forward<_Args>(__args)...);
 	    }
 	  else if constexpr (is_scalar_v<type>)
 	    {
 	      // This might invoke a potentially-throwing conversion operator:
 	      const type __tmp(std::forward<_Args>(__args)...);
-	      // But these steps won't throw:
-	      this->_M_reset();
-	      __variant::__construct_by_index<_Np>(*this, __tmp);
+	      // But this won't throw:
+	      __variant::__emplace<_Np>(*this, __tmp);
 	    }
 	  else if constexpr (__variant::_Never_valueless_alt<type>()
 	      && _Traits::_S_move_assign)
@@ -1614,9 +1607,7 @@ namespace __variant
 	    {
 	      // This case only provides the basic exception-safety guarantee,
 	      // i.e. the variant can become valueless.
-	      this->_M_reset();
-	      __variant::__construct_by_index<_Np>(*this,
-		std::forward<_Args>(__args)...);
+	      __variant::__emplace<_Np>(*this, std::forward<_Args>(__args)...);
 	    }
 	  return std::get<_Np>(*this);
 	}
@@ -1636,9 +1627,8 @@ namespace __variant
 						   initializer_list<_Up>&,
 						   _Args...>)
 	    {
-	      this->_M_reset();
-	      __variant::__construct_by_index<_Np>(*this, __il,
-		  std::forward<_Args>(__args)...);
+	      __variant::__emplace<_Np>(*this, __il,
+					std::forward<_Args>(__args)...);
 	    }
 	  else if constexpr (__variant::_Never_valueless_alt<type>()
 	      && _Traits::_S_move_assign)
@@ -1653,9 +1643,8 @@ namespace __variant
 	    {
 	      // This case only provides the basic exception-safety guarantee,
 	      // i.e. the variant can become valueless.
-	      this->_M_reset();
-	      __variant::__construct_by_index<_Np>(*this, __il,
-		std::forward<_Args>(__args)...);
+	      __variant::__emplace<_Np>(*this, __il,
+					std::forward<_Args>(__args)...);
 	    }
 	  return std::get<_Np>(*this);
 	}
@@ -1686,61 +1675,57 @@ namespace __variant
       noexcept((__is_nothrow_swappable<_Types>::value && ...)
 	       && is_nothrow_move_constructible_v<variant>)
       {
-	__detail::__variant::__raw_idx_visit(
+	static_assert((is_move_constructible_v<_Types> && ...));
+
+	// Handle this here to simplify the visitation.
+	if (__rhs.valueless_by_exception()) [[__unlikely__]]
+	  {
+	    if (!this->valueless_by_exception()) [[__likely__]]
+	      __rhs.swap(*this);
+	    return;
+	  }
+
+	namespace __variant = __detail::__variant;
+
+	__variant::__raw_idx_visit(
 	  [this, &__rhs](auto&& __rhs_mem, auto __rhs_index) mutable
 	  {
-	    if constexpr (__rhs_index != variant_npos)
+	    constexpr size_t __j = __rhs_index;
+	    if constexpr (__j != variant_npos)
 	      {
-		if (this->index() == __rhs_index)
+		if (this->index() == __j)
 		  {
-		    auto& __this_mem =
-		      std::get<__rhs_index>(*this);
 		    using std::swap;
-		    swap(__this_mem, __rhs_mem);
+		    swap(std::get<__j>(*this), __rhs_mem);
 		  }
 		else
 		  {
-		    constexpr size_t __j = __rhs_index;
-		    if (!this->valueless_by_exception()) [[__likely__]]
-		      {
-			auto __tmp(std::move(__rhs_mem));
-			__rhs = std::move(*this);
-			this->_M_reset();
-			std::_Construct(std::__addressof(this->_M_u),
-					in_place_index<__j>,
-					std::move(__tmp));
-			this->_M_index = __j;
-		      }
+		    auto __tmp(std::move(__rhs_mem));
+
+		    if constexpr (_Traits::_S_trivial_move_assign)
+		      __rhs = std::move(*this);
 		    else
-		      {
-			this->_M_reset();
-			std::_Construct(std::__addressof(this->_M_u),
-					in_place_index<__j>,
-					std::move(__rhs_mem));
-			this->_M_index = __j;
-			__rhs._M_reset();
-		      }
-		  }
-	      }
-	    else
-	      {
-		if (!this->valueless_by_exception()) [[__likely__]]
-		  {
-		    __rhs = std::move(*this);
-		    this->_M_reset();
+		      __variant::__raw_idx_visit(
+			[&__rhs](auto&& __this_mem, auto __this_index) mutable
+			{
+			  constexpr size_t __k = __this_index;
+			  if constexpr (__k != variant_npos)
+			    __variant::__emplace<__k>(__rhs,
+						      std::move(__this_mem));
+			}, *this);
+
+		    __variant::__emplace<__j>(*this, std::move(__tmp));
 		  }
 	      }
 	  }, __rhs);
       }
 
-    private:
-
 #if defined(__clang__) && __clang_major__ <= 7
     public:
       using _Base::_M_u; // See https://bugs.llvm.org/show_bug.cgi?id=31852
-    private:
 #endif
 
+    private:
       template<size_t _Np, typename _Vp>
 	friend constexpr decltype(auto)
 	__detail::__variant::__get(_Vp&& __v) noexcept;
-- 
2.31.1


      parent reply	other threads:[~2021-11-04  9:42 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-04  9:42 [committed 1/3] libstdc++: Fix handling of const types in std::variant [PR102912] Jonathan Wakely
2021-11-04  9:42 ` [committed 2/3] libstdc++: Optimize std::variant traits and improve diagnostics Jonathan Wakely
2021-11-04  9:42 ` Jonathan Wakely [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211104094209.1930691-3-jwakely@redhat.com \
    --to=jwakely@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).