[libcxx-commits] [PATCH] D99398: [libcxx] reworks invocable and regular_invocable tests

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added a comment.

In D99398#2653006 <https://reviews.llvm.org/D99398#2653006>, @Quuxplusone wrote:

> Well, it hinges on what you mean by
>
>> and does not introduce new duplication
>
> From my POV, obviously this patch introduces MASSIVE duplication. Lines 1-500 are very similar to lines 501-1000. That's literally all. I don't think we should have 1000 lines of highly repetitive tests, just to test a three-line metafunction. It makes it harder for the maintainer to see what's actually tested and what's not.

Each test checks something different, so there aren't any "highly repetitive tests".

The reason the tests are duplicated across two files is because `regular_invocable<T> = invocable<T>`, and the easiest way to ensure that is to make sure that they pass identical tests. Short of moving everything common into a header and using a macro, there isn't a way to eliminate said duplication across files.

> (Minorly, I think line 258 doesn't match the style we've been using for other constexpr tests. These constexpr functions could all be using `assert` in the bodies and `return true` at the end.)

It matches the style of all previous concepts tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99398