[libcxx-commits] [PATCH] D100073: [libcxx] adds `std::indirectly_readable` to <iterator>

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added a comment.

Overall, I think this patch is great. There are a lot of tests, but they're all super simple and easily verifiable. Other than the few very small comments I made inline, this LGTM.



================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.readable/indirectly_readable.compile.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
I really like all the tests in here, thanks!


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.readable/indirectly_readable.compile.pass.cpp:145
+
+struct different_reference_types_but_share_common_reference {
+  using value_type = iter_ref3;
----------------
A bit shorter: `different_reference_types_with_common_reference`?


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.readable/indirectly_readable.compile.pass.cpp:308
+static_assert(
+    check_indirectly_readable<std::unordered_multiset<int, int>::iterator>());
+static_assert(check_indirectly_readable<
----------------
Only one template param. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100073