Skip to content

Conversation

@H-G-Hristov
Copy link
Contributor

[[nodiscard]] should be applied to functions where discarding the return value is most likely a correctness issue.

`[[nodiscard]]` should be applied to functions where discarding the return value is most likely a correctness issue.

- https://libcxx.llvm.org/CodingGuidelines.htm
- https://wg21.link/map
@H-G-Hristov H-G-Hristov requested a review from a team as a code owner November 29, 2025 09:56
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 29, 2025
@llvmbot
Copy link
Member

llvmbot commented Nov 29, 2025

@llvm/pr-subscribers-libcxx

Author: Hristo Hristov (H-G-Hristov)

Changes

[[nodiscard]] should be applied to functions where discarding the return value is most likely a correctness issue.


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

3 Files Affected:

  • (modified) libcxx/include/set (+49-39)
  • (added) libcxx/test/libcxx/diagnostics/multiset.nodiscard.verify.cpp (+18)
  • (modified) libcxx/test/libcxx/diagnostics/set.nodiscard.verify.cpp (+96-8)
diff --git a/libcxx/include/set b/libcxx/include/set
index 3d6f571a42a1a..54a24e74ff52e 100644
--- a/libcxx/include/set
+++ b/libcxx/include/set
@@ -701,24 +701,28 @@ public:
 
   _LIBCPP_HIDE_FROM_ABI ~set() { static_assert(sizeof(std::__diagnose_non_const_comparator<_Key, _Compare>()), ""); }
 
