[libcxx-commits] [PATCH] D127760: [libc++][NFC] Merges unused functions in callers.
Joe Loser via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Jun 22 11:29:30 PDT 2022
jloser added inline comments.
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.
Valid point. It is indirectly tested with `bit_floor` and `bit_width`, though I do like the direct testing of it - thanks for adding that back.
Appreciate the added tests in general - LGTM!
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits