[libcxx-commits] [PATCH] D101242: [libcxx][nfc] renames test iterator types to `legacy_*_iterator`

Mon Apr 26 10:10:54 PDT 2021

cjdb added a subscriber: tcanens.
cjdb added a comment.

In D101242#2717210 <https://reviews.llvm.org/D101242#2717210>, @ldionne wrote:

> I agree this is making names longer and I dislike that. However, I think being able to distinguish between an archetype for a C++20 `input_iterator` and a pre-concepts `InputIterator` archetype is useful and necessary in the test suite, since those two concepts are different (see note about equality comparable https://en.cppreference.com/w/cpp/iterator/input_iterator).
>
> One other option would be to call those `cpp17_XXXXX_iterator` instead. That way, if we introduce new iterator concepts eventually in the future, we can call the current ones `cpp20_XXXXX_iterator` instead of being stuck with a naming problem. WDYT?

I have grave objections to `cpp17` as a prefix (I even raised that in LWG once). It's weird see a C++11 test to have a `cpp17_XXXX_iterator`. I'm pretty sure that's why @tcanens pushed so strongly for //LegacyXXXXIterator// on cppreference too.

> I also agree with @Mordante , if we're going to rename the header in the future, we might just as well rename it now. If there's no plans to rename the header, then this LGTM.

No plans to rename the header, but I could be talked into it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101242