[libcxx][modularisation] properly modularises advance, next, and prev

[cjdb]

Summary

__function_like wasn't being exported, so certain properties of the
ranges functions weren't being propagated in modules land.

[cjdb]

Distinct from D105076 since this patch does more than just splicing.

[cjdb]

• simplifies patch based on current libc++ trajectory

[zoecarver]

Thanks for the cleanup.

libcxx/include/module.modulemap

481–484

I don't like the formatting here. But whatever.

libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/special_function.compile.pass.cpp

17

Don't we still support clang-13 or am I missing something? Does this patch work around the bug?

[• Quuxplusone]

libcxx/include/module.modulemap

481–484

There should be no need to export __function_like here. Then it'll all fit on one line, consistent with all the other lines, so there won't be a formatting problem anymore.

787

Probably this private is the original problem. What happens if you remove it?
(Compare to __hash_table, __nullptr, etc.)

[cjdb]

libcxx/include/module.modulemap

481–484

I don't like the formatting here. But whatever.

Yeah, I didn't notice till after I threw it up on Phab. Will fix before merging.

There should be no need to export __function_like here. Then it'll all fit on one line, consistent with all the other lines, so there won't be a formatting problem anymore.

There is a need to export __function_like. I've gone into this in other patches before, so I'm not going to repeat myself here.

787

Nope. Nothing gets fixed because this isn't the problem.

libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/special_function.compile.pass.cpp

17

This patch isn't a workaround: it's the correct fix.

[Mordante]

LGTM, but please hold off landing a short time to give @Quuxplusone an opportunity to respond.

libcxx/include/module.modulemap

481–484

There should be no need to export __function_like here.

I agree, but clang disagrees. I had the same issue in my format series, before we made these headers private.
I know no other solution than what @cjdb did here.

[• Quuxplusone]

libcxx/include/__iterator/advance.h

73

Btw, should there be a private here? __next_fn and __prev_fn both use private inheritance here, not public inheritance. At least we should be consistent.
(obsoleted by D106567)

libcxx/include/module.modulemap

481–484

I've looked into it a bit, and uploaded D106567 with a simpler fix.

[cjdb]

@ldionne now needs to weigh in on this in order for any progress to be made, which I was hoping to avoid.

libcxx/include/module.modulemap

481–484

You have a strange definition of the word "simpler".

[ldionne]

Ah-Ah! So that was the issue all along then.

IMO this module-specific fix is cleaner than using a macro just to satisfy the modules build.

libcxx/include/__iterator/advance.h

 #define _LIBCPP___ITERATOR_ADVANCE_H
 
 #include <__config>
 #include <__debug>
 #include <__function_like.h>
 #include <__iterator/concepts.h>
 #include <__iterator/incrementable_traits.h>
 #include <__iterator/iterator_traits.h>
+#include <__utility/move.h>
 #include <cstdlib>
 #include <concepts>
 #include <limits>
 #include <type_traits>
