[libcxx-commits] [PATCH] D99854: [libcxx] adds `cpp17-.*iterator` concepts (as `__legacy_.*iterator`)

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/test/libcxx/iterators/iterator.requirements/iterator.assoc.types/iterator.traits/legacy_bidirectional_iterator.compile.pass.cpp:159
+static_assert(!std::__legacy_bidirectional_iterator<
+              std::unordered_set<int, int>::iterator>);
+static_assert(!std::__legacy_bidirectional_iterator<
----------------
This should only have one template. Otherwise it's going to think `int` is the hash type ;)


================
Comment at: libcxx/test/libcxx/iterators/iterator.requirements/iterator.assoc.types/iterator.traits/legacy_forward_iterator.compile.pass.cpp:70
+static_assert(!std::__legacy_forward_iterator<
+              std::back_insert_iterator<std::vector<int> > >);
+static_assert(!std::__legacy_forward_iterator<
----------------
Why isn't this a __legacy_forward_iterator? 


================
Comment at: libcxx/test/libcxx/iterators/iterator.requirements/iterator.assoc.types/iterator.traits/legacy_forward_iterator.compile.pass.cpp:151
+static_assert(std::__legacy_forward_iterator<
+              std::unordered_set<int, int>::const_iterator>);
+
----------------
Same comment. And for `unordered_multiset`. Also why not test (const_) local_iterator for both while you're at it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99854