[libcxx][ranges] Update `{front_,back_,}insert_iterator` for C++20.

[zoecarver]

Summary

Makes difference_type = std::ptrdiff_t and removes parent.

[• Quuxplusone]

libcxx/include/iterator

770–776

https://quuxplusone.github.io/blog/2021/05/07/std-iterator-as-a-base-class/

[cjdb]

This patch appears to conflict with D101729.

libcxx/include/iterator

770–776

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.

[zoecarver]

libcxx/include/iterator

770–776

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?

[• Quuxplusone]

libcxx/include/iterator

770–776

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?

[Mordante]

Thanks for working on this!

libcxx/include/iterator

770–776

I'm also in favour of adding an ABI flag.

• Actually I don't expect a lot of customers using the unstable ABI. But we could discuss whether we make the unstable ABI ABI v2 and plan how to roll that out to our customers.
• The customer can use the LIBCXX_ABI_UNSTABLE CMake flag to enable the unstable ABI, or define the individual flags manually.
• The UBSAN build bot tests it. Recently I was considering to create a buildbot for it, but I noticed we already have one.

[cjdb]

libcxx/include/iterator

770–776

who is the target audience who is expected to turn them on in production?

🤷 Jason Turner noted that in his (relatively biased) survey on breaking ABI, no one seemed to flag that ABI stability was important to them, personally (link to video).

Perhaps we should survey our users and find out

• how many of them genuinely care about ABI stability for projects they own
• which C++ standard they're currently using
• which version of LLVM they're currently using
• when they plan to upgrade LLVM and which version they're planning to upgrade to
• when they plan to move to a newer C++ standard and which they plan to move to

how do they control which flags they turn on?

I'd say that users who care about ABI stability should enable macros that they care about. Unlike the nodiscard discussion, ABI stability is something that should be opt-in, because it lets us conform to the latest standard while also preserving something that "users" supposedly care about.

Perhaps _LIBCPP_ABI_NO_CXX17_EMPTY_ITERATOR_BASE for this one, and _LIBCPP_ABI_NO_EMPTY_BASES as a general control mechanism for all the empty base deletions. We could also have _LIBCPP_NO_CXX17_ABI_BREAK (to be bikeshedded) that controls all _LIBCPP_ABI_NO_CXX17_* macros, in case users want to preserve stability with C++14. This idea is far from fleshed out, and deserves more discussion on Discord.

how are these flags tested by the buildbots?

I think we can do this with more fine grained macros and libcxx tests.

[Mordante]

libcxx/include/iterator

775

It seems the class also got a default constructor in C++20. Can you add _LIBCPP_INLINE_VISIBILITY constexpr back_insert_iterator() noexcept = default;?
Please also update the synopsis and front_insert_iterator? With these changes I can use the std::back_insert_iterator in std::format :-)

[ldionne]

I think this can be closed now.

libcxx/include/iterator

 reverse_iterator<_Iter> make_reverse_iterator(_Iter __i)
 {
     return reverse_iterator<_Iter>(__i);
 }
 #endif
 
 template <class _Container>
 class _LIBCPP_TEMPLATE_VIS back_insert_iterator
+#if _LIBCPP_STD_VER <= 17
     : public iterator<output_iterator_tag,
                       void,
                       void,
                       void,
                       void>
+#endif // _LIBCPP_STD_VER <= 17
 {
 protected:
     _Container* container;
 public:
     typedef _Container container_type;
+#if _LIBCPP_STD_VER > 17
+    typedef output_iterator_tag iterator_category;
+    typedef void value_type;
+    typedef ptrdiff_t difference_type;
+    typedef void pointer;
+    typedef void reference;
+#endif // _LIBCPP_STD_VER > 17
 
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 explicit back_insert_iterator(_Container& __x) : container(_VSTD::addressof(__x)) {}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 back_insert_iterator& operator=(const typename _Container::value_type& __value_)
         {container->push_back(__value_); return *this;}
 #ifndef _LIBCPP_CXX03_LANG
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 back_insert_iterator& operator=(typename _Container::value_type&& __value_)
         {container->push_back(_VSTD::move(__value_)); return *this;}
 #endif // _LIBCPP_CXX03_LANG
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 back_insert_iterator& operator*()     {return *this;}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 back_insert_iterator& operator++()    {return *this;}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 back_insert_iterator  operator++(int) {return *this;}
 };
 
 template <class _Container>
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
 back_insert_iterator<_Container>
 back_inserter(_Container& __x)
 {
     return back_insert_iterator<_Container>(__x);
 }
 
 template <class _Container>
 class _LIBCPP_TEMPLATE_VIS front_insert_iterator
+#if _LIBCPP_STD_VER <= 17
     : public iterator<output_iterator_tag,
                       void,
                       void,
                       void,
                       void>
+#endif // _LIBCPP_STD_VER <= 17
 {
 protected:
     _Container* container;
 public:
     typedef _Container container_type;
+#if _LIBCPP_STD_VER > 17
+    typedef output_iterator_tag iterator_category;
+    typedef void value_type;
+    typedef ptrdiff_t difference_type;
+    typedef void pointer;
+    typedef void reference;
+#endif // _LIBCPP_STD_VER > 17
 
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 explicit front_insert_iterator(_Container& __x) : container(_VSTD::addressof(__x)) {}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 front_insert_iterator& operator=(const typename _Container::value_type& __value_)
         {container->push_front(__value_); return *this;}
 #ifndef _LIBCPP_CXX03_LANG
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 front_insert_iterator& operator=(typename _Container::value_type&& __value_)
         {container->push_front(_VSTD::move(__value_)); return *this;}
 #endif // _LIBCPP_CXX03_LANG
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 front_insert_iterator& operator*()     {return *this;}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 front_insert_iterator& operator++()    {return *this;}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 front_insert_iterator  operator++(int) {return *this;}
 };
 
 template <class _Container>
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
 front_insert_iterator<_Container>
 front_inserter(_Container& __x)
 {
     return front_insert_iterator<_Container>(__x);
 }
 
 template <class _Container>
 class _LIBCPP_TEMPLATE_VIS insert_iterator
+#if _LIBCPP_STD_VER <= 17
     : public iterator<output_iterator_tag,
                       void,
                       void,
                       void,
                       void>
+#endif // _LIBCPP_STD_VER <= 17
 {
 protected:
     _Container* container;
     typename _Container::iterator iter;
 public:
     typedef _Container container_type;
+#if _LIBCPP_STD_VER > 17
+    typedef output_iterator_tag iterator_category;
+    typedef void value_type;
+    typedef ptrdiff_t difference_type;
+    typedef void pointer;
+    typedef void reference;
+#endif // _LIBCPP_STD_VER > 17
 
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 insert_iterator(_Container& __x, typename _Container::iterator __i)
         : container(_VSTD::addressof(__x)), iter(__i) {}
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 insert_iterator& operator=(const typename _Container::value_type& __value_)
         {iter = container->insert(iter, __value_); ++iter; return *this;}
 #ifndef _LIBCPP_CXX03_LANG
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 insert_iterator& operator=(typename _Container::value_type&& __value_)
         {iter = container->insert(iter, _VSTD::move(__value_)); ++iter; return *this;}

libcxx/test/std/iterators/iterator.primitives/iterator.traits/cxx20_iterator_traits.compile.pass.cpp

 static_assert(testStdlibIteratorConst<std::unordered_multiset<int>::const_iterator, int, std::forward_iterator_tag>());
 static_assert(testStdlibIteratorConst<std::unordered_multiset<int>::local_iterator, int, std::forward_iterator_tag>());
 static_assert(
     testStdlibIteratorConst<std::unordered_multiset<int>::const_local_iterator, int, std::forward_iterator_tag>());
 
 static_assert(
     testStdlibIterator<std::reverse_iterator<std::vector<int>::iterator>, int, std::random_access_iterator_tag>());
 static_assert(
-    testStdlibIterator<std::back_insert_iterator<std::vector<int>::iterator>, void, std::output_iterator_tag>());
+    testStdlibIterator<std::back_insert_iterator<std::vector<int>::iterator>, void, std::output_iterator_tag, std::ptrdiff_t>());
 static_assert(
-    testStdlibIterator<std::front_insert_iterator<std::vector<int>::iterator>, void, std::output_iterator_tag>());
-static_assert(testStdlibIterator<std::insert_iterator<std::vector<int> >, void, std::output_iterator_tag>());
+    testStdlibIterator<std::front_insert_iterator<std::vector<int>::iterator>, void, std::output_iterator_tag, std::ptrdiff_t>());
+static_assert(testStdlibIterator<std::insert_iterator<std::vector<int> >, void, std::output_iterator_tag, std::ptrdiff_t>());
 static_assert(testStdlibIteratorConst<std::istream_iterator<int, char>, int, std::input_iterator_tag>());
 
 #if !defined(_LIBCPP_HAS_NO_LOCALIZATION)
 static_assert(
     testStdlibIterator<std::istreambuf_iterator<char>, char, long long, char, char*, std::input_iterator_tag>());
 using MoveIter = std::move_iterator<std::vector<int>::iterator>;
 static_assert(
     testStdlibIterator<MoveIter, int, std::ptrdiff_t, int&&, MoveIter::pointer, std::random_access_iterator_tag>());

libcxx/test/std/iterators/predef.iterators/insert.iterators/back.insert.iterator/types.pass.cpp

 test()
 {
     typedef std::back_insert_iterator<C> R;
     C c;
     find_container<C> q(c);
     q.test();
     static_assert((std::is_same<typename R::container_type, C>::value), "");
     static_assert((std::is_same<typename R::value_type, void>::value), "");
-    static_assert((std::is_same<typename R::difference_type, void>::value), "");
+    static_assert((std::is_same<typename R::difference_type, std::ptrdiff_t>::value), "");
     static_assert((std::is_same<typename R::reference, void>::value), "");
     static_assert((std::is_same<typename R::pointer, void>::value), "");
     static_assert((std::is_same<typename R::iterator_category, std::output_iterator_tag>::value), "");
 }
 
 int main(int, char**)
 {
     test<std::vector<int> >();
 
   return 0;
 }

libcxx/test/std/iterators/predef.iterators/insert.iterators/front.insert.iterator/types.pass.cpp

 test()
 {
     typedef std::front_insert_iterator<C> R;
     C c;
     find_container<C> q(c);
     q.test();
     static_assert((std::is_same<typename R::container_type, C>::value), "");
     static_assert((std::is_same<typename R::value_type, void>::value), "");
-    static_assert((std::is_same<typename R::difference_type, void>::value), "");
+    static_assert((std::is_same<typename R::difference_type, std::ptrdiff_t>::value), "");
     static_assert((std::is_same<typename R::reference, void>::value), "");
     static_assert((std::is_same<typename R::pointer, void>::value), "");
     static_assert((std::is_same<typename R::iterator_category, std::output_iterator_tag>::value), "");
 }
 
 int main(int, char**)
 {
     test<std::vector<int> >();
 
   return 0;
 }

libcxx/test/std/iterators/predef.iterators/insert.iterators/insert.iterator/types.pass.cpp

 test()
 {
     typedef std::insert_iterator<C> R;
     C c;
     find_members<C> q(c);
     q.test();
     static_assert((std::is_same<typename R::container_type, C>::value), "");
     static_assert((std::is_same<typename R::value_type, void>::value), "");
-    static_assert((std::is_same<typename R::difference_type, void>::value), "");
+    static_assert((std::is_same<typename R::difference_type, std::ptrdiff_t>::value), "");
     static_assert((std::is_same<typename R::reference, void>::value), "");
     static_assert((std::is_same<typename R::pointer, void>::value), "");
     static_assert((std::is_same<typename R::iterator_category, std::output_iterator_tag>::value), "");
 }
 
 int main(int, char**)
 {
     test<std::vector<int> >();
 
   return 0;
 }