[libcxx-commits] [PATCH] D102657: [libcxx][ranges] Update `{front_, back_, }insert_iterator` for C++20.

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/iterator:770-776
+#if _LIBCPP_STD_VER <= 17
     : public iterator<output_iterator_tag,
                       void,
                       void,
                       void,
                       void>
+#endif // _LIBCPP_STD_VER <= 17
----------------
zoecarver wrote:
> cjdb wrote:
> > Quuxplusone wrote:
> > > https://quuxplusone.github.io/blog/2021/05/07/std-iterator-as-a-base-class/
> > It's an ABI break that I'm okay with (since I'm okay with //all// ABI breaks), but we should probably discuss this on the mailing list.
> Ah yes I forgot about that discussion. Would it be better to just add a dummy base class that we inherit from in ABI-stable mode after C++17?
> Would it be better to just add a dummy base class that we inherit from in ABI-stable mode

Yes, iff that base class is spelled `iterator<output_iterator_tag, void, void, void, void>`. Anything else would be an ABI break.
I tentatively suggest making an ABI flag for `_LIBCPP_ABI_NO_EMPTY_BASES` affecting //all// the obsolete empty bases; and rolling `reverse_iterator`'s 2^n explosion into that flag as well.  The reason I'm "tentative" is that I still don't understand what is the point of all these ABI flags —
- who is the target audience who is expected to turn them on in production?
- how do they control which flags they turn on?
- how are these flags tested by the buildbots?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102657