libstdc++: Fix minor bugs in std::common_iterator
authorJonathan Wakely <jwakely@redhat.com>
Wed, 20 Jul 2022 11:49:28 +0000 (12:49 +0100)
committerJonathan Wakely <jwakely@redhat.com>
Wed, 20 Jul 2022 22:34:20 +0000 (23:34 +0100)
The noexcept-specifier for some std::common_iterator constructors was
incorrectly using an rvalue as the first argument of
std::is_nothrow_assignable_v. This gave the wrong answer for some types,
e.g. std::common_iterator<int*, S>, because an rvalue of scalar type
cannot be assigned to.

Also fix the friend declaration to use the same constraints as on the
definition of the class template. G++ fails to diagnose this error, due
to PR c++/96830.

Finally, the copy constructor was using std::move for its argument
in some cases, which should be removed.

libstdc++-v3/ChangeLog:

* include/bits/stl_iterator.h (common_iterator): Fix incorrect
uses of is_nothrow_assignable_v. Fix inconsistent constraints on
friend declaration. Do not move argument in copy constructor.
* testsuite/24_iterators/common_iterator/1.cc: Check for
noexcept constructibnle/assignable.

libstdc++-v3/include/bits/stl_iterator.h
libstdc++-v3/testsuite/24_iterators/common_iterator/1.cc

index 049cb02..fd0ae3a 100644 (file)
@@ -1838,7 +1838,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _S_noexcept1()
       {
        if constexpr (is_trivially_default_constructible_v<_Tp>)
-         return is_nothrow_assignable_v<_Tp, _Up>;
+         return is_nothrow_assignable_v<_Tp&, _Up>;
        else
          return is_nothrow_constructible_v<_Tp, _Up>;
       }
@@ -1932,14 +1932,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       if (_M_index == 0)
        {
          if constexpr (is_trivially_default_constructible_v<_It>)
-           _M_it = std::move(__x._M_it);
+           _M_it = __x._M_it;
          else
            std::construct_at(std::__addressof(_M_it), __x._M_it);
        }
       else if (_M_index == 1)
        {
          if constexpr (is_trivially_default_constructible_v<_Sent>)
-           _M_sent = std::move(__x._M_sent);
+           _M_sent = __x._M_sent;
          else
            std::construct_at(std::__addressof(_M_sent), __x._M_sent);
        }
@@ -1964,8 +1964,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       operator=(const common_iterator<_It2, _Sent2>& __x)
       noexcept(is_nothrow_constructible_v<_It, const _It2&>
               && is_nothrow_constructible_v<_Sent, const _Sent2&>
-              && is_nothrow_assignable_v<_It, const _It2&>
-              && is_nothrow_assignable_v<_Sent, const _Sent2&>)
+              && is_nothrow_assignable_v<_It&, const _It2&>
+              && is_nothrow_assignable_v<_Sent&, const _Sent2&>)
       {
        switch(_M_index << 2 | __x._M_index)
          {
@@ -2164,6 +2164,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   private:
     template<input_or_output_iterator _It2, sentinel_for<_It2> _Sent2>
+      requires (!same_as<_It2, _Sent2>) && copyable<_It2>
       friend class common_iterator;
 
     constexpr bool _M_has_value() const noexcept { return _M_index < 2; }
index 365ee89..ec4a86c 100644 (file)
@@ -27,15 +27,30 @@ test01()
   using I = std::common_iterator<int*, const int*>;
   static_assert( std::is_default_constructible_v<I> );
   static_assert( std::is_copy_constructible_v<I> );
+  static_assert( std::is_move_constructible_v<I> );
   static_assert( std::is_copy_assignable_v<I> );
+  static_assert( std::is_move_assignable_v<I> );
   static_assert( std::is_constructible_v<I, int*> );
   static_assert( std::is_constructible_v<I, const int*> );
 
-  struct sentinel { operator int*() const { return nullptr; } };
+  static_assert( std::is_nothrow_copy_constructible_v<I> ); // GCC extension
+  static_assert( std::is_nothrow_move_constructible_v<I> ); // GCC extension
+  static_assert( std::is_nothrow_copy_assignable_v<I> ); // GCC extension
+  static_assert( std::is_nothrow_move_assignable_v<I> ); // GCC extension
+
+  struct sentinel { operator int*() const noexcept { return nullptr; } };
   using K = std::common_iterator<int*, sentinel>;
   static_assert( std::is_constructible_v<I, const K&> );
   static_assert( std::is_assignable_v<I, const K&> );
 
+  static_assert( std::is_nothrow_assignable_v<I&, const K&> ); // GCC extension
+
+  struct sentinel_throwing { operator int*() const { return nullptr; } };
+  using K_throwing = std::common_iterator<int*, sentinel_throwing>;
+  // Conversion is noexcept(false)
+  static_assert( ! std::is_nothrow_assignable_v<I&, const K_throwing&> );
+
+
   struct sentinel2
   {
     const int* p;
@@ -46,6 +61,12 @@ test01()
   using J = std::common_iterator<const int*, sentinel2>;
   static_assert( std::is_constructible_v<J, const I&> );
   static_assert( std::is_convertible_v<const I&, J> );
+
+  static_assert( std::is_constructible_v<J, I> );
+  static_assert( std::is_convertible_v<I, J> );
+
+  // Constructor is noexcept(false)
+  static_assert( ! std::is_nothrow_constructible_v<J, I> );
 }
 
 void