-  _LIBCPP_HIDE_FROM_ABI iterator begin() _NOEXCEPT { return __tree_.begin(); }
-  _LIBCPP_HIDE_FROM_ABI const_iterator begin() const _NOEXCEPT { return __tree_.begin(); }
-  _LIBCPP_HIDE_FROM_ABI iterator end() _NOEXCEPT { return __tree_.end(); }
-  _LIBCPP_HIDE_FROM_ABI const_iterator end() const _NOEXCEPT { return __tree_.end(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI iterator begin() _NOEXCEPT { return __tree_.begin(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator begin() const _NOEXCEPT { return __tree_.begin(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI iterator end() _NOEXCEPT { return __tree_.end(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator end() const _NOEXCEPT { return __tree_.end(); }
 
-  _LIBCPP_HIDE_FROM_ABI reverse_iterator rbegin() _NOEXCEPT { return reverse_iterator(end()); }
-  _LIBCPP_HIDE_FROM_ABI const_reverse_iterator rbegin() const _NOEXCEPT { return const_reverse_iterator(end()); }
-  _LIBCPP_HIDE_FROM_ABI reverse_iterator rend() _NOEXCEPT { return reverse_iterator(begin()); }
-  _LIBCPP_HIDE_FROM_ABI const_reverse_iterator rend() const _NOEXCEPT { return const_reverse_iterator(begin()); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI reverse_iterator rbegin() _NOEXCEPT { return reverse_iterator(end()); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_reverse_iterator rbegin() const _NOEXCEPT {
+    return const_reverse_iterator(end());
+  }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI reverse_iterator rend() _NOEXCEPT { return reverse_iterator(begin()); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_reverse_iterator rend() const _NOEXCEPT {
+    return const_reverse_iterator(begin());
+  }
 
-  _LIBCPP_HIDE_FROM_ABI const_iterator cbegin() const _NOEXCEPT { return begin(); }
-  _LIBCPP_HIDE_FROM_ABI const_iterator cend() const _NOEXCEPT { return end(); }
-  _LIBCPP_HIDE_FROM_ABI const_reverse_iterator crbegin() const _NOEXCEPT { return rbegin(); }
-  _LIBCPP_HIDE_FROM_ABI const_reverse_iterator crend() const _NOEXCEPT { return rend(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator cbegin() const _NOEXCEPT { return begin(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator cend() const _NOEXCEPT { return end(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_reverse_iterator crbegin() const _NOEXCEPT { return rbegin(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_reverse_iterator crend() const _NOEXCEPT { return rend(); }
 
   [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool empty() const _NOEXCEPT { return __tree_.size() == 0; }
-  _LIBCPP_HIDE_FROM_ABI size_type size() const _NOEXCEPT { return __tree_.size(); }
-  _LIBCPP_HIDE_FROM_ABI size_type max_size() const _NOEXCEPT { return __tree_.max_size(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type size() const _NOEXCEPT { return __tree_.size(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type max_size() const _NOEXCEPT { return __tree_.max_size(); }
 
   // modifiers:
 #  ifndef _LIBCPP_CXX03_LANG
@@ -777,10 +781,10 @@ public:
                                         "node_type with incompatible allocator passed to set::insert()");
     return __tree_.template __node_handle_insert_unique<node_type>(__hint, std::move(__nh));
   }
-  _LIBCPP_HIDE_FROM_ABI node_type extract(key_type const& __key) {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI node_type extract(key_type const& __key) {
     return __tree_.template __node_handle_extract<node_type>(__key);
   }
-  _LIBCPP_HIDE_FROM_ABI node_type extract(const_iterator __it) {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI node_type extract(const_iterator __it) {
     return __tree_.template __node_handle_extract<node_type>(__it);
   }
   template <class _Compare2>
@@ -811,43 +815,47 @@ public:
 
   _LIBCPP_HIDE_FROM_ABI void swap(set& __s) _NOEXCEPT_(__is_nothrow_swappable_v<__base>) { __tree_.swap(__s.__tree_); }
 
-  _LIBCPP_HIDE_FROM_ABI allocator_type get_allocator() const _NOEXCEPT { return __tree_.__alloc(); }
-  _LIBCPP_HIDE_FROM_ABI key_compare key_comp() const { return __tree_.value_comp(); }
-  _LIBCPP_HIDE_FROM_ABI value_compare value_comp() const { return __tree_.value_comp(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI allocator_type get_allocator() const _NOEXCEPT { return __tree_.__alloc(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI key_compare key_comp() const { return __tree_.value_comp(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI value_compare value_comp() const { return __tree_.value_comp(); }
 
   // set operations:
-  _LIBCPP_HIDE_FROM_ABI iterator find(const key_type& __k) { return __tree_.find(__k); }
-  _LIBCPP_HIDE_FROM_ABI const_iterator find(const key_type& __k) const { return __tree_.find(__k); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI iterator find(const key_type& __k) { return __tree_.find(__k); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator find(const key_type& __k) const { return __tree_.find(__k); }
 #  if _LIBCPP_STD_VER >= 14
   template <typename _K2, enable_if_t<__is_transparent_v<_Compare, _K2>, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI iterator find(const _K2& __k) {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI iterator find(const _K2& __k) {
     return __tree_.find(__k);
   }
   template <typename _K2, enable_if_t<__is_transparent_v<_Compare, _K2>, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI const_iterator find(const _K2& __k) const {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator find(const _K2& __k) const {
     return __tree_.find(__k);
   }
 #  endif
 
-  _LIBCPP_HIDE_FROM_ABI size_type count(const key_type& __k) const { return __tree_.__count_unique(__k); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type count(const key_type& __k) const {
+    return __tree_.__count_unique(__k);
+  }
 #  if _LIBCPP_STD_VER >= 14
   template <typename _K2, enable_if_t<__is_transparent_v<_Compare, _K2>, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI size_type count(const _K2& __k) const {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type count(const _K2& __k) const {
     return __tree_.__count_multi(__k);
   }
 #  endif
 
 #  if _LIBCPP_STD_VER >= 20
-  _LIBCPP_HIDE_FROM_ABI bool contains(const key_type& __k) const { return find(__k) != end(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool contains(const key_type& __k) const { return find(__k) != end(); }
   template <typename _K2, enable_if_t<__is_transparent_v<_Compare, _K2>, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI bool contains(const _K2& __k) const {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool contains(const _K2& __k) const {
     return find(__k) != end();
   }
 #  endif // _LIBCPP_STD_VER >= 20
 
-  _LIBCPP_HIDE_FROM_ABI iterator lower_bound(const key_type& __k) { return __tree_.__lower_bound_unique(__k); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI iterator lower_bound(const key_type& __k) {
+    return __tree_.__lower_bound_unique(__k);
+  }
 
-  _LIBCPP_HIDE_FROM_ABI const_iterator lower_bound(const key_type& __k) const {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator lower_bound(const key_type& __k) const {
     return __tree_.__lower_bound_unique(__k);
   }
 
@@ -855,46 +863,48 @@ public:
   // match multiple elements.
 #  if _LIBCPP_STD_VER >= 14
   template <typename _K2, enable_if_t<__is_transparent_v<_Compare, _K2>, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI iterator lower_bound(const _K2& __k) {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI iterator lower_bound(const _K2& __k) {
     return __tree_.__lower_bound_multi(__k);
   }
 
   template <typename _K2, enable_if_t<__is_transparent_v<_Compare, _K2>, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI const_iterator lower_bound(const _K2& __k) const {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator lower_bound(const _K2& __k) const {
     return __tree_.__lower_bound_multi(__k);
   }
 #  endif
 
-  _LIBCPP_HIDE_FROM_ABI iterator upper_bound(const key_type& __k) { return __tree_.__upper_bound_unique(__k); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI iterator upper_bound(const key_type& __k) {
+    return __tree_.__upper_bound_unique(__k);
+  }
 
-  _LIBCPP_HIDE_FROM_ABI const_iterator upper_bound(const key_type& __k) const {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator upper_bound(const key_type& __k) const {
     return __tree_.__upper_bound_unique(__k);
   }
 
 #  if _LIBCPP_STD_VER >= 14
   template <typename _K2, enable_if_t<__is_transparent_v<_Compare, _K2>, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI iterator upper_bound(const _K2& __k) {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI iterator upper_bound(const _K2& __k) {
     return __tree_.__upper_bound_multi(__k);
   }
   template <typename _K2, enable_if_t<__is_transparent_v<_Compare, _K2>, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI const_iterator upper_bound(const _K2& __k) const {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator upper_bound(const _K2& __k) const {
     return __tree_.__upper_bound_multi(__k);
   }
 #  endif
 
-  _LIBCPP_HIDE_FROM_ABI pair<iterator, iterator> equal_range(const key_type& __k) {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI pair<iterator, iterator> equal_range(const key_type& __k) {
     return __tree_.__equal_range_unique(__k);
   }
-  _LIBCPP_HIDE_FROM_ABI pair<const_iterator, const_iterator> equal_range(const key_type& __k) const {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI pair<const_iterator, const_iterator> equal_range(const key_type& __k) const {
     return __tree_.__equal_range_unique(__k);
   }
 #  if _LIBCPP_STD_VER >= 14
   template <typename _K2, enable_if_t<__is_transparent_v<_Compare, _K2>, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI pair<iterator, iterator> equal_range(const _K2& __k) {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI pair<iterator, iterator> equal_range(const _K2& __k) {
     return __tree_.__equal_range_multi(__k);
   }
   template <typename _K2, enable_if_t<__is_transparent_v<_Compare, _K2>, int> = 0>
-  _LIBCPP_HIDE_FROM_ABI pair<const_iterator, const_iterator> equal_range(const _K2& __k) const {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI pair<const_iterator, const_iterator> equal_range(const _K2& __k) const {
     return __tree_.__equal_range_multi(__k);
   }
 #  endif
diff --git a/libcxx/test/libcxx/diagnostics/multiset.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/multiset.nodiscard.verify.cpp
new file mode 100644
index 0000000000000..828d2e2897e4b
--- /dev/null
+++ b/libcxx/test/libcxx/diagnostics/multiset.nodiscard.verify.cpp
@@ -0,0 +1,18 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+
+// Check that functions are marked [[nodiscard]]
+
+#include <set>
+
+void test() {
+  std::multiset<int> multiset;
+  multiset.empty(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+}
diff --git a/libcxx/test/libcxx/diagnostics/set.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/set.nodiscard.verify.cpp
index cc6fb55854dac..046b8b06bb587 100644
--- a/libcxx/test/libcxx/diagnostics/set.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/set.nodiscard.verify.cpp
@@ -8,16 +8,104 @@
 
 // UNSUPPORTED: c++03
 
-// check that <set> functions are marked [[nodiscard]]
+// Check that functions are marked [[nodiscard]]
 
 #include <set>
 
-void set_test() {
-  std::set<int> set;
-  set.empty(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
-}
+#include "test_macros.h"
+
+template <typename T>
+struct TransparentKey {
+  T t;
+
+  constexpr explicit operator T() const { return t; }
+};
+
+struct TransparentCompare {
+  using is_transparent = void; // This makes the comparator transparent
+
+  template <typename T>
+  constexpr bool operator()(const T& t, const TransparentKey<T>& transparent) const {
+    return t < transparent.t;
+  }
+
+  template <typename T>
+  constexpr bool operator()(const TransparentKey<T>& transparent, const T& t) const {
+    return transparent.t < t;
+  }
+
+  template <typename T>
+  constexpr bool operator()(const T& t1, const T& t2) const {
+    return t1 < t2;
+  }
+};
+
+void test() {
+  std::set<int, TransparentCompare> s;
+  const std::set<int, TransparentCompare> cs{};
+
+  s.begin();  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.begin(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  s.end();    // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.end();   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  s.rbegin();  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.rbegin(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  s.rend();    // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.rend();   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  cs.cbegin();  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.cend();    // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.crbegin(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.crend();   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  s.empty();    // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  s.size();     // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  s.max_size(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  int key = 0;
+
+#if TEST_STD_VER >= 17
+  s.extract(key);       // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  s.extract(s.begin()); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
+
+  s.get_allocator(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  s.key_comp();      // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  s.value_comp();    // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  s.find(key);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.find(key); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#if TEST_STD_VER >= 14
+  TransparentKey<int> tkey;
+
+  s.find(tkey);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.find(tkey); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
+
+  s.count(key); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#if TEST_STD_VER >= 14
+  s.count(tkey); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
+
+  s.lower_bound(key);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.lower_bound(key); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#if TEST_STD_VER >= 14
+  s.lower_bound(tkey);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.lower_bound(tkey); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
+
+  s.upper_bound(key);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.upper_bound(key); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#if TEST_STD_VER >= 14
+  s.upper_bound(tkey);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.upper_bound(tkey); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
 
-void multiset_test() {
-  std::multiset<int> multiset;
-  multiset.empty(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  s.equal_range(key);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.equal_range(key); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#if TEST_STD_VER >= 14
+  s.equal_range(tkey);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cs.equal_range(tkey); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants