[libcxx-commits] [PATCH] D103171: [libc++] Deprecate std::iterator and remove it as a base class

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision.
Quuxplusone added a comment.
This revision is now accepted and ready to land.

LGTM, ship it!
Will still need a followup to fix the `difference_type` of the output iterators for C++20. Are you doing that, or am I repurposing D103101 <https://reviews.llvm.org/D103101>, or what?



================
Comment at: libcxx/include/__config:97-99
+// Give reverse_iterator<T> one data member of type T, not two.
+// Also, in C++17 and later, don't derive iterator types from std::iterator.
+#  define _LIBCPP_ABI_NO_ITERATOR_BASES
----------------
I wish this list were alphabetized, but it's already not-alphabetized, so meh.


================
Comment at: libcxx/include/__memory/raw_storage_iterator.h:45
+    typedef void                pointer;
+    typedef void                reference;
+
----------------
ldionne wrote:
> Mordante wrote:
> > Is it intended to change the types of these typedefs and the base class?
> Yes, I'm making it standards conforming. I'll add a test too, it seems like we didn't have any.
This is also, technically, an ABI break ;) but you're right, it fixes a bug that's apparently been undetected forever.


================
Comment at: libcxx/test/std/iterators/stream.iterators/istream.iterator/types.pass.cpp:35
 
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
+
----------------
I think you don't need this because `std::iterator` wasn't deprecated <=14 and now you're mentioning it only under `#if TEST_STD_VER <= 14`.
Presumably ditto in some other files as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103171