[libc++] Implements LWG3600 Making istream_iterator copy constructor trivial is an ABI break

[mordante]

Closes: #105003

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

…trivial is an ABI break

Closes: #105003

---

Full diff: https://github.com/llvm/llvm-project/pull/127343.diff

4 Files Affected:

• (modified) libcxx/docs/Status/Cxx23Issues.csv (+1-1)
• (modified) libcxx/include/__iterator/istream_iterator.h (+3)
• (modified) libcxx/include/iterator (+1-1)
• (modified) libcxx/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/copy.pass.cpp (+31-4)

diff --git a/libcxx/docs/Status/Cxx23Issues.csv b/libcxx/docs/Status/Cxx23Issues.csv
index 1215f21985eb9..9ae6322b7fdbe 100644
--- a/libcxx/docs/Status/Cxx23Issues.csv
+++ b/libcxx/docs/Status/Cxx23Issues.csv
@@ -194,7 +194,7 @@
 "`LWG3569 <https://wg21.link/LWG3569>`__","``join_view`` fails to support ranges of ranges with non-default_initializable iterators","2022-11 (Kona)","","",""
 "`LWG3594 <https://wg21.link/LWG3594>`__","``inout_ptr`` — inconsistent ``release()`` in destructor","2022-11 (Kona)","|Complete|","19",""
 "`LWG3597 <https://wg21.link/LWG3597>`__","Unsigned integer types don't model advanceable","2022-11 (Kona)","","",""
-"`LWG3600 <https://wg21.link/LWG3600>`__","Making ``istream_iterator`` copy constructor trivial is an ABI break","2022-11 (Kona)","","",""
+"`LWG3600 <https://wg21.link/LWG3600>`__","Making ``istream_iterator`` copy constructor trivial is an ABI break","2022-11 (Kona)","|Nothing To Do|","",""
 "`LWG3629 <https://wg21.link/LWG3629>`__","``make_error_code`` and ``make_error_condition`` are customization points","2022-11 (Kona)","|Complete|","16",""
 "`LWG3636 <https://wg21.link/LWG3636>`__","``formatter<T>::format`` should be ``const``-qualified","2022-11 (Kona)","|Complete|","16",""
 "`LWG3646 <https://wg21.link/LWG3646>`__","``std::ranges::view_interface::size`` returns a signed type","2022-11 (Kona)","|Complete|","16",""
diff --git a/libcxx/include/__iterator/istream_iterator.h b/libcxx/include/__iterator/istream_iterator.h
index a6c74d00178d2..430fada300960 100644
--- a/libcxx/include/__iterator/istream_iterator.h
+++ b/libcxx/include/__iterator/istream_iterator.h
@@ -58,6 +58,9 @@ class _LIBCPP_TEMPLATE_VIS istream_iterator
       __in_stream_ = nullptr;
   }
 
+  // LWG3600 Changed the wording of the copy constructor. In libc++ this constructor
+  // can still be trivial after this change.
+
   _LIBCPP_HIDE_FROM_ABI const _Tp& operator*() const { return __value_; }
   _LIBCPP_HIDE_FROM_ABI const _Tp* operator->() const { return std::addressof((operator*())); }
   _LIBCPP_HIDE_FROM_ABI istream_iterator& operator++() {
diff --git a/libcxx/include/iterator b/libcxx/include/iterator
index 74ee712b945b3..d25fdfd2a8b33 100644
--- a/libcxx/include/iterator
+++ b/libcxx/include/iterator
@@ -530,7 +530,7 @@ public:
     istream_iterator(); // constexpr since C++11
     constexpr istream_iterator(default_sentinel_t); // since C++20
     istream_iterator(istream_type& s);
-    istream_iterator(const istream_iterator& x);
+    constexpr istream_iterator(const istream_iterator& x) noexcept(see below);
     ~istream_iterator();
 
     const T& operator*() const;
diff --git a/libcxx/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/copy.pass.cpp b/libcxx/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/copy.pass.cpp
index d6a3b0862879a..050f4ac7cc6d5 100644
--- a/libcxx/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/copy.pass.cpp
+++ b/libcxx/test/std/iterators/stream.iterators/istream.iterator/istream.iterator.cons/copy.pass.cpp
@@ -10,9 +10,7 @@
 
 // class istream_iterator
 
-// istream_iterator(const istream_iterator& x);
-//  C++17 says:  If is_trivially_copy_constructible_v<T> is true, then
-//     this constructor is a trivial copy constructor.
+// istream_iterator(const istream_iterator& x) noexcept(see below);
 
 #include <iterator>
 #include <sstream>
@@ -20,12 +18,37 @@
 
 #include "test_macros.h"
 
+// The copy constructor is constexpr in C++11, but that is not easy to test.
+// The comparison of the class is not constexpr so this is only a compile test.
+TEST_CONSTEXPR_CXX14 bool test_constexpr() {
+  std::istream_iterator<int> io;
+  [[maybe_unused]] std::istream_iterator<int> i = io;
+
+  return true;
+}
+
+struct thowing_copy_constructor {
+  thowing_copy_constructor() {}
+  thowing_copy_constructor(const thowing_copy_constructor&) TEST_NOEXCEPT_FALSE {}
+};
+
 int main(int, char**)
 {
     {
         std::istream_iterator<int> io;
         std::istream_iterator<int> i = io;
         assert(i == std::istream_iterator<int>());
+#if TEST_STD_VER >= 11
+        static_assert(std::is_nothrow_copy_constructible<std::istream_iterator<int>>::value, "");
+#endif
+    }
+    {
+      std::istream_iterator<thowing_copy_constructor> io;
+      std::istream_iterator<thowing_copy_constructor> i = io;
+      assert(i == std::istream_iterator<thowing_copy_constructor>());
+#if TEST_STD_VER >= 11
+      static_assert(!std::is_nothrow_copy_constructible<std::istream_iterator<thowing_copy_constructor>>::value, "");
+#endif
     }
     {
         std::istringstream inf(" 1 23");
@@ -37,5 +60,9 @@ int main(int, char**)
         assert(j == 1);
     }
 
-  return 0;
+#if TEST_STD_VER >= 14
+    static_assert(test_constexpr(), "");
+#endif
+
+    return 0;
 }

[philnik777]

I'd much rather just mention that our copy construct or trivial as a conforming extension. I don't think it's useful in the future to know which LWG issue changed that.

[mordante]

I added the LWG issue number to let the reader know we did look at this constructor for LWG3600.

[philnik777]

I'm not sure what you're trying to say with "we did look at this constructor for LWG3600". Do you mean we checked whether we're conforming because LWG3600 exists? If you mean that, I'm really not sure why anybody would care.

[mordante]

I care a lot. In the past we had several LWG issues closed as Completed or Nothing To Do when that was not correct. By explicitly mentioning it, it will safe time when somebody notices a difference between libc++ and the Standard.