public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [committed] libstdc++: Make __gnu_cxx::sequence_buffer move-aware [PR101542]
@ 2021-07-21 16:21 Jonathan Wakely
  0 siblings, 0 replies; only message in thread
From: Jonathan Wakely @ 2021-07-21 16:21 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1114 bytes --]

The PR explains that Clang trunk now selects a different constructor
when a non-const sequence_buffer is returned in a context where it
qualifies as an implicitly-movable entity. Because lookup is first
performed using an rvalue, the sequence_buffer(const sequence_buffer&)
constructor gets chosen, which makes a copy instead of a "pseudo-move"
via the sequence_buffer(sequence_buffer&) constructor. The problem isn't
seen with GCC because as noted in the r11-2412 commit log, GCC actually
implements a slightly modified rule that avoids breaking exactly this
type of code.

This patch adds a move constructor to sequence_buffer, so that implicit
or explicit moves will have the same effect, calling the
sequence_buffer(sequence_buffer&) constructor. A move assignment
operator is also added to make move assignment work similarly.

Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

libstdc++-v3/ChangeLog:

	PR libstdc++/101542
	* include/ext/rope (sequence_buffer): Add move constructor and
	move assignment operator.
	* testsuite/ext/rope/101542.cc: New test.

Tested powerpc64le-linux. Committed to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3383 bytes --]

commit 8edb61420502c62fa2cccdd98876a9aa039b72a6
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jul 21 15:29:19 2021

    libstdc++: Make __gnu_cxx::sequence_buffer move-aware [PR101542]
    
    The PR explains that Clang trunk now selects a different constructor
    when a non-const sequence_buffer is returned in a context where it
    qualifies as an implicitly-movable entity. Because lookup is first
    performed using an rvalue, the sequence_buffer(const sequence_buffer&)
    constructor gets chosen, which makes a copy instead of a "pseudo-move"
    via the sequence_buffer(sequence_buffer&) constructor. The problem isn't
    seen with GCC because as noted in the r11-2412 commit log, GCC actually
    implements a slightly modified rule that avoids breaking exactly this
    type of code.
    
    This patch adds a move constructor to sequence_buffer, so that implicit
    or explicit moves will have the same effect, calling the
    sequence_buffer(sequence_buffer&) constructor. A move assignment
    operator is also added to make move assignment work similarly.
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/101542
            * include/ext/rope (sequence_buffer): Add move constructor and
            move assignment operator.
            * testsuite/ext/rope/101542.cc: New test.

diff --git a/libstdc++-v3/include/ext/rope b/libstdc++-v3/include/ext/rope
index 81e4f23708f..9681dbc6225 100644
--- a/libstdc++-v3/include/ext/rope
+++ b/libstdc++-v3/include/ext/rope
@@ -203,6 +203,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	std::copy(__x._M_buffer, __x._M_buffer + __x._M_buf_count, _M_buffer);
       }
       
+      // Non-const "copy" modifies the parameter - yuck
       sequence_buffer(sequence_buffer& __x)
       {
 	__x.flush();
@@ -213,6 +214,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       sequence_buffer(_Sequence& __s)
       : _M_prefix(&__s), _M_buf_count(0) { }
       
+      // Non-const "copy" modifies the parameter - yuck
       sequence_buffer&
       operator=(sequence_buffer& __x)
       {
@@ -230,7 +232,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	std::copy(__x._M_buffer, __x._M_buffer + __x._M_buf_count, _M_buffer);
 	return *this;
       }
-      
+
+#if __cplusplus >= 201103L
+      sequence_buffer(sequence_buffer&& __x) : sequence_buffer(__x) { }
+      sequence_buffer& operator=(sequence_buffer&& __x) { return *this = __x; }
+#endif
+
       void
       push_back(value_type __x)
       {
diff --git a/libstdc++-v3/testsuite/ext/rope/101542.cc b/libstdc++-v3/testsuite/ext/rope/101542.cc
new file mode 100644
index 00000000000..e89f23d3d48
--- /dev/null
+++ b/libstdc++-v3/testsuite/ext/rope/101542.cc
@@ -0,0 +1,27 @@
+// { dg-do run { target c++11 } }
+// PR libstdc++/101542
+#include <ext/rope>
+#include <testsuite_hooks.h>
+
+template<typename T> T f(T x) { return x; }
+template<typename T> T g(T x) { return std::move(x); }
+
+int main()
+{
+  std::string s;
+  {
+    __gnu_cxx::sequence_buffer<std::string> a(s);
+    {
+      __gnu_cxx::sequence_buffer<std::string> b = std::move(a);
+      b.push_back('h');
+      b.push_back('e');
+      b.push_back('l');
+      b.push_back('l');
+      b.push_back('o');
+
+      __gnu_cxx::sequence_buffer<std::string> c;
+      c = f(g((std::move(b))));
+    }
+  }
+  VERIFY( s == "hello" );
+}

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-07-21 16:21 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-21 16:21 [committed] libstdc++: Make __gnu_cxx::sequence_buffer move-aware [PR101542] Jonathan Wakely

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).