public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Patrick Palka <ppalka@redhat.com>
To: gcc-patches@gcc.gnu.org
Cc: libstdc++@gcc.gnu.org, Patrick Palka <ppalka@redhat.com>
Subject: [PATCH] libstdc++: Fix constexpr _Safe_iterator in C++20 mode
Date: Wed, 17 Jan 2024 21:48:06 -0500	[thread overview]
Message-ID: <20240118024806.436235-1-ppalka@redhat.com> (raw)

Tested on x86_64-pc-linux-gnu, does this look OK for trunk?

-- >8 --

Some _Safe_iterator member functions define a variable of non-literal
type __gnu_cxx::__scoped_lock, which automatically disqualifies them from
being constexpr in C++20 mode even if that code path is never constant
evaluated.  This restriction was lifted by P2242R3 for C++23, but we
need to work around it in C++20 mode.  To that end this patch defines
a pair of macros that encapsulate the lambda-based workaround mentioned
in that paper and uses them to make the functions valid C++20 constexpr
functions.  The augmented std::vector test element_access/constexpr.cc
now successfully compiles in C++20 mode with -D_GLIBCXX_DEBUG (and it
tests all modified member functions).

libstdc++-v3/ChangeLog:

	* include/debug/safe_base.h (_Safe_sequence_base::_M_swap):
	Remove _GLIBCXX20_CONSTEXPR.
	* include/debug/safe_iterator.h (_GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN):
	(_GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END): Define.
	(_Safe_iterator::operator=): Use them around the code path that
	defines a variable of type __gnu_cxx::__scoped_lock.
	(_Safe_iterator::operator++): Likewise.
	(_Safe_iterator::operator--): Likewise.
	(_Safe_iterator::operator+=): Likewise.
	(_Safe_iterator::operator-=): Likewise.
	* testsuite/23_containers/vector/element_access/constexpr.cc
	(test_iterators): Also test copy and move assignment.
	* testsuite/std/ranges/adaptors/all.cc (test08) [_GLIBCXX_DEBUG]:
	Use std::vector unconditionally.
---
 libstdc++-v3/include/debug/safe_base.h        |  1 -
 libstdc++-v3/include/debug/safe_iterator.h    | 48 ++++++++++++++-----
 .../vector/element_access/constexpr.cc        |  2 +
 .../testsuite/std/ranges/adaptors/all.cc      |  4 --
 4 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/libstdc++-v3/include/debug/safe_base.h b/libstdc++-v3/include/debug/safe_base.h
index 107fef3cb02..d5fbe4b1320 100644
--- a/libstdc++-v3/include/debug/safe_base.h
+++ b/libstdc++-v3/include/debug/safe_base.h
@@ -268,7 +268,6 @@ namespace __gnu_debug
      *  operation is complete all iterators that originally referenced
      *  one container now reference the other container.
      */
-    _GLIBCXX20_CONSTEXPR
     void
     _M_swap(_Safe_sequence_base& __x) _GLIBCXX_USE_NOEXCEPT;
 
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index 1bc7c904ee0..929fd9b0ade 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -65,6 +65,20 @@
   _GLIBCXX_DEBUG_VERIFY_OPERANDS(_Lhs, _Rhs, __msg_distance_bad,	\
 				 __msg_distance_different)
 
