[libcxx-commits] [PATCH] D127760: [libc++][NFC] Merges unused functions in callers.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked an inline comment as done.
Mordante added a comment.

Thanks for the reviews!



================
Comment at: libcxx/test/libcxx/numerics/bit.ops.pass.cpp:34
-    static_assert( std::__libcpp_popcount(v) == 7, "");
-    static_assert( std::__bit_log2(v) == 12, "");
-    static_assert(!std::__has_single_bit(v), "");
----------------
jloser wrote:
> This is a drop in test coverage - I think we should keep this one for `__bit_log2`. In general, I think we should:
> - Keep existing test coverage for the private functions kept (i.e. ones that had two or more call sites used by public functions).
> - Verify existing coverage for public functions where we removed the private function it delegated to if we removed the private function (since it only had one call site). 
> 
> How does that sound? If you're up for it, we could go one step further and verify the `noexcept` properties.
I disagree it's a drop in test coverage. The function is indirectly tested by its callers.
But I don't mind to restore the test, so I'll add it back.

This is the only function "dropped" the other two remaining functions are still tested.
There is test coverage for the public functions, I didn't verify all, but I trust our contributors and reviewers. These tests are more detailed than the sanity checks in this file. For example `test/std/numerics/bit/bitops.count/countl_one.pass.cpp`.

I think it's useful to validate the `noexcept` status when it's not mandated by the Standard.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127760