[libcxx-commits] [PATCH] D119687: [libc++] [test] Improve the tests for std::{begin, end}(valarray).

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added a comment.



> We exercise judgement regarding this. We do try to split different functions and overloads into separate files, except when it provides little value.

Undoubtedly, we exercise judgement. I was asking for that judgement to be articulated.
The value to structure is that things conform to it. So we can locate the relevant tests given only the stable name in the standard.

>> There's a bunch of functions that were previously tested here that are no longer. What's up with that?
>
> Such as? I see the reverse — `begin(const valarray<T>&)` was never tested on the left-hand side, but it //is// tested on the right-hand side.

Sorry, I meant to comment that on D119677 <https://reviews.llvm.org/D119677>. Where the tests for a lot of equality operators were removed.

> Consistency with D119677 <https://reviews.llvm.org/D119677>, and making sure we test every one of the four functions in the same way.
> The original raison d'etre here was just to get rid of the unqualified ADL calls to `begin` and `end`, and make sure we're spelling them `std::begin` and `std::end` in this directory. The rest of the cleanup was just "as long as I'm here."

Have we addressed whether the ADL was intentional? Because it seems very possible that it was. Do we have tests that ensure ADL fires as it's supposed to?
And I don't know that I agree this is "cleanup". I stand with ldionne that these should be separate files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119687