+// This pair of macros helps with writing valid C++20 constexpr functions that
+// contain a non-constexpr code path that defines a non-literal variable, which
+// was otherwise disallowed until P2242R3 for C++23.  We use them below for
+// __gnu_cxx::__scoped_lock so that the containing functions are still
+// considered valid C++20 constexpr functions.
+
+#if __cplusplus >= 202002L && __cpp_constexpr < 202110L
+# define _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN [&]() -> void { do
+# define _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END while(false); }();
+#else
+# define _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN
+# define _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END
+#endif
+
 namespace __gnu_debug
 {
   /** Helper struct to deal with sequence offering a before_begin
@@ -266,11 +280,11 @@ namespace __gnu_debug
 			      ._M_iterator(__x, "other"));
 
 	if (this->_M_sequence && this->_M_sequence == __x._M_sequence)
-	  {
+	  _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN {
 	    __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
 	    base() = __x.base();
 	    _M_version = __x._M_sequence->_M_version;
-	  }
+	  } _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END
 	else
 	  {
 	    _M_detach();
@@ -306,11 +320,11 @@ namespace __gnu_debug
 	  return *this;
 
 	if (this->_M_sequence && this->_M_sequence == __x._M_sequence)
-	  {
+	  _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN {
 	    __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
 	    base() = __x.base();
 	    _M_version = __x._M_sequence->_M_version;
-	  }
+	  } _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END
 	else
 	  {
 	    _M_detach();
@@ -378,8 +392,10 @@ namespace __gnu_debug
 	_GLIBCXX_DEBUG_VERIFY(this->_M_incrementable(),
 			      _M_message(__msg_bad_inc)
 			      ._M_iterator(*this, "this"));
-	__gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
-	++base();
+	_GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN {
+	  __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
+	  ++base();
+	} _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END
 	return *this;
       }
 
@@ -697,8 +713,10 @@ namespace __gnu_debug
 	_GLIBCXX_DEBUG_VERIFY(this->_M_decrementable(),
 			      _M_message(__msg_bad_dec)
 			      ._M_iterator(*this, "this"));
-	__gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
-	--this->base();
+	_GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN {
+	  __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
+	  --this->base();
+	} _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END
 	return *this;
       }
 
@@ -912,8 +930,10 @@ namespace __gnu_debug
 	_GLIBCXX_DEBUG_VERIFY(this->_M_can_advance(__n),
 			      _M_message(__msg_advance_oob)
 			      ._M_iterator(*this)._M_integer(__n));
-	__gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
-	this->base() += __n;
+	_GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN {
+	  __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
+	  this->base() += __n;
+	} _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END
 	return *this;
       }
 
@@ -930,8 +950,10 @@ namespace __gnu_debug
 	_GLIBCXX_DEBUG_VERIFY(this->_M_can_advance(-__n),
 			      _M_message(__msg_retreat_oob)
 			      ._M_iterator(*this)._M_integer(__n));
-	__gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
-	this->base() -= __n;
+	_GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN {
+	  __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
+	  this->base() -= __n;
+	} _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END
 	return *this;
       }
 
@@ -1156,6 +1178,8 @@ _GLIBCXX_END_NAMESPACE_VERSION
 }
 #endif
 
+#undef _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_END
+#undef _GLIBCXX20_CONSTEXPR_NON_LITERAL_SCOPE_BEGIN
 #undef _GLIBCXX_DEBUG_VERIFY_DIST_OPERANDS
 #undef _GLIBCXX_DEBUG_VERIFY_REL_OPERANDS
 #undef _GLIBCXX_DEBUG_VERIFY_EQ_OPERANDS
diff --git a/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc b/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc
index ee93d2fd95e..ab1e7f1bb70 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/element_access/constexpr.cc
@@ -25,6 +25,8 @@ test_iterators()
   it -= 2;
   it += 1;
   VERIFY( (it + 1) == v.end() );
+  it = it + 1;
+  it = it;
 
   auto rit = v.rbegin();
   VERIFY( &*rit == &v.back() );
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc
index e7010f80e18..5f7206dc8c3 100644
--- a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc
@@ -156,11 +156,7 @@ test07()
 constexpr bool
 test08()
 {
-#ifdef _GLIBCXX_DEBUG
-  using std::_GLIBCXX_STD_C::vector;
-#else
   using std::vector;
-#endif
 
   // Verify P2415R2 "What is a view?" changes.
   // In particular, rvalue non-view non-borrowed ranges are now viewable.
-- 
2.43.0.367.g186b115d30


             reply	other threads:[~2024-01-18  2:48 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-18  2:48 Patrick Palka [this message]
2024-01-18 10:30 ` Jonathan Wakely
2024-01-18 13:51   ` Patrick Palka
2024-01-18 14:17     ` Jonathan Wakely

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=20240118024806.436235-1-ppalka@redhat.com \
    --to=ppalka@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).