[libcxx-commits] [PATCH] D103101: [libc++] Add _LIBCPP_ABI_NO_ITERATOR_BASES. Fix input/output iterators' difference_type for C++20.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/iterator:792
 class _LIBCPP_TEMPLATE_VIS back_insert_iterator
-    : public iterator<output_iterator_tag,
-                      void,
-                      void,
-                      void,
-                      void>
+#if _LIBCPP_STD_VER <= 11 || !defined(_LIBCPP_ABI_NO_ITERATOR_BASES)
+    : public iterator<output_iterator_tag, void, void, void, void>
----------------
Quuxplusone wrote:
> ldionne wrote:
> > There is no ABI implication here AFAICT, since the first member will always begin at offset 0 anyway. So this means this can just be `#if _LIBCPP_STD_VER <= 14`.
> > 
> > Also, notice that your `_LIBCPP_STD_VER` is wrong, it should be compared to 14, not 11. The `std::iterator` bases were removed in C++17.
> The ABI implication **is** the existence of the empty base (see https://quuxplusone.github.io/blog/2021/05/07/std-iterator-as-a-base-class/ ).  For example,
> ```
> #include <vector>
> #include <iterator>
> #include <stdio.h>
> struct S : std::back_insert_iterator<std::vector<int>>, std::iterator<std::output_iterator_tag, void, void, void, void> {};
> int main() { printf("%d\n", (int)sizeof(S)); }
> ```
> returns 16 when compiled with `clang++ -std=c++17 -D_LIBCPP_ABI_VERSION=1` (or before this patch), but 8 when compiled (after this patch) with `clang++ -std=c++17 -D_LIBCPP_ABI_VERSION=2`.
> IOW, the ABI break is confined to the new ABI; the old ABI continues working the same as before.
> 
> > The iterator bases were removed in C++17 [not C++14]
> 
> Darn, you're right. I had written in my post that [LWG2438](https://cplusplus.github.io/LWG/issue2438) was "a very late-breaking change to C++14," but I guess it was so late that it was C++17. ;)  @tcanens' [HTML version of N4140 (C++14)](https://timsong-cpp.github.io/cppwp/n4140/predef.iterators#reverse.iterator) agrees that the base classes are still there in C++14. Will change.
Yeah, indeed, I just realized that you were right. This is pretty terrible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103101