[libc++] Add [[nodiscard]] extensions to the functions in <bit>

[ldionne]

libcxx/include/__bit/bit_ceil.h

20

Can you update the list in libcxx/docs/UsingLibcxx.rst?

[Mordante]

Thanks for working on this!

libcxx/include/__bit/bit_floor.h

26

These should be documented in libcxx/docs/UsingLibcxx.rst.

Maybe we should look at a script/clang-tidy check to generate the documentation for these extensions.

libcxx/include/__bit/countr.h

26–34

These would make sense to, doesn't even need to be an extension.

libcxx/include/__bit/rotate.h

37

These should be nodiscard unconditionally. http://eel.is/c++draft/bit#rotate ...

I've no clue what makes these two functions special.

libcxx/test/libcxx/diagnostics/bit.nodiscard_extensions.compile.pass.cpp

20

Please add bit_cast too; it's already marked, but not tested.

[Mordante]

I see Louis and I reviewed at the same time.

[philnik]

Address comments

libcxx/include/__bit/bit_floor.h

26

IMO we should just remove the list, since I don't really see any use for it. A user doesn't care which functions are marked [[nodiscard]] unless the compiler complains (or doesn't complain if a [[nodiscard]] is missing), and implementers can just look at the code or the *.nodiscard_externsions.*.cpp. files or grep for _LIBCPP_NODISCARD_EXT.

[Mordante]

LGTM modulo one nit.

libcxx/docs/UsingLibcxx.rst

407–412

FYI I landed D152097 today, this fixed the ordering of this list. So there might be some merge conflicts.

449–450

These are not extensions

libcxx/include/__bit/bit_floor.h

26

I strongly disagree. This is an extension and it should be documented.
Actually I think it would be good to see whether it would be good to Standardize noexcept in more places in the Standard so we don't need extensions.

[philnik]

libcxx/include/__bit/bit_floor.h

26

Arguably it's not. Compilers are allowed to warn on anything they want, and that's what we do here - just in the library instead of the compiler. Just because we use a standard attribute for it doesn't mean it's an extension. It's not like there is a meaningful difference between [[nodiscard]] and [[clang:: warn_unused_result]]. Regarding standardization - IMO that's wasted time. Implementations can add it at their leisure, which makes spending committee time on it basically useless. Furthermore, implementations don't even have to add it, since it's just recommended practice to issue a diagnostic: http://eel.is/c++draft/dcl.attr.nodiscard#4, making every [[nodiscard]] in the standard nothing more than loose guidance.

libcxx/docs/UsingLibcxx.rst

 which no dialect declares as such (See the second form described above).
 
 * ``adjacent_find``
 * ``all_of``
 * ``any_of``
 * ``as_const``
 * ``binary_search``
 * ``bit_cast``
+* ``bit_ceil``
+* ``bit_floor``
+* ``bit_width``
+* ``byteswap``
 * ``cbrt``
 * ``ceil``
 * ``clamp``
 * ``copysign``
 * ``count_if``
 * ``count``
+* ``countl_zero``
+* ``countl_one``
+* ``countr_zero``
+* ``countr_one``
 * ``equal_range``
 * ``equal``
 * ``fabs``
 * ``find_end``
 * ``find_first_of``
 * ``find_if_not``
 * ``find_if``
 * ``find``
 * ``floor``
 * ``fmax``
 * ``fmin``
 * ``forward``
 * ``fpclassify``
 * ``get_temporary_buffer``
+* ``has_single_bit``
 * ``identity::operator()``
 * ``includes``
 * ``is_heap_until``
 * ``is_heap``
 * ``is_partitioned``
 * ``is_permutation``
 * ``is_sorted_until``
 * ``is_sorted``

 * ``min``
 * ``minmax_element``
 * ``minmax``
 * ``mismatch``
 * ``move_if_noexcept``
 * ``move``
 * ``nearbyint``
 * ``none_of``
+* ``popcount``
 * ``ranges::adjacent_find``
 * ``ranges::all_of``
 * ``ranges::any_of``
 * ``ranges::binary_search``
 * ``ranges::clamp``
 * ``ranges::count_if``
 * ``ranges::count``
 * ``ranges::equal_range``
 * ``ranges::equal``

libcxx/include/__bit/bit_ceil.h

 #include <__assert>
 #include <__bit/countl.h>
 #include <__concepts/arithmetic.h>
 #include <__config>
 #include <limits>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if _LIBCPP_STD_VER >= 20
 
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr _Tp bit_ceil(_Tp __t) noexcept {
+_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr _Tp bit_ceil(_Tp __t) noexcept {
   if (__t < 2)
     return 1;
   const unsigned __n = numeric_limits<_Tp>::digits - std::countl_zero((_Tp)(__t - 1u));
   _LIBCPP_ASSERT(__n != numeric_limits<_Tp>::digits, "Bad input to bit_ceil");
 
   if constexpr (sizeof(_Tp) >= sizeof(unsigned))
     return _Tp{1} << __n;
   else {

libcxx/include/__bit/bit_floor.h

 #  pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if _LIBCPP_STD_VER >= 20
 
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr _Tp bit_floor(_Tp __t) noexcept {
+_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr _Tp bit_floor(_Tp __t) noexcept {
   return __t == 0 ? 0 : _Tp{1} << std::__bit_log2(__t);
 }
 
 #endif // _LIBCPP_STD_VER >= 20
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___BIT_BIT_FLOOR_H

libcxx/include/__bit/bit_width.h

 #  pragma GCC system_header
 #endif
 
 #if _LIBCPP_STD_VER >= 20
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr int bit_width(_Tp __t) noexcept {
+_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr int bit_width(_Tp __t) noexcept {
   return __t == 0 ? 0 : std::__bit_log2(__t) + 1;
 }
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP_STD_VER >= 20
 
 #endif // _LIBCPP___BIT_BIT_WIDTH_H

libcxx/include/__bit/byteswap.h

 #  pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if _LIBCPP_STD_VER >= 23
 
 template <integral _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr _Tp byteswap(_Tp __val) noexcept {
+_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr _Tp byteswap(_Tp __val) noexcept {
 
   if constexpr (sizeof(_Tp) == 1) {
     return __val;
   } else if constexpr (sizeof(_Tp) == 2) {
     return __builtin_bswap16(__val);
   } else if constexpr (sizeof(_Tp) == 4) {
     return __builtin_bswap32(__val);
   } else if constexpr (sizeof(_Tp) == 8) {

libcxx/include/__bit/countl.h

 #  pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+_LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
 int __libcpp_clz(unsigned __x)           _NOEXCEPT { return __builtin_clz(__x); }
 
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+_LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
 int __libcpp_clz(unsigned long __x)      _NOEXCEPT { return __builtin_clzl(__x); }
 
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+_LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
 int __libcpp_clz(unsigned long long __x) _NOEXCEPT { return __builtin_clzll(__x); }
 
 #  ifndef _LIBCPP_HAS_NO_INT128
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
 int __libcpp_clz(__uint128_t __x) _NOEXCEPT {
   // The function is written in this form due to C++ constexpr limitations.
   // The algorithm:
   // - Test whether any bit in the high 64-bits is set
else
             }
         return __ret + __iter;
     }
 }
 
 #if _LIBCPP_STD_VER >= 20
 
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr int countl_zero(_Tp __t) noexcept {
+_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr int countl_zero(_Tp __t) noexcept {
   return std::__countl_zero(__t);
 }
 
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr int countl_one(_Tp __t) noexcept {
+_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr int countl_one(_Tp __t) noexcept {
   return __t != numeric_limits<_Tp>::max() ? std::countl_zero(static_cast<_Tp>(~__t)) : numeric_limits<_Tp>::digits;
 }
 
 #endif // _LIBCPP_STD_VER >= 20
 
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS
 
 #endif // _LIBCPP___BIT_COUNTL_H

libcxx/include/__bit/countr.h

 #  pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+_LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
 int __libcpp_ctz(unsigned __x)           _NOEXCEPT { return __builtin_ctz(__x); }
 
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+_LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
 int __libcpp_ctz(unsigned long __x)      _NOEXCEPT { return __builtin_ctzl(__x); }
 
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+_LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR
 int __libcpp_ctz(unsigned long long __x) _NOEXCEPT { return __builtin_ctzll(__x); }
 
 #if _LIBCPP_STD_VER >= 20
 
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr int countr_zero(_Tp __t) noexcept {
+_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr int countr_zero(_Tp __t) noexcept {
   if (__t == 0)
     return numeric_limits<_Tp>::digits;
 
   if (sizeof(_Tp) <= sizeof(unsigned int))
     return std::__libcpp_ctz(static_cast<unsigned int>(__t));
   else if (sizeof(_Tp) <= sizeof(unsigned long))
     return std::__libcpp_ctz(static_cast<unsigned long>(__t));
   else if (sizeof(_Tp) <= sizeof(unsigned long long))
     return std::__libcpp_ctz(static_cast<unsigned long long>(__t));
   else {
     int __ret = 0;
     const unsigned int __ulldigits = numeric_limits<unsigned long long>::digits;
     while (static_cast<unsigned long long>(__t) == 0uLL) {
       __ret += __ulldigits;
       __t >>= __ulldigits;
     }
     return __ret + std::__libcpp_ctz(static_cast<unsigned long long>(__t));
   }
 }
 
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr int countr_one(_Tp __t) noexcept {
+_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr int countr_one(_Tp __t) noexcept {
   return __t != numeric_limits<_Tp>::max() ? std::countr_zero(static_cast<_Tp>(~__t)) : numeric_limits<_Tp>::digits;
 }
 
 #endif // _LIBCPP_STD_VER >= 20
 
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS
 
 #endif // _LIBCPP___BIT_COUNTR_H

libcxx/include/__bit/has_single_bit.h

 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
 #if _LIBCPP_STD_VER >= 20
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr bool has_single_bit(_Tp __t) noexcept {
+_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr bool has_single_bit(_Tp __t) noexcept {
   return __t != 0 && (((__t & (__t - 1)) == 0));
 }
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP_STD_VER >= 20
 
 _LIBCPP_POP_MACROS
 
 #endif // _LIBCPP___BIT_HAS_SINGLE_BIT_H

libcxx/include/__bit/popcount.h

 int __libcpp_popcount(unsigned long __x)      _NOEXCEPT { return __builtin_popcountl(__x); }
 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
 int __libcpp_popcount(unsigned long long __x) _NOEXCEPT { return __builtin_popcountll(__x); }
 
 #if _LIBCPP_STD_VER >= 20
 
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr int popcount(_Tp __t) noexcept {
+_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr int popcount(_Tp __t) noexcept {
   if (sizeof(_Tp) <= sizeof(unsigned int))
     return std::__libcpp_popcount(static_cast<unsigned int>(__t));
   else if (sizeof(_Tp) <= sizeof(unsigned long))
     return std::__libcpp_popcount(static_cast<unsigned long>(__t));
   else if (sizeof(_Tp) <= sizeof(unsigned long long))
     return std::__libcpp_popcount(static_cast<unsigned long long>(__t));
   else {
     int __ret = 0;

libcxx/include/__bit/rotate.h

_Tp __rotr(_Tp __t, unsigned int __cnt) _NOEXCEPT
     if ((__cnt % __dig) == 0)
         return __t;
     return (__t >> (__cnt % __dig)) | (__t << (__dig - (__cnt % __dig)));
 }
 
 #if _LIBCPP_STD_VER >= 20
 
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr _Tp rotl(_Tp __t, unsigned int __cnt) noexcept {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp rotl(_Tp __t, unsigned int __cnt) noexcept {
   const unsigned int __dig = numeric_limits<_Tp>::digits;
   if ((__cnt % __dig) == 0)
     return __t;
   return (__t << (__cnt % __dig)) | (__t >> (__dig - (__cnt % __dig)));
 }
 
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr _Tp rotr(_Tp __t, unsigned int __cnt) noexcept {
+[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp rotr(_Tp __t, unsigned int __cnt) noexcept {
   return std::__rotr(__t, __cnt);
 }
 
 #endif // _LIBCPP_STD_VER >= 20
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___BIT_ROTATE_H

libcxx/test/libcxx/diagnostics/bit.nodiscard_extensions.compile.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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, c++11, c++14, c++17
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_NODISCARD_EXT
+
+// Check that <bit> functions aren't marked [[nodiscard]] when
+// _LIBCPP_DISBALE_NODISCARD_EXT is defined
+
+#include <bit>
+
+#include "test_macros.h"
+
+void func() {
+  std::bit_cast<unsigned int>(42);
+  std::bit_ceil(0u);
+  std::bit_floor(0u);
+  std::bit_width(0u);
+#if TEST_STD_VER >= 23
+  std::byteswap(0u);
+#endif
+  std::countl_zero(0u);
+  std::countl_one(0u);
+  std::countr_zero(0u);
+  std::countr_one(0u);
+  std::has_single_bit(0u);
+  std::popcount(0u);
+}

libcxx/test/libcxx/diagnostics/bit.nodiscard_extensions.verify.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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, c++11, c++14, c++17
+
+// check that <bit> functions are marked [[nodiscard]]
+
+#include <bit>
+
+#include "test_macros.h"
+
+void func() {
+  std::bit_cast<unsigned int>(42); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::bit_ceil(0u); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::bit_floor(0u); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::bit_width(0u); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#if TEST_STD_VER >= 23
+  std::byteswap(0u); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+#endif
+  std::countl_zero(0u); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::countl_one(0u); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::countr_zero(0u); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::countr_one(0u); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::has_single_bit(0u); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::popcount(0u); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+}

libcxx/test/libcxx/diagnostics/nodiscard_extensions.compile.pass.cpp

 
 void test_nontemplate_cast_wrappers()
 {
 #if TEST_STD_VER > 14
   std::byte b{42};
   std::to_integer<int>(b);
 #endif
 
-#if TEST_STD_VER > 17
-  std::bit_cast<unsigned int>(42);
-#endif
-
 #if TEST_STD_VER > 20
   enum E { Apple, Orange } e = Apple;
   std::to_underlying(e);
 #endif
 }
 
 void tests() {
   test_algorithms();
 
   int i = 42;
   test_template_cast_wrappers(i, std::move(i));
   test_nontemplate_cast_wrappers();
 }

libcxx/test/libcxx/diagnostics/nodiscard_extensions.verify.cpp

 void test_nontemplate_cast_wrappers()
 {
 #if TEST_STD_VER > 14
   std::byte b{42};
   // expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
   std::to_integer<int>(b);
 #endif
 
-#if TEST_STD_VER > 17
-  // expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
-  std::bit_cast<unsigned int>(42);
-#endif
-
 #if TEST_STD_VER > 20
   enum E { Apple, Orange } e = Apple;
   // expected-warning@+1 {{ignoring return value of function declared with 'nodiscard' attribute}}
   std::to_underlying(e);
 #endif
 }
 
 void f() {
   test_algorithms();
 
   int i = 42;
   test_template_cast_wrappers(i, std::move(i));
   test_nontemplate_cast_wrappers();
 }

libcxx/test/std/numerics/bit/bitops.rot/nodiscard.verify.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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, c++11, c++14, c++17
+
+// Check that std::rotl and std::rotr are marked [[nodiscard]]
+
+#include <bit>
+
+void func() {
+  std::rotl(0u, 0); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::rotr(0u, 0); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+}