From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gcc-bugzilla@gcc.gnu.org>
Received: by sourceware.org (Postfix, from userid 48)
	id 944443858C5F; Tue, 28 Mar 2023 16:35:39 +0000 (GMT)
DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 944443858C5F
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org;
	s=default; t=1680021339;
	bh=cZBbt9v6Uves1tfqp6oyqwW+4vVynF7X4vT+L72Xk1Y=;
	h=From:To:Subject:Date:In-Reply-To:References:From;
	b=YTfAsrTfTMHbDTUxIvI+viZqQ4cOVOzT6FDb2p/HcPZY4w760H+luq4Yob/KTTeRz
	 kUD8Sfm6BzGfSbiJBVhaXTpM02j9lFZ1jDYZdZTL8fSzShOtGXERvhBCbgZnuJG8hw
	 uD15i5VfM+r+QEMQArOvAWso6VCVUn7UX+rXz8Cw=
From: "redi at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug libstdc++/109305] Allocator copy in basic_string::operator=
Date: Tue, 28 Mar 2023 16:35:38 +0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: libstdc++
X-Bugzilla-Version: 12.1.0
X-Bugzilla-Keywords: 
X-Bugzilla-Severity: normal
X-Bugzilla-Who: redi at gcc dot gnu.org
X-Bugzilla-Status: RESOLVED
X-Bugzilla-Resolution: INVALID
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags: 
X-Bugzilla-Changed-Fields: 
Message-ID: <bug-109305-4-OJdOS8S4Dx@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-109305-4@http.gcc.gnu.org/bugzilla/>
References: <bug-109305-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
List-Id: <gcc-bugs.sourceware.org>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D109305

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #5)
> Removing it would make the code less efficient and more complex.

In fact, I don't even see how it would be possible, except by making *anoth=
er*
copy, e.g.

--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -1588,20 +1588,33 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
                    _M_destroy(_M_allocated_capacity);
                    _M_data(_M_use_local_data());
                    _M_set_length(0);
+                   std::__alloc_on_copy(_M_get_allocator(),
+                                        __str._M_get_allocator());
                  }
                else
                  {
+                   auto __backup_alloc =3D _M_get_allocator();
+                   std::__alloc_on_copy(_M_get_allocator(),
+                                        __str._M_get_allocator());
                    const auto __len =3D __str.size();
-                   auto __alloc =3D __str._M_get_allocator();
-                   // If this allocation throws there are no effects:
-                   auto __ptr =3D _Alloc_traits::allocate(__alloc, __len +=
 1);
-                   _M_destroy(_M_allocated_capacity);
+                   pointer __ptr;
+                   __try {
+                     __ptr =3D _Alloc_traits::allocate(_M_get_allocator(),
+                                                     __len + 1);
+                   } __catch (...) {
+                     std::__alloc_on_copy(_M_get_allocator(), __backup_all=
oc);
+                     __throw_exception_again;
+                   }
+                   _Alloc_traits::deallocate(__backup_alloc, _M_data(),
+                                             _M_allocated_capacity + 1);
                    _M_data(__ptr);
                    _M_capacity(__len);
                    _M_set_length(__len);
                  }
              }
-           std::__alloc_on_copy(_M_get_allocator(), __str._M_get_allocator=
());
+           else
+             std::__alloc_on_copy(_M_get_allocator(),
+                                  __str._M_get_allocator());
          }
 #endif
        this->_M_assign(__str);

This makes the code much worse, and slower, and still has a copy (the
__backup_alloc) variable.=