[libcxx-commits] [PATCH] D137476: [libc++] Add utilites for instantiating functions with multiple types

Hui via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Nov 7 11:54:22 PST 2022


huixie90 added a comment.

In D137476#3910815 <https://reviews.llvm.org/D137476#3910815>, @EricWF wrote:

> I don't think the test suite is the place to get clever with code deduplication and template magic.
>
> Readability is significantly more important than repeating yourself. Tests need to be correct upon inspection, because we don't have tests for our tests.
> Also I imagine this significantly borks any diagnostics you might get when the test fails.
>
> Tests are not the place to be clever.

I agree with this and readability is more important. But in certain scenarios, e.g the math tests refactoring Nicolas did which increases the coverage for math tests, it was really hard for me to figure out what types are missing in the original tests as the original tests have hundreds of lines that enumerates all the types it is testing, for every argument, which in my opinion, not very readable. I also think this utility is needed for 3 iterator arguments range algorithms tests , as enumerate the combinations of types make the tests not very readable and hard to spot any combination missing. I think it is a useful tool but requires careful testing of itself


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137476



More information about the libcxx-commits mailing list