[libcxx-commits] [PATCH] D99141: [libcxx] adds `std::incrementable_traits` to <iterator>

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added a comment.

> I'd like to put an end to comments about "too many tests" once and for all. I think we all agree that test coverage is a good property to have. I think we also agree that readability is a good property to have, and that code duplication is bad. We want both, as much as possible.
>
> [...]

I really like your suggestion. The only two things I'd say are

1. We could take that even further and have a test macro that generates all the const/ref/volatile combinations we want (this is sort of like what you were saying with the two loops). Then we could just pass it a macro that generated the tests for //any// given concept.
2. These tests are good because they cover as much code as possible, but they make it harder to verify what's actually being tested (especially if it's two macros deep). I'd suggest that we have the macro tests //and// some simple, easily verifyable tests that verify the basic/common case.

Using macros like this will also make it easy to write lots of tests quickly, so it shouldn't be too hard to back port to the other concepts that have already landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99141