[libcxx-commits] [libcxx] [libc++] Add [[nodiscard]] to std::prev and std::next (PR #109550)
Marc Auberer via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Sep 28 04:49:31 PDT 2024
https://github.com/marcauberer updated https://github.com/llvm/llvm-project/pull/109550
>From ea7e0a70b997fb09ec1798941e4310f7a30a3ff5 Mon Sep 17 00:00:00 2001
From: Marc Auberer <marc.auberer at chillibits.com>
Date: Sat, 21 Sep 2024 22:47:15 +0200
Subject: [PATCH 1/8] [libc++] Add [[nodiscard]] to std::prev and std::next
---
libcxx/include/__iterator/next.h | 8 ++++----
libcxx/include/__iterator/prev.h | 6 +++---
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/libcxx/include/__iterator/next.h b/libcxx/include/__iterator/next.h
index fb6c8ea6d75508..4064bcec4e183b 100644
--- a/libcxx/include/__iterator/next.h
+++ b/libcxx/include/__iterator/next.h
@@ -43,25 +43,25 @@ 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;
}
diff --git a/libcxx/include/__iterator/prev.h b/libcxx/include/__iterator/prev.h
index e950d8dc414717..0b88d33d924cfe 100644
--- a/libcxx/include/__iterator/prev.h
+++ b/libcxx/include/__iterator/prev.h
@@ -42,19 +42,19 @@ 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;
}
>From e988c95b6a9e57138cd9d4efb485eff9108baac3 Mon Sep 17 00:00:00 2001
From: Marc Auberer <marc.auberer at chillibits.com>
Date: Sat, 21 Sep 2024 22:52:50 +0200
Subject: [PATCH 2/8] Format
---
libcxx/include/__iterator/next.h | 3 ++-
libcxx/include/__iterator/prev.h | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/libcxx/include/__iterator/next.h b/libcxx/include/__iterator/next.h
index 4064bcec4e183b..27b21e6f52c272 100644
--- a/libcxx/include/__iterator/next.h
+++ b/libcxx/include/__iterator/next.h
@@ -61,7 +61,8 @@ struct __next {
}
template <input_or_output_iterator _Ip, sentinel_for<_Ip> _Sp>
- [[nodiscard]] _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;
}
diff --git a/libcxx/include/__iterator/prev.h b/libcxx/include/__iterator/prev.h
index 0b88d33d924cfe..6ee9f08308660e 100644
--- a/libcxx/include/__iterator/prev.h
+++ b/libcxx/include/__iterator/prev.h
@@ -54,7 +54,8 @@ struct __prev {
}
template <bidirectional_iterator _Ip>
- [[nodiscard]] _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;
}
>From 9f8f94e983e22d3d97dec285453af9d8e4894aa2 Mon Sep 17 00:00:00 2001
From: Marc Auberer <marc.auberer at chillibits.com>
Date: Wed, 25 Sep 2024 23:25:13 +0200
Subject: [PATCH 3/8] Add tests
---
libcxx/include/__iterator/next.h | 2 +-
libcxx/include/__iterator/prev.h | 2 +-
.../diagnostics/iterator.nodiscard.verify.cpp | 14 +++++++++++---
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/libcxx/include/__iterator/next.h b/libcxx/include/__iterator/next.h
index 27b21e6f52c272..1f68a5bec8f39c 100644
--- a/libcxx/include/__iterator/next.h
+++ b/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`.
diff --git a/libcxx/include/__iterator/prev.h b/libcxx/include/__iterator/prev.h
index 6ee9f08308660e..7e97203836eb98 100644
--- a/libcxx/include/__iterator/prev.h
+++ b/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`.
diff --git a/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp
index 8f9bc3e411f907..b84a35baca85f0 100644
--- a/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp
@@ -20,7 +20,15 @@ void test() {
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}}
+ std::ranges::prev(c_array); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ auto pv = std::ranges::prev(container.end(), 2); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ std::ranges::next(pv, 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}}
+ auto nv = std::ranges::next(container.begin(), 2); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ std::ranges::next(pv, 1, container.end()); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
}
>From 7d6bd723a4e3819c46a22c83de9ef8a12d41f6f9 Mon Sep 17 00:00:00 2001
From: Marc Auberer <marc.auberer at chillibits.com>
Date: Wed, 25 Sep 2024 23:29:46 +0200
Subject: [PATCH 4/8] Fix unused variable
---
libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp
index b84a35baca85f0..42244f8717019e 100644
--- a/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp
@@ -30,5 +30,5 @@ void test() {
std::ranges::next(pv, 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}}
auto nv = std::ranges::next(container.begin(), 2); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
- std::ranges::next(pv, 1, container.end()); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ std::ranges::next(nv, 1, container.end()); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
}
>From e2dfff2312ee57385925b2324d5e7b71feb6597f Mon Sep 17 00:00:00 2001
From: Marc Auberer <marc.auberer at chillibits.com>
Date: Wed, 25 Sep 2024 23:43:55 +0200
Subject: [PATCH 5/8] Fix problem in test
---
.../diagnostics/iterator.nodiscard.verify.cpp | 22 +++++++++----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp
index 42244f8717019e..9819bd82c46e85 100644
--- a/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp
@@ -20,15 +20,15 @@ void test() {
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::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}}
- std::ranges::prev(c_array); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
- auto pv = std::ranges::prev(container.end(), 2); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
- std::ranges::next(pv, 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}}
- auto nv = std::ranges::next(container.begin(), 2); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
- std::ranges::next(nv, 1, container.end()); // 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}}
+ 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}}
}
>From 0d7fd0bfdee133be27be7273da580162bbad400a Mon Sep 17 00:00:00 2001
From: Marc Auberer <marc.auberer at chillibits.com>
Date: Fri, 27 Sep 2024 20:52:58 +0200
Subject: [PATCH 6/8] Fix CI
---
libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp
index 9819bd82c46e85..c7cd2f5ce57674 100644
--- a/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/iterator.nodiscard.verify.cpp
@@ -15,6 +15,8 @@
#include <iterator>
#include <vector>
+#include "test_macros.h"
+
void test() {
std::vector<int> container;
int c_array[] = {1, 2, 3};
@@ -25,10 +27,12 @@ void test() {
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
}
>From 9f467cc41e3345acfa83e9fab9cd6ed751bead89 Mon Sep 17 00:00:00 2001
From: Marc Auberer <marc.auberer at chillibits.com>
Date: Sat, 28 Sep 2024 13:40:54 +0200
Subject: [PATCH 7/8] Ignore return value for existing test cases
---
libcxx/test/libcxx/iterators/assert.next.pass.cpp | 4 ++--
libcxx/test/libcxx/iterators/assert.prev.pass.cpp | 10 +++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/libcxx/test/libcxx/iterators/assert.next.pass.cpp b/libcxx/test/libcxx/iterators/assert.next.pass.cpp
index 242a0c6f0f7ce4..c3f43e4e1f4eec 100644
--- a/libcxx/test/libcxx/iterators/assert.next.pass.cpp
+++ b/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;
diff --git a/libcxx/test/libcxx/iterators/assert.prev.pass.cpp b/libcxx/test/libcxx/iterators/assert.prev.pass.cpp
index a5a04f1bbeb6be..b758839a21f6ca 100644
--- a/libcxx/test/libcxx/iterators/assert.prev.pass.cpp
+++ b/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;
>From a1ac81aee2f4e0133cfa0da4d34596619b506793 Mon Sep 17 00:00:00 2001
From: Marc Auberer <marc.auberer at chillibits.com>
Date: Sat, 28 Sep 2024 13:49:18 +0200
Subject: [PATCH 8/8] Format
---
libcxx/test/libcxx/iterators/assert.next.pass.cpp | 4 ++--
libcxx/test/libcxx/iterators/assert.prev.pass.cpp | 10 +++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/libcxx/test/libcxx/iterators/assert.next.pass.cpp b/libcxx/test/libcxx/iterators/assert.next.pass.cpp
index c3f43e4e1f4eec..f6fd24284bbfd0 100644
--- a/libcxx/test/libcxx/iterators/assert.next.pass.cpp
+++ b/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);
- (void) std::next(it, 1); // should work fine
- (void) 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;
diff --git a/libcxx/test/libcxx/iterators/assert.prev.pass.cpp b/libcxx/test/libcxx/iterators/assert.prev.pass.cpp
index b758839a21f6ca..08cbe5e03dd5fa 100644
--- a/libcxx/test/libcxx/iterators/assert.prev.pass.cpp
+++ b/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);
- (void) std::prev(bidi, -1); // should work fine
- (void) std::prev(bidi, 0); // should work fine
- (void) 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);
- (void) std::prev(it, -1); // should work fine
- (void) 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;
More information about the libcxx-commits
mailing list