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.

@H-G-Hristov H-G-Hristov requested a review from a team as a code owner December 4, 2025 08:04
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 4, 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/170623.diff

2 Files Affected:

  • (modified) libcxx/include/bitset (+24-15)
  • (added) libcxx/test/libcxx/utilities/template.bitset/nodiscard.verify.cpp (+48)
diff --git a/libcxx/include/bitset b/libcxx/include/bitset
index 3453c2fcde71e..82df661ed30ff 100644
--- a/libcxx/include/bitset
+++ b/libcxx/include/bitset
@@ -681,45 +681,54 @@ public:
 
   // element access:
 #  ifdef _LIBCPP_ABI_BITSET_VECTOR_BOOL_CONST_SUBSCRIPT_RETURN_BOOL
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bool operator[](size_t __p) const {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bool operator[](size_t __p) const {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__p < _Size, "bitset::operator[] index out of bounds");
     return __base::__make_ref(__p);
   }
 #  else
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __const_reference operator[](size_t __p) const {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __const_reference operator[](size_t __p) const {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__p < _Size, "bitset::operator[] index out of bounds");
     return __base::__make_ref(__p);
   }
 #  endif
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 reference operator[](size_t __p) {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 reference operator[](size_t __p) {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(__p < _Size, "bitset::operator[] index out of bounds");
     return __base::__make_ref(__p);
   }
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unsigned long to_ulong() const { return __base::to_ulong(); }
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unsigned long long to_ullong() const {
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unsigned long to_ulong() const {
+    return __base::to_ulong();
+  }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unsigned long long to_ullong() const {
     return __base::to_ullong();
   }
   template <class _CharT, class _Traits, class _Allocator>
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 basic_string<_CharT, _Traits, _Allocator>
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 basic_string<_CharT, _Traits, _Allocator>
   to_string(_CharT __zero = _CharT('0'), _CharT __one = _CharT('1')) const;
   template <class _CharT, class _Traits>
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 basic_string<_CharT, _Traits, allocator<_CharT> >
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 basic_string<_CharT, _Traits, allocator<_CharT> >
   to_string(_CharT __zero = _CharT('0'), _CharT __one = _CharT('1')) const;
   template <class _CharT>
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 basic_string<_CharT, char_traits<_CharT>, allocator<_CharT> >
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 basic_string<_CharT, char_traits<_CharT>, allocator<_CharT> >
   to_string(_CharT __zero = _CharT('0'), _CharT __one = _CharT('1')) const;
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 basic_string<char, char_traits<char>, allocator<char> >
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI
+  _LIBCPP_CONSTEXPR_SINCE_CXX23 basic_string<char, char_traits<char>, allocator<char> >
   to_string(char __zero = '0', char __one = '1') const;
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 size_t count() const _NOEXCEPT;
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR size_t size() const _NOEXCEPT { return _Size; }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 size_t count() const _NOEXCEPT;
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR size_t size() const _NOEXCEPT { return _Size; }
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool operator==(const bitset& __rhs) const _NOEXCEPT;
 #  if _LIBCPP_STD_VER <= 17
   _LIBCPP_HIDE_FROM_ABI bool operator!=(const bitset& __rhs) const _NOEXCEPT;
 #  endif
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool test(size_t __pos) const;
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool all() const _NOEXCEPT { return __base::all(); }
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool any() const _NOEXCEPT { return __base::any(); }
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool none() const _NOEXCEPT { return !any(); }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool test(size_t __pos) const;
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool all() const _NOEXCEPT {
+    return __base::all();
+  }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool any() const _NOEXCEPT {
+    return __base::any();
+  }
+  [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool none() const _NOEXCEPT { return !any(); }
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bitset operator<<(size_t __pos) const _NOEXCEPT;
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bitset operator>>(size_t __pos) const _NOEXCEPT;
 
diff --git a/libcxx/test/libcxx/utilities/template.bitset/nodiscard.verify.cpp b/libcxx/test/libcxx/utilities/template.bitset/nodiscard.verify.cpp
new file mode 100644
index 0000000000000..8fc9b5d402d3b
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/template.bitset/nodiscard.verify.cpp
@@ -0,0 +1,48 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// <bitset>
+
+// Check that functions are marked [[nodiscard]]
+
+#include <bitset>
+
+#include "test_macros.h"
+#include "test_allocator.h"
+
+void test() {
+  std::bitset<10> bs;
+  const std::bitset<10> cbs;
+
+  bs[0];          // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  cbs[0];         // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  bs.to_ulong();  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  bs.to_ullong(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  struct CharTraits : public std::char_traits<char> {};
+
+  // expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+  bs.to_string<char, CharTraits, test_allocator<char>>();
+  // expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+  bs.to_string<char, CharTraits>();
+#if !defined(TEST_HAS_NO_WIDE_CHARACTERS)
+  // expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+  bs.to_string<wchar_t>();
+#endif
+  // expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+  bs.to_string();
+
+  bs.count(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  bs.size();  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  bs.test(0); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  bs.all();   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  bs.any();   // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  bs.none();  // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+}

Copy link
Contributor

@Thibault-Monnier Thibault-Monnier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You left out operator<< and operator>>. Is that intentional?

@H-G-Hristov
Copy link
Contributor Author

You left out operator<< and operator>>. Is that intentional?

Yes. Can you think of an example where it would matter?

@Thibault-Monnier
Copy link
Contributor

If we make operator[] nodiscard, why wouldn't we make these nodiscard too?

@Zingam
Copy link
Contributor

Zingam commented Dec 5, 2025

If we make operator[] nodiscard, why wouldn't we make these nodiscard too?

I thoght you meant the stream's <<, the bitwise operators should be annoteted. Thanks.

@H-G-Hristov H-G-Hristov force-pushed the hgh/libcxx/nodiscard-to-bitset branch from 22b307f to 100d36f Compare December 5, 2025 04:50
`[[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/bitset
@H-G-Hristov H-G-Hristov force-pushed the hgh/libcxx/nodiscard-to-bitset branch from 100d36f to c47f4d5 Compare December 5, 2025 05:12
@github-actions
Copy link

github-actions bot commented Dec 6, 2025

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

@Zingam
Copy link
Contributor

Zingam commented Dec 6, 2025

Thanks!

@Zingam Zingam merged commit 9031544 into llvm:main Dec 6, 2025
80 checks passed
@H-G-Hristov H-G-Hristov deleted the hgh/libcxx/nodiscard-to-bitset branch December 6, 2025 17:23
honeygoyal pushed a commit to honeygoyal/llvm-project that referenced this pull request Dec 9, 2025
`[[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/bitset
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.

6 participants