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.

@github-actions
Copy link

github-actions bot commented Dec 3, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@H-G-Hristov H-G-Hristov force-pushed the hgh/libcxx/nodiscard-to-unordered_set branch from 780f708 to e42367d Compare December 3, 2025 07:07
[[nodiscard]] should be applied to functions where discarding the return value is most likely a correctness issue.

- https://libcxx.llvm.org/CodingGuidelines.html
- https://wg21.link/unord.set
@H-G-Hristov H-G-Hristov force-pushed the hgh/libcxx/nodiscard-to-unordered_set branch from e42367d to 7eaafda Compare December 3, 2025 07:12
@H-G-Hristov H-G-Hristov marked this pull request as ready for review December 3, 2025 07:29
@H-G-Hristov H-G-Hristov requested a review from a team as a code owner December 3, 2025 07:29
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 3, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 3, 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/170435.diff

3 Files Affected:

  • (modified) libcxx/include/unordered_set (+47-37)
  • (added) libcxx/test/libcxx/diagnostics/unordered_multiset.nodiscard.verify.cpp (+18)
  • (modified) libcxx/test/libcxx/diagnostics/unordered_set.nodiscard.verify.cpp (+89-10)
diff --git a/libcxx/include/unordered_set b/libcxx/include/unordered_set
index 9873f1ec70664..6243983f3b689 100644
--- a/libcxx/include/unordered_set
+++ b/libcxx/include/unordered_set
@@ -736,20 +736,20 @@ public:
   _LIBCPP_HIDE_FROM_ABI unordered_set& operator=(initializer_list<value_type> __il);
 #  endif // _LIBCPP_CXX03_LANG
 
