[libc++] [ranges] Replace __function_like inheritance with a macro.

[• Quuxplusone]

Summary

This is an alternative to D105078 which doesn't require exporting anything additional in our modulemaps.

I'm still not sure exactly what the compiler is doing in D105078, but the gist seems to be that if class C (in module M) inherits publicly from class D (in module N), and M doesn't re-export N, then it's as if that inheritance relationship doesn't exist. If a TU that imports M calls a function f(c) that would ordinarily find f(D&), it'll actually unambiguously find f(...) or whatever other less-good candidates exist. In this particular case, a call to &c doesn't find D::operator& but rather prefers the built-in candidate.

The solution adopted by me here is: don't inherit, then. "Inheritance is for sharing an interface"; here we don't want the interface to be shared; so, eliminate the inheritance and the modules problem goes with it.

[• Quuxplusone]

Having confirmed the "Modular build" step fails with the original patch ( https://buildkite.com/llvm-project/libcxx-ci/builds/4430#9311f9f8-91ca-4465-a9b1-0ca67dd641ca ), now try it with __function_like non-private (to match __hash_table etc).

[• Quuxplusone]

Note to self: The way to check the troublesome modules test locally, without a full test run, is

./bin/clang++ -std=c++20 ../libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/special_function.compile.pass.cpp -I ../libcxx/test/support/ "-fmodules" "-Xclang" "-fmodules-local-submodule-visibility" "-fcoroutines-ts" -c

[Mordante]

I don't see anything wrong with this patch. However I'm not too fond of macro's so I prefer the approach taken in D105078.
I'm not fond of export __function_like in D105078. However I see that as work-around for a clang bug; once clang has been fixed it's possible to clean this up.

Actually the problem isn't the inheritance. My guess its usage causes the problem. For example D103357 has the same issue. Here std:optional<std::locale> is a function argument. Even when the caller doesn't specify the argument (and thus uses the defaulted value for the argument) the issue occurs.

[ldionne]

I think this can be abandoned since we shipped D105078.

[• Quuxplusone]

@ldionne: Okay. Although I look forward to reopening this after the addition of another dozen Ranges algorithms start spaghettifying the modulemap beyond your tolerance for spaghetti. :)

libcxx/include/__function_like.h

 #define _LIBCPP___ITERATOR_FUNCTION_LIKE_H
 
 #include <__config>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
-_LIBCPP_PUSH_MACROS
-#include <__undef_macros>
-
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-#if !defined(_LIBCPP_HAS_NO_RANGES)
-
-namespace ranges {
 // Per [range.iter.ops.general] and [algorithms.requirements], functions in namespace std::ranges
 // can't be found by ADL and inhibit ADL when found by unqualified lookup. The easiest way to
 // facilitate this is to use function objects.
 //
 // Since these are still standard library functions, we use `__function_like` to eliminate most of
 // the properties that function objects get by default (e.g. semiregularity, addressability), to
 // limit the surface area of the unintended public interface, so as to curb the effect of Hyrum's
 // law.
-struct __function_like {
-  __function_like() = delete;
-  __function_like(__function_like const&) = delete;
-  __function_like& operator=(__function_like const&) = delete;
-
-  void operator&() const = delete;
-
-  struct __tag { };
-
-protected:
-  constexpr explicit __function_like(__tag) noexcept {}
-  ~__function_like() = default;
-};
-} // namespace ranges
-
-#endif // !defined(_LIBCPP_HAS_NO_RANGES)
-
-_LIBCPP_END_NAMESPACE_STD
-
-_LIBCPP_POP_MACROS
+#define _LIBCPP_FUNCTION_LIKE(x) \
+    struct __tag {}; \
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit x(__tag) noexcept {} \
+    x() = delete; \
+    x(const x&) = delete; \
+    void operator=(const x&) = delete; \
+    void operator&() const = delete
 
 #endif // _LIBCPP___ITERATOR_FUNCTION_LIKE_H

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 {
+  _LIBCPP_FUNCTION_LIKE(__advance_fn);
 private:
   template <class _Tp>
   _LIBCPP_HIDE_FROM_ABI
   static constexpr _Tp __abs(_Tp __n) noexcept {
     return __n < 0 ? -__n : __n;
   }
 
   template <class _Ip>
private:
   static constexpr void __advance_backward(_Ip& __i, iter_difference_t<_Ip> __n) {
     while (__n < 0) {
       ++__n;
       --__i;
     }
   }
 
 public:
-  constexpr explicit __advance_fn(__tag __x) noexcept : __function_like(__x) {}
-
   // Preconditions: If `I` does not model `bidirectional_iterator`, `n` is not negative.
   template <input_or_output_iterator _Ip>
   _LIBCPP_HIDE_FROM_ABI
   constexpr void operator()(_Ip& __i, iter_difference_t<_Ip> __n) const {
     _LIBCPP_ASSERT(__n >= 0 || bidirectional_iterator<_Ip>,
                    "If `n < 0`, then `bidirectional_iterator<I>` must be true.");
 
     // If `I` models `random_access_iterator`, equivalent to `i += n`.
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) {
if constexpr (sized_sentinel_for<_Sp, _Ip>) {
       }
       return __n;
     }
 
     _LIBCPP_UNREACHABLE();
   }
 };
 
