[libcxx-commits] [PATCH] D152653: [libc++] Add [[nodiscard]] extensions to the functions in <bit>

[Mark de Wever via Phabricator via libcxx-commits]

Mordante requested changes to this revision.
Mordante added a comment.
This revision now requires changes to proceed.

Thanks for working on this!



================
Comment at: libcxx/include/__bit/bit_floor.h:26
 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);
----------------
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.


================
Comment at: libcxx/include/__bit/countr.h:26-34
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
 int __libcpp_ctz(unsigned __x)           _NOEXCEPT { return __builtin_ctz(__x); }
 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
 int __libcpp_ctz(unsigned long __x)      _NOEXCEPT { return __builtin_ctzl(__x); }
 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
----------------
These would make sense to, doesn't even need to be an extension.


================
Comment at: libcxx/include/__bit/rotate.h:37
 template <__libcpp_unsigned_integer _Tp>
-_LIBCPP_HIDE_FROM_ABI constexpr _Tp rotl(_Tp __t, unsigned int __cnt) noexcept {
+_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr _Tp rotl(_Tp __t, unsigned int __cnt) noexcept {
   const unsigned int __dig = numeric_limits<_Tp>::digits;
----------------
These should be `nodiscard` unconditionally. http://eel.is/c++draft/bit#rotate ...

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






================
Comment at: libcxx/test/libcxx/diagnostics/bit.nodiscard_extensions.compile.pass.cpp:19
+void func() {
+  std::bit_ceil(0u);
+  std::bit_floor(0u);
----------------
Please add `bit_cast` too; it's already marked, but not tested.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152653/new/

https://reviews.llvm.org/D152653