-  _LIBCPP_HIDE_FROM_ABI allocator_type get_allocator() const _NOEXCEPT {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI allocator_type get_allocator() const _NOEXCEPT {
     return allocator_type(__table_.__node_alloc());
   }
 
   [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI bool empty() const _NOEXCEPT { return __table_.size() == 0; }
-  _LIBCPP_HIDE_FROM_ABI size_type size() const _NOEXCEPT { return __table_.size(); }
-  _LIBCPP_HIDE_FROM_ABI size_type max_size() const _NOEXCEPT { return __table_.max_size(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type size() const _NOEXCEPT { return __table_.size(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type max_size() const _NOEXCEPT { return __table_.max_size(); }
 
-  _LIBCPP_HIDE_FROM_ABI iterator begin() _NOEXCEPT { return __table_.begin(); }
-  _LIBCPP_HIDE_FROM_ABI iterator end() _NOEXCEPT { return __table_.end(); }
-  _LIBCPP_HIDE_FROM_ABI const_iterator begin() const _NOEXCEPT { return __table_.begin(); }
-  _LIBCPP_HIDE_FROM_ABI const_iterator end() const _NOEXCEPT { return __table_.end(); }
-  _LIBCPP_HIDE_FROM_ABI const_iterator cbegin() const _NOEXCEPT { return __table_.begin(); }
-  _LIBCPP_HIDE_FROM_ABI const_iterator cend() const _NOEXCEPT { return __table_.end(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI iterator begin() _NOEXCEPT { return __table_.begin(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI iterator end() _NOEXCEPT { return __table_.end(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator begin() const _NOEXCEPT { return __table_.begin(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator end() const _NOEXCEPT { return __table_.end(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator cbegin() const _NOEXCEPT { return __table_.begin(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator cend() const _NOEXCEPT { return __table_.end(); }
 
 #  ifndef _LIBCPP_CXX03_LANG
   template <class... _Args>
@@ -801,10 +801,10 @@ public:
                                         "node_type with incompatible allocator passed to unordered_set::insert()");
     return __table_.template __node_handle_insert_unique<node_type>(__h, 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 __table_.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 __table_.template __node_handle_extract<node_type>(__it);
   }
 
@@ -838,71 +838,81 @@ public:
     __table_.swap(__u.__table_);
   }
 
-  _LIBCPP_HIDE_FROM_ABI hasher hash_function() const { return __table_.hash_function(); }
-  _LIBCPP_HIDE_FROM_ABI key_equal key_eq() const { return __table_.key_eq(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI hasher hash_function() const { return __table_.hash_function(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI key_equal key_eq() const { return __table_.key_eq(); }
 
-  _LIBCPP_HIDE_FROM_ABI iterator find(const key_type& __k) { return __table_.find(__k); }
-  _LIBCPP_HIDE_FROM_ABI const_iterator find(const key_type& __k) const { return __table_.find(__k); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI iterator find(const key_type& __k) { return __table_.find(__k); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_iterator find(const key_type& __k) const { return __table_.find(__k); }
 #  if _LIBCPP_STD_VER >= 20
   template <class _K2, enable_if_t<__is_transparent_v<hasher, _K2> && __is_transparent_v<key_equal, _K2>>* = nullptr>
-  _LIBCPP_HIDE_FROM_ABI iterator find(const _K2& __k) {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI iterator find(const _K2& __k) {
     return __table_.find(__k);
   }
   template <class _K2, enable_if_t<__is_transparent_v<hasher, _K2> && __is_transparent_v<key_equal, _K2>>* = nullptr>
-  _LIBCPP_HIDE_FROM_ABI const_iterator find(const _K2& __k) const {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI const_iterator find(const _K2& __k) const {
     return __table_.find(__k);
   }
 #  endif // _LIBCPP_STD_VER >= 20
 
-  _LIBCPP_HIDE_FROM_ABI size_type count(const key_type& __k) const { return __table_.__count_unique(__k); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type count(const key_type& __k) const {
+    return __table_.__count_unique(__k);
+  }
 #  if _LIBCPP_STD_VER >= 20
   template <class _K2, enable_if_t<__is_transparent_v<hasher, _K2> && __is_transparent_v<key_equal, _K2>>* = nullptr>
-  _LIBCPP_HIDE_FROM_ABI size_type count(const _K2& __k) const {
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI size_type count(const _K2& __k) const {
     return __table_.__count_unique(__k);
   }
 #  endif // _LIBCPP_STD_VER >= 20
 
 #  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 <class _K2, enable_if_t<__is_transparent_v<hasher, _K2> && __is_transparent_v<key_equal, _K2>>* = nullptr>
-  _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 pair<iterator, iterator> equal_range(const key_type& __k) {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI pair<iterator, iterator> equal_range(const key_type& __k) {
     return __table_.__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 __table_.__equal_range_unique(__k);
   }
 #  if _LIBCPP_STD_VER >= 20
   template <class _K2, enable_if_t<__is_transparent_v<hasher, _K2> && __is_transparent_v<key_equal, _K2>>* = nullptr>
-  _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 __table_.__equal_range_unique(__k);
   }
   template <class _K2, enable_if_t<__is_transparent_v<hasher, _K2> && __is_transparent_v<key_equal, _K2>>* = nullptr>
-  _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 __table_.__equal_range_unique(__k);
   }
 #  endif // _LIBCPP_STD_VER >= 20
 
-  _LIBCPP_HIDE_FROM_ABI size_type bucket_count() const _NOEXCEPT { return __table_.bucket_count(); }
-  _LIBCPP_HIDE_FROM_ABI size_type max_bucket_count() const _NOEXCEPT { return __table_.max_bucket_count(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type bucket_count() const _NOEXCEPT { return __table_.bucket_count(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type max_bucket_count() const _NOEXCEPT {
+    return __table_.max_bucket_count();
+  }
 
-  _LIBCPP_HIDE_FROM_ABI size_type bucket_size(size_type __n) const { return __table_.bucket_size(__n); }
-  _LIBCPP_HIDE_FROM_ABI size_type bucket(const key_type& __k) const { return __table_.bucket(__k); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type bucket_size(size_type __n) const {
+    return __table_.bucket_size(__n);
+  }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI size_type bucket(const key_type& __k) const { return __table_.bucket(__k); }
 
-  _LIBCPP_HIDE_FROM_ABI local_iterator begin(size_type __n) { return __table_.begin(__n); }
-  _LIBCPP_HIDE_FROM_ABI local_iterator end(size_type __n) { return __table_.end(__n); }
-  _LIBCPP_HIDE_FROM_ABI const_local_iterator begin(size_type __n) const { return __table_.cbegin(__n); }
-  _LIBCPP_HIDE_FROM_ABI const_local_iterator end(size_type __n) const { return __table_.cend(__n); }
-  _LIBCPP_HIDE_FROM_ABI const_local_iterator cbegin(size_type __n) const { return __table_.cbegin(__n); }
-  _LIBCPP_HIDE_FROM_ABI const_local_iterator cend(size_type __n) const { return __table_.cend(__n); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI local_iterator begin(size_type __n) { return __table_.begin(__n); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI local_iterator end(size_type __n) { return __table_.end(__n); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_local_iterator begin(size_type __n) const {
+    return __table_.cbegin(__n);
+  }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_local_iterator end(size_type __n) const { return __table_.cend(__n); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_local_iterator cbegin(size_type __n) const {
+    return __table_.cbegin(__n);
+  }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI const_local_iterator cend(size_type __n) const { return __table_.cend(__n); }
 
-  _LIBCPP_HIDE_FROM_ABI float load_factor() const _NOEXCEPT { return __table_.load_factor(); }
-  _LIBCPP_HIDE_FROM_ABI float max_load_factor() const _NOEXCEPT { return __table_.max_load_factor(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI float load_factor() const _NOEXCEPT { return __table_.load_factor(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI float max_load_factor() const _NOEXCEPT { return __table_.max_load_factor(); }
   _LIBCPP_HIDE_FROM_ABI void max_load_factor(float __mlf) { __table_.max_load_factor(__mlf); }
   _LIBCPP_HIDE_FROM_ABI void rehash(size_type __n) { __table_.__rehash_unique(__n); }
   _LIBCPP_HIDE_FROM_ABI void reserve(size_type __n) { __table_.__reserve_unique(__n); }
diff --git a/libcxx/test/libcxx/diagnostics/unordered_multiset.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/unordered_multiset.nodiscard.verify.cpp
new file mode 100644
index 0000000000000..16eec2a7bc423
--- /dev/null
+++ b/libcxx/test/libcxx/diagnostics/unordered_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 <unordered_set>
+
+void test() {
+  std::unordered_multiset<int> us;
+  us.empty(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+}
diff --git a/libcxx/test/libcxx/diagnostics/unordered_set.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/unordered_set.nodiscard.verify.cpp
index 5d35cf00c5f5f..50c2212819bb0 100644
--- a/libcxx/test/libcxx/diagnostics/unordered_set.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/unordered_set.nodiscard.verify.cpp
@@ -8,18 +8,97 @@
 
 // UNSUPPORTED: c++03
 
-// check that <unordered_set> functions are marked [[nodiscard]]
-
-// clang-format off
+// Check that functions are marked [[nodiscard]]
 
 #include <unordered_set>
+#include <utility>
 
-void unordered_set_test() {
-  std::unordered_set<int> unordered_set;
-  unordered_set.empty(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
-}
+#include "test_macros.h"
+
+struct TransparentKey {};
+
+struct StoredKey {
+  friend bool operator==(StoredKey const&, StoredKey const&) { return true; }
+  friend bool operator==(StoredKey const&, TransparentKey const&) { return true; }
+};
+
+struct TransparentKeyHash {
+  using is_transparent = void;
+
+  constexpr std::size_t operator()(TransparentKey const&) const { return 0; }
+  constexpr std::size_t operator()(StoredKey const&) const { return 0; }
+};
+
+void test() {
+  std::unordered_set<int> us;
+  const std::unordered_set<int> cus;
+
+  us.get_allocator(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  us.empty();    // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  us.size();     // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  us.max_size(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  us.begin();   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  us.end();     // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cus.begin();  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cus.end();    // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cus.cbegin(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cus.cend();   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  int key = 0;
+
+#if TEST_STD_VER >= 17
+  us.extract(key);        // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  us.extract(us.begin()); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
+
+  us.hash_function(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  us.key_eq();        // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  us.find(key);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cus.find(key); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#if TEST_STD_VER >= 20
+  std::unordered_set<StoredKey, TransparentKeyHash, std::equal_to<>> tus;
+  const std::unordered_set<StoredKey, TransparentKeyHash, std::equal_to<>> ctus;
+  TransparentKey tkey;
+
+  tus.find(tkey);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  ctus.find(tkey); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
+
+  us.count(key); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#if TEST_STD_VER >= 20
+  tus.count(tkey); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
+
+#if TEST_STD_VER >= 20
+  us.contains(key);   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  tus.contains(tkey); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
+
+  us.equal_range(key);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cus.equal_range(key); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#if TEST_STD_VER >= 20
+  tus.equal_range(tkey);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  ctus.equal_range(tkey); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
+
+  us.bucket_count();     // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  us.max_bucket_count(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  int size = 0;
+
+  us.bucket_size(size); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  us.bucket(key);       // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  us.begin(size);   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  us.end(size);     // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cus.begin(size);  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cus.end(size);    // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cus.cbegin(size); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cus.cend(size);   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
 
-void unordered_multiset_test() {
-  std::unordered_multiset<int> unordered_multiset;
-  unordered_multiset.empty(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  us.load_factor();     // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  us.max_load_factor(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
 }

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