-inline constexpr auto advance = __advance_fn(__function_like::__tag());
+inline constexpr auto advance = __advance_fn(__advance_fn::__tag());
 } // namespace ranges
 
 #endif // !defined(_LIBCPP_HAS_NO_RANGES)
 
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS
 
 #endif // _LIBCPP___ITERATOR_ADVANCE_H

libcxx/include/__iterator/next.h

inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
 
   _VSTD::advance(__x, __n);
   return __x;
 }
 
 #if !defined(_LIBCPP_HAS_NO_RANGES)
 
 namespace ranges {
-struct __next_fn final : private __function_like {
-  _LIBCPP_HIDE_FROM_ABI
-  constexpr explicit __next_fn(__tag __x) noexcept : __function_like(__x) {}
+struct __next_fn final {
+  _LIBCPP_FUNCTION_LIKE(__next_fn);
 
   template <input_or_output_iterator _Ip>
   _LIBCPP_HIDE_FROM_ABI
   constexpr _Ip operator()(_Ip __x) const {
     ++__x;
     return __x;
   }
 
struct __next_fn final {
   template <input_or_output_iterator _Ip, sentinel_for<_Ip> _Sp>
   _LIBCPP_HIDE_FROM_ABI
   constexpr _Ip operator()(_Ip __x, iter_difference_t<_Ip> __n, _Sp __bound) const {
     ranges::advance(__x, __n, __bound);
     return __x;
   }
 };
 
-inline constexpr auto next = __next_fn(__function_like::__tag());
+inline constexpr auto next = __next_fn(__next_fn::__tag());
 } // namespace ranges
 
 #endif // !defined(_LIBCPP_HAS_NO_RANGES)
 
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS
 
 #endif // _LIBCPP___ITERATOR_PRIMITIVES_H

libcxx/include/__iterator/prev.h

_LIBCPP_ASSERT(__n <= 0 || __is_cpp17_bidirectional_iterator<_InputIter>::value,
                  "Attempt to prev(it, n) with a positive n on a non-bidirectional iterator");
   _VSTD::advance(__x, -__n);
   return __x;
 }
 
 #if !defined(_LIBCPP_HAS_NO_RANGES)
 
 namespace ranges {
-struct __prev_fn final : private __function_like {
-  _LIBCPP_HIDE_FROM_ABI
-  constexpr explicit __prev_fn(__tag __x) noexcept : __function_like(__x) {}
+struct __prev_fn final {
+  _LIBCPP_FUNCTION_LIKE(__prev_fn);
 
   template <bidirectional_iterator _Ip>
   _LIBCPP_HIDE_FROM_ABI
   constexpr _Ip operator()(_Ip __x) const {
     --__x;
     return __x;
   }
 
   template <bidirectional_iterator _Ip>
   _LIBCPP_HIDE_FROM_ABI
   constexpr _Ip operator()(_Ip __x, iter_difference_t<_Ip> __n) const {
     ranges::advance(__x, -__n);
     return __x;
   }
 
   template <bidirectional_iterator _Ip>
   _LIBCPP_HIDE_FROM_ABI
   constexpr _Ip operator()(_Ip __x, iter_difference_t<_Ip> __n, _Ip __bound) const {
     ranges::advance(__x, -__n, __bound);
     return __x;
   }
 };
 
-inline constexpr auto prev = __prev_fn(__function_like::__tag());
+inline constexpr auto prev = __prev_fn(__prev_fn::__tag());
 } // namespace ranges
 
 #endif // !defined(_LIBCPP_HAS_NO_RANGES)
 
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS
 
 #endif // _LIBCPP___ITERATOR_PREV_H

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)>());