-#include <utility>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
_LIBCPP_ASSERT(__n >= 0 || __is_cpp17_bidirectional_iterator<_InputIter>::value,
                  "Attempt to advance(it, n) with negative n on a non-bidirectional iterator");
   _VSTD::__advance(__i, __n, typename iterator_traits<_InputIter>::iterator_category());
 }
 
 #if !defined(_LIBCPP_HAS_NO_RANGES)
 
 namespace ranges {
 // [range.iter.op.advance]
-struct __advance_fn final : __function_like {
+struct __advance_fn final : private __function_like {
 private:
   template <class _Tp>
   _LIBCPP_HIDE_FROM_ABI
   static constexpr _Tp __abs(_Tp __n) noexcept {
     return __n < 0 ? -__n : __n;
   }
 
   template <class _Ip>
public:
   }
 
   // Preconditions: Either `assignable_from<I&, S> || sized_sentinel_for<S, I>` is modeled, or [i, bound) denotes a range.
   template <input_or_output_iterator _Ip, sentinel_for<_Ip> _Sp>
   _LIBCPP_HIDE_FROM_ABI
   constexpr void operator()(_Ip& __i, _Sp __bound) const {
     // If `I` and `S` model `assignable_from<I&, S>`, equivalent to `i = std::move(bound)`.
     if constexpr (assignable_from<_Ip&, _Sp>) {
-      __i = std::move(__bound);
+      __i = _VSTD::move(__bound);
     }
     // Otherwise, if `S` and `I` model `sized_sentinel_for<S, I>`, equivalent to `ranges::advance(i, bound - i)`.
     else if constexpr (sized_sentinel_for<_Sp, _Ip>) {
       (*this)(__i, __bound - __i);
     }
     // Otherwise, while `bool(i != bound)` is true, increments `i`.
     else {
       while (__i != __bound) {

libcxx/include/module.modulemap

module istream {
     export *
   }
   module iterator {
     header "iterator"
     export *
 
     module __iterator {
       module access                { private header "__iterator/access.h" }
-      module advance               { private header "__iterator/advance.h" }
+      module advance               {
+        private header "__iterator/advance.h"
+        export __function_like
+      }
       module back_insert_iterator  { private header "__iterator/back_insert_iterator.h" }
       module common_iterator       { private header "__iterator/common_iterator.h" }
       module concepts              { private header "__iterator/concepts.h" }
       module data                  { private header "__iterator/data.h" }
       module default_sentinel      { private header "__iterator/default_sentinel.h" }
       module distance              { private header "__iterator/distance.h" }
       module empty                 { private header "__iterator/empty.h" }
       module erase_if_container    { private header "__iterator/erase_if_container.h" }
       module front_insert_iterator { private header "__iterator/front_insert_iterator.h" }
       module incrementable_traits  { private header "__iterator/incrementable_traits.h" }
       module insert_iterator       { private header "__iterator/insert_iterator.h" }
       module istream_iterator      { private header "__iterator/istream_iterator.h" }
       module istreambuf_iterator   { private header "__iterator/istreambuf_iterator.h" }
       module iter_move             { private header "__iterator/iter_move.h" }
       module iter_swap             { private header "__iterator/iter_swap.h" }
       module iterator              { private header "__iterator/iterator.h" }
       module iterator_traits       { private header "__iterator/iterator_traits.h" }
       module move_iterator         { private header "__iterator/move_iterator.h" }
-      module next                  { private header "__iterator/next.h" }
+      module next                  {
+        private header "__iterator/next.h"
+        export __function_like
+      }
       module ostream_iterator      { private header "__iterator/ostream_iterator.h" }
       module ostreambuf_iterator   { private header "__iterator/ostreambuf_iterator.h" }
-      module prev                  { private header "__iterator/prev.h" }
+      module prev                  {
+        private header "__iterator/prev.h"
+        export __function_like
+      }
       module projected             { private header "__iterator/projected.h" }
       module readable_traits       { private header "__iterator/readable_traits.h" }
       module reverse_access        { private header "__iterator/reverse_access.h" }
       module reverse_iterator      { private header "__iterator/reverse_iterator.h" }
       module size                  { private header "__iterator/size.h" }
       module wrap_iter             { private header "__iterator/wrap_iter.h" }
     }
   }
module std [system] {
 
   // __config not modularised due to a bug in Clang
   // FIXME: These should be private.
   module __availability      { private header "__availability"      export * }
   module __bit_reference     { private header "__bit_reference"     export * }
   module __bits              { private header "__bits"              export * }
   module __debug             { header "__debug"             export * }
   module __errc              { private header "__errc"              export * }
   module __function_like     { private header "__function_like.h"   export * }
   module __hash_table        { header "__hash_table"        export * }
   module __locale            { private header "__locale"            export * }
   module __mutex_base        { private header "__mutex_base"        export * }
   module __node_handle       { private header "__node_handle"       export * }
   module __nullptr           { header "__nullptr"           export * }
   module __split_buffer      { private header "__split_buffer"      export * }
   module __std_stream        { private header "__std_stream"        export * }
   module __string            { private header "__string"            export * }

libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/special_function.compile.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 // UNSUPPORTED: libcpp-no-concepts
 // UNSUPPORTED: gcc-10
 
-// Clang has a bug when modules are enabled that causes `__function_like::operator&`
-// not to be picked up by the derived class.
-// XFAIL: clang-11 && modules-build
-// XFAIL: clang-12 && modules-build
-// XFAIL: clang-13 && modules-build
-
 // ranges::advance
 
 #include <iterator>
 
 #include "test_standard_function.h"
 
 static_assert(is_function_like<decltype(std::ranges::advance)>());
 

libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.next/special_function.compile.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 // UNSUPPORTED: libcpp-no-concepts
 // UNSUPPORTED: gcc-10
 
-// Clang has a bug when modules are enabled that causes `__function_like::operator&`
-// not to be picked up by the derived class.
-// XFAIL: clang-11 && modules-build
-// XFAIL: clang-12 && modules-build
-// XFAIL: clang-13 && modules-build
-
 // ranges::next
 
 #include <iterator>
 
 #include "test_standard_function.h"
 
 static_assert(is_function_like<decltype(std::ranges::next)>());
 

libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.prev/special_function.compile.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 // UNSUPPORTED: libcpp-no-concepts
 // UNSUPPORTED: gcc-10
 
-// Clang has a bug when modules are enabled that causes `__function_like::operator&`
-// not to be picked up by the derived class.
-// XFAIL: clang-11 && modules-build
-// XFAIL: clang-12 && modules-build
-// XFAIL: clang-13 && modules-build
-
 // ranges::prev
 
 #include <iterator>
 
 #include "test_iterators.h"
 #include "test_standard_function.h"
 
 static_assert(is_function_like<decltype(std::ranges::prev)>());