[libcxx-commits] [PATCH] D136908: [libc++] Remove duplication in math_h.pass.cpp and improve coverage

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D136908#3894913 <https://reviews.llvm.org/D136908#3894913>, @EricWF wrote:

> Missing description.
>
> All changes need a description.

What description do you want? "It improves coverage." or what?

> In particular, why is the behavior we're adding for `int128` correct, and why do we want it?

I'm not sure what you're asking about with "why the behaviour is correct". We claim support for `__int128`, so I consider it to be a bug.

> Further, I don't think deduplicated these tests is useful. It makes the tests harder to read by virtue of introducing indirection.

It's not just deduplicating. We are currently lacking coverage for a lot of types, which this patch adds. It is harder to check which combinations of types are checked, but with the indirection you only have to check it once, not for every function. I'm quite certain the duplication was a contributing factor why we didn't have any coverage for most types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136908