Incidentally, this removes some unqualified ADL calls to begin and end. (See also D119677.)
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
There's a bunch of functions that were previously tested here that are no longer. What's up with that?
The test suite has typically been structured so that each overload gets its own test file. Your changes undo this. Can you state why you believe this to be an improvement?
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.
The test suite has typically been structured so that each overload gets its own test file. Your changes undo this. Can you state why you believe this to be an improvement?
Consistency with 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."
We exercise judgement regarding this. We do try to split different functions and overloads into separate files, except when it provides little value.
FWIW, I'd rather keep begin and end tests in different files (ditto in D119677), but that's non-blocking. I also don't see a reduction in test coverage, so LGTM.
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. Where the tests for a lot of equality operators were removed.
Consistency with 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.