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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante accepted this revision.
Mordante added a comment.
This revision is now accepted and ready to land.

LGTM modulo one nit.



================
Comment at: libcxx/docs/UsingLibcxx.rst:401
 * ``find_if``
 * ``find``
 * ``get_temporary_buffer``
----------------
FYI I landed D152097 today, this fixed the ordering of this list. So there might be some merge conflicts.


================
Comment at: libcxx/docs/UsingLibcxx.rst:424-425
 * ``remove``
+* ``rotl``
+* ``rotr``
 * ``search_n``
----------------
These are not extensions


================
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);
----------------
philnik wrote:
> Mordante wrote:
> > 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.
> 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`.
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152653