[libc++] Add [[nodiscard]] to std::prev and std::next (#109550)

Add `[[nodiscard]]` attribute to `std::prev` and `std::next`. Those are
potential pitfalls for users who might think they mutate the iterator.

Fixes #109452

Author: marcauberer

libcxx/include/__iterator/next.h

@@ -25,7 +25,7 @@
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _InputIter, __enable_if_t<__has_input_iterator_category<_InputIter>::value, int> = 0>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _InputIter
+[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _InputIter
 next(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n = 1) {
   // Calling `advance` with a negative value on a non-bidirectional iterator is a no-op in the current implementation.
   // Note that this check duplicates the similar check in `std::advance`.
@@ -43,25 +43,26 @@ next(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n =
 namespace ranges {
 struct __next {
   template <input_or_output_iterator _Ip>
-  _LIBCPP_HIDE_FROM_ABI constexpr _Ip operator()(_Ip __x) const {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Ip operator()(_Ip __x) const {
     ++__x;
     return __x;
   }
 
   template <input_or_output_iterator _Ip>
-  _LIBCPP_HIDE_FROM_ABI constexpr _Ip operator()(_Ip __x, iter_difference_t<_Ip> __n) const {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Ip operator()(_Ip __x, iter_difference_t<_Ip> __n) const {
     ranges::advance(__x, __n);
     return __x;
   }
 
   template <input_or_output_iterator _Ip, sentinel_for<_Ip> _Sp>
-  _LIBCPP_HIDE_FROM_ABI constexpr _Ip operator()(_Ip __x, _Sp __bound_sentinel) const {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Ip operator()(_Ip __x, _Sp __bound_sentinel) const {
     ranges::advance(__x, __bound_sentinel);
     return __x;
   }
 
   template <input_or_output_iterator _Ip, sentinel_for<_Ip> _Sp>
-  _LIBCPP_HIDE_FROM_ABI constexpr _Ip operator()(_Ip __x, iter_difference_t<_Ip> __n, _Sp __bound_sentinel) const {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Ip
+  operator()(_Ip __x, iter_difference_t<_Ip> __n, _Sp __bound_sentinel) const {
     ranges::advance(__x, __n, __bound_sentinel);
     return __x;
   }

libcxx/include/__iterator/prev.h

@@ -25,7 +25,7 @@
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _InputIter, __enable_if_t<__has_input_iterator_category<_InputIter>::value, int> = 0>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _InputIter
+[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _InputIter
 prev(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n = 1) {
   // Calling `advance` with a negative value on a non-bidirectional iterator is a no-op in the current implementation.
   // Note that this check duplicates the similar check in `std::advance`.
@@ -42,19 +42,20 @@ prev(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n =
 namespace ranges {
 struct __prev {
   template <bidirectional_iterator _Ip>
-  _LIBCPP_HIDE_FROM_ABI constexpr _Ip operator()(_Ip __x) const {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Ip operator()(_Ip __x) const {
     --__x;
     return __x;
   }
 
   template <bidirectional_iterator _Ip>
-  _LIBCPP_HIDE_FROM_ABI constexpr _Ip operator()(_Ip __x, iter_difference_t<_Ip> __n) const {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Ip operator()(_Ip __x, iter_difference_t<_Ip> __n) const {
     ranges::advance(__x, -__n);
     return __x;
   }
 
   template <bidirectional_iterator _Ip>
-  _LIBCPP_HIDE_FROM_ABI constexpr _Ip operator()(_Ip __x, iter_difference_t<_Ip> __n, _Ip __bound_iter) const {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Ip
+  operator()(_Ip __x, iter_difference_t<_Ip> __n, _Ip __bound_iter) const {
     ranges::advance(__x, -__n, __bound_iter);
     return __x;
   }

libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp

@@ -15,12 +15,24 @@
 #include <iterator>
 #include <vector>
 
+#include "test_macros.h"
+
 void test() {
   std::vector<int> container;
   int c_array[] = {1, 2, 3};
   std::initializer_list<int> initializer_list;
 
-  std::empty(container);        // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
-  std::empty(c_array);          // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
-  std::empty(initializer_list); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::empty(container);                                     // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::empty(c_array);                                       // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::empty(initializer_list);                              // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::prev(c_array);                                        // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::next(c_array);                                        // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#if TEST_STD_VER >= 20
+  std::ranges::prev(c_array);                                // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::ranges::prev(container.end(), 2);                     // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::ranges::next(container.end(), 2, container.begin());  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::ranges::next(c_array);                                // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::ranges::next(container.begin(), 2);                   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::ranges::next(container.end(), 1, container.end());    // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
 }

libcxx/test/libcxx/iterators/assert.next.pass.cpp

@@ -23,8 +23,8 @@
 int main(int, char**) {
     int a[] = {1, 2, 3};
     forward_iterator<int *> it(a+1);
-    std::next(it, 1);  // should work fine
-    std::next(it, 0);  // should work fine
+    (void)std::next(it, 1); // should work fine
+    (void)std::next(it, 0); // should work fine
     TEST_LIBCPP_ASSERT_FAILURE(std::next(it, -1), "Attempt to next(it, n) with negative n on a non-bidirectional iterator");
 
     return 0;

libcxx/test/libcxx/iterators/assert.prev.pass.cpp

@@ -24,13 +24,13 @@ int main(int, char**) {
     int a[] = {1, 2, 3};
 
     bidirectional_iterator<int *> bidi(a+1);
-    std::prev(bidi, -1);  // should work fine
-    std::prev(bidi,  0);  // should work fine
-    std::prev(bidi,  1);  // should work fine
+    (void)std::prev(bidi, -1); // should work fine
+    (void)std::prev(bidi, 0);  // should work fine
+    (void)std::prev(bidi, 1);  // should work fine
 
     forward_iterator<int *> it(a+1);
-    std::prev(it, -1); // should work fine
-    std::prev(it,  0); // should work fine
+    (void)std::prev(it, -1); // should work fine
+    (void)std::prev(it, 0);  // should work fine
     TEST_LIBCPP_ASSERT_FAILURE(std::prev(it, 1), "Attempt to prev(it, n) with a positive n on a non-bidirectional iterator");
 
     return 0;