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

Mon Apr 26 11:34:12 PDT 2021

Mordante added a comment.

In D101242#2717221 <https://reviews.llvm.org/D101242#2717221>, @cjdb wrote:

> In D101242#2717210 <https://reviews.llvm.org/D101242#2717210>, @ldionne wrote:
>
>> 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'm not too fond of the name Legacy. Unless I'm mistaken the standard uses Cpp17 for these iterators. So that would be my preference.

>> 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.

I have a strong opinion about the renaming of the header. Just wanted to make sure there wouldn't be a second update in the near future.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101242