[PATCH] PR libstdc++/86751 default assignment operators for std::pair
Jonathan Wakely
jwakely@redhat.com
Mon Oct 15 11:53:00 GMT 2018
On 31/07/18 23:34 +0100, Jonathan Wakely wrote:
>On 31/07/18 18:40 +0100, Jonathan Wakely wrote:
>>On 31/07/18 20:14 +0300, Ville Voutilainen wrote:
>>>On 31 July 2018 at 20:07, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>>The solution for PR 77537 causes ambiguities due to the extra copy
>>>>assignment operator taking a __nonesuch_no_braces parameter. The copy
>>>>and move assignment operators can be defined as defaulted to meet the
>>>>semantics required by the standard.
>>>>
>>>>In order to preserve ABI compatibility (specifically argument passing
>>>>conventions for pair<T, T>) we need a new base class that makes the
>>>>assignment operators non-trivial.
>>>>
>>>> PR libstdc++/86751
>>>> * include/bits/stl_pair.h (__nonesuch_no_braces): Remove.
>>>> (__pair_base): New class with non-trivial copy assignment operator.
>>>> (pair): Derive from __pair_base. Define copy assignment and move
>>>> assignment operators as defaulted.
>>>> * testsuite/20_util/pair/86751.cc: New test.
>>>>
>>>>
>>>>Ville, this passes all our tests, but am I forgetting something that
>>>>means this isn't right?
>>>
>>>Pairs of references?
>>
>>I knew there was a reason.
>>
>>We need better tests, since nothing failed when I made this change.
>>
>>OK, let me rework the patch ...
>
>Here's the patch I've committed. It adds a test for pairs of
>references, so I don't try to define t he assignment ops as defaulted
>again :-) Thanks for the quick feedback for these patches.
>
>Tested powerpc64le-linux, committed to trunk.
>
>This is a regression on all branches, but I'd like to leave it on
>trunk for a short while before backporting it.
Now backported to all active branches.
>commit 988a9158fd074353621f4f216270109c767a4725
>Author: Jonathan Wakely <jwakely@redhat.com>
>Date: Tue Jul 31 17:26:04 2018 +0100
>
> PR libstdc++/86751 default assignment operators for std::pair
>
> The solution for PR 77537 causes ambiguities due to the extra copy
> assignment operator taking a __nonesuch_no_braces parameter. By making
> the base class non-assignable we don't need the extra deleted overload
> in std::pair. The copy assignment operator will be implicitly deleted
> (and the move assignment operator not declared) as needed. Without the
> additional user-provided operator in std::pair the ambiguity is avoided.
>
> PR libstdc++/86751
> * include/bits/stl_pair.h (__pair_base): New class with deleted copy
> assignment operator.
> (pair): Derive from __pair_base.
> (pair::operator=): Remove deleted overload.
> * python/libstdcxx/v6/printers.py (StdPairPrinter): New pretty printer
> so that new base class isn't shown in GDB.
> * testsuite/20_util/pair/86751.cc: New test.
> * testsuite/20_util/pair/ref_assign.cc: New test.
>
>diff --git a/libstdc++-v3/include/bits/stl_pair.h b/libstdc++-v3/include/bits/stl_pair.h
>index a2486ba8244..ea8bd981559 100644
>--- a/libstdc++-v3/include/bits/stl_pair.h
>+++ b/libstdc++-v3/include/bits/stl_pair.h
>@@ -185,8 +185,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> struct __nonesuch_no_braces : std::__nonesuch {
> explicit __nonesuch_no_braces(const __nonesuch&) = delete;
> };
>+#endif // C++11
>
>-#endif
>+ class __pair_base
>+ {
>+#if __cplusplus >= 201103L
>+ template<typename _T1, typename _T2> friend struct pair;
>+ __pair_base() = default;
>+ ~__pair_base() = default;
>+ __pair_base(const __pair_base&) = default;
>+ __pair_base& operator=(const __pair_base&) = delete;
>+#endif // C++11
>+ };
>
> /**
> * @brief Struct holding two objects of arbitrary type.
>@@ -196,6 +206,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> */
> template<typename _T1, typename _T2>
> struct pair
>+ : private __pair_base
> {
> typedef _T1 first_type; /// @c first_type is the first bound type
> typedef _T2 second_type; /// @c second_type is the second bound type
>@@ -374,19 +385,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> return *this;
> }
>
>- pair&
>- operator=(typename conditional<
>- __not_<__and_<is_copy_assignable<_T1>,
>- is_copy_assignable<_T2>>>::value,
>- const pair&, const __nonesuch_no_braces&>::type __p) = delete;
>-
> pair&
> operator=(typename conditional<
> __and_<is_move_assignable<_T1>,
> is_move_assignable<_T2>>::value,
> pair&&, __nonesuch_no_braces&&>::type __p)
> noexcept(__and_<is_nothrow_move_assignable<_T1>,
>- is_nothrow_move_assignable<_T2>>::value)
>+ is_nothrow_move_assignable<_T2>>::value)
> {
> first = std::forward<first_type>(__p.first);
> second = std::forward<second_type>(__p.second);
>diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
>index 34d8b4e6606..43d459ec8ec 100644
>--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
>+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
>@@ -1229,6 +1229,39 @@ class StdExpPathPrinter:
> return self._iterator(self.val['_M_cmpts'])
>
>
>+class StdPairPrinter:
>+ "Print a std::pair object, with 'first' and 'second' as children"
>+
>+ def __init__(self, typename, val):
>+ self.val = val
>+
>+ class _iter(Iterator):
>+ "An iterator for std::pair types. Returns 'first' then 'second'."
>+
>+ def __init__(self, val):
>+ self.val = val
>+ self.which = 'first'
>+
>+ def __iter__(self):
>+ return self
>+
>+ def __next__(self):
>+ if self.which is None:
>+ raise StopIteration
>+ which = self.which
>+ if which == 'first':
>+ self.which = 'second'
>+ else:
>+ self.which = None
>+ return (which, self.val[which])
>+
>+ def children(self):
>+ return self._iter(self.val)
>+
>+ def to_string(self):
>+ return None
>+
>+
> # A "regular expression" printer which conforms to the
> # "SubPrettyPrinter" protocol from gdb.printing.
> class RxPrinter(object):
>@@ -1629,6 +1662,7 @@ def build_libstdcxx_dictionary ():
> libstdcxx_printer.add_container('std::', 'map', StdMapPrinter)
> libstdcxx_printer.add_container('std::', 'multimap', StdMapPrinter)
> libstdcxx_printer.add_container('std::', 'multiset', StdSetPrinter)
>+ libstdcxx_printer.add_version('std::', 'pair', StdPairPrinter)
> libstdcxx_printer.add_version('std::', 'priority_queue',
> StdStackOrQueuePrinter)
> libstdcxx_printer.add_version('std::', 'queue', StdStackOrQueuePrinter)
>diff --git a/libstdc++-v3/testsuite/20_util/pair/86751.cc b/libstdc++-v3/testsuite/20_util/pair/86751.cc
>new file mode 100644
>index 00000000000..76a76c0d656
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/20_util/pair/86751.cc
>@@ -0,0 +1,33 @@
>+// Copyright (C) 2018 Free Software Foundation, Inc.
>+//
>+// This file is part of the GNU ISO C++ Library. This library is free
>+// software; you can redistribute it and/or modify it under the
>+// terms of the GNU General Public License as published by the
>+// Free Software Foundation; either version 3, or (at your option)
>+// any later version.
>+
>+// This library is distributed in the hope that it will be useful,
>+// but WITHOUT ANY WARRANTY; without even the implied warranty of
>+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+// GNU General Public License for more details.
>+
>+// You should have received a copy of the GNU General Public License along
>+// with this library; see the file COPYING3. If not see
>+// <http://www.gnu.org/licenses/>.
>+
>+// { dg-do compile { target c++11 } }
>+
>+#include <utility>
>+
>+struct X {
>+ template<typename T> operator T() const;
>+};
>+
>+
>+void
>+test01()
>+{
>+ std::pair<int, int> p;
>+ X x;
>+ p = x; // PR libstdc++/86751
>+}
>diff --git a/libstdc++-v3/testsuite/20_util/pair/ref_assign.cc b/libstdc++-v3/testsuite/20_util/pair/ref_assign.cc
>new file mode 100644
>index 00000000000..ea37fcfcf52
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/20_util/pair/ref_assign.cc
>@@ -0,0 +1,74 @@
>+// Copyright (C) 2018 Free Software Foundation, Inc.
>+//
>+// This file is part of the GNU ISO C++ Library. This library is free
>+// software; you can redistribute it and/or modify it under the
>+// terms of the GNU General Public License as published by the
>+// Free Software Foundation; either version 3, or (at your option)
>+// any later version.
>+
>+// This library is distributed in the hope that it will be useful,
>+// but WITHOUT ANY WARRANTY; without even the implied warranty of
>+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+// GNU General Public License for more details.
>+
>+// You should have received a copy of the GNU General Public License along
>+// with this library; see the file COPYING3. If not see
>+// <http://www.gnu.org/licenses/>.
>+
>+// { dg-do run { target c++11 } }
>+
>+#include <utility>
>+#include <testsuite_hooks.h>
>+
>+void
>+test01()
>+{
>+ typedef std::pair<int&, int> pair_type;
>+ int i = 1;
>+ int j = 2;
>+ pair_type p(i, 3);
>+ const pair_type q(j, 4);
>+ p = q;
>+ VERIFY( p.first == q.first );
>+ VERIFY( p.second == q.second );
>+ VERIFY( i == j );
>+}
>+
>+void
>+test02()
>+{
>+ typedef std::pair<int, int&> pair_type;
>+ int i = 1;
>+ int j = 2;
>+ pair_type p(3, i);
>+ const pair_type q(4, j);
>+ p = q;
>+ VERIFY( p.first == q.first );
>+ VERIFY( p.second == q.second );
>+ VERIFY( i == j );
>+}
>+
>+void
>+test03()
>+{
>+ typedef std::pair<int&, int&> pair_type;
>+ int i = 1;
>+ int j = 2;
>+ int k = 3;
>+ int l = 4;
>+ pair_type p(i, j);
>+ const pair_type q(k, l);
>+ p = q;
>+ VERIFY( p.first == q.first );
>+ VERIFY( p.second == q.second );
>+ VERIFY( i == k );
>+ VERIFY( j == l );
>+}
>+
>+int
>+main()
>+{
>+ test01();
>+ test02();
>+ test03();
>+}
More information about the Libstdc++
mailing list