[libcxx-commits] [PATCH] D108575: [libcxx] Define insert_iterator::iter with ranges::iterator_t

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Aug 23 14:41:53 PDT 2021


Quuxplusone added a comment.

Also LGTM % lack of test. The new test should use a simple custom container that has a `ranges::iterator_t` but no nested `::iterator` type. The only member function your custom container needs to provide is `.insert`.



================
Comment at: libcxx/include/__iterator/insert_iterator.h:20-22
+#if _LIBCPP_STD_VER > 17
+#include <__ranges/access.h>
+#endif
----------------
Our preferred style here is to `#include <__ranges/access.h>` unconditionally, alphabetized among the above headers. If `!(_LIBCPP_STD_VER > 17)`, this `#include` will just be a no-op, and that's fine.

(Compare `#include <__iterator/iterator.h>` on line 14: that type is used on line 37 only if `_LIBCPP_STD_VER <= 14 || !defined(_LIBCPP_ABI_NO_ITERATOR_BASES)`, but we don't guard line 14, only line 37.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108575



More information about the libcxx-commits mailing list