[libc++] Remove __functional_base

[• Quuxplusone]

Basically LGTM, % significant comment about Hyrum's Law.

libcxx/include/__functional_base

22–26

If you're marking this as NFC, then please make sure that each top-level header that previously transitively included <__functional_base> still (directly) includes each of these headers. E.g. if some user is doing

#include <bitset>
auto x = typeid(int);  // depends on <typeinfo>

we don't want to break them. You can mark any #include lines added in this way with a comment like // TODO: remove this.

IMHO we don't need to add direct inclusions to <format> or <ranges>, but I'd recommend doing it for all the others. (And by doing it for <iterator>, you'll pick up <ranges> for free anyway.)

libcxx/test/std/ranges/range.factories/range.iota.view/iterator/minus.pass.cpp

20

Please leave <ranges> first; historically this is our style for tests that test a particular header (although I admit I don't see much point to it).

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/deduct.pass.cpp

22 ↗

(On Diff #407528)

This doesn't look needed. Why would we need this here?

[ldionne]

libcxx/include/__functional_base

22–26

This ^

Removing includes is a very common source of breakage. Unfortunately, people don't include what they use all the time, and as a result they can be broken when we remove a transitive include. Removing headers is OK, but it has to be called out explicitly and release-noted, and concretely we have to be aware that it's going to trigger some work on the vendor side (they will see failures downstream when rebuilding code and they will have to fix the projects that didn't have the correct includes).

Ideally, this commit would remove __functional_base while still making sure we include all the stuff we used to include. And in a separate commit, we can remove those includes and release-note it. I can also run a build and try to see what the impact of the removal would be.

[Mordante]

libcxx/include/__functional_base

22–26

Wouldn't it be easier to remove all includes from __functional_base and then remove it in a separate commit?
+1 for adding this to the release notes.

(A slight side note, but maybe it's a good time to try to remove some of the unneeded transitive includes in this release cycle. That might break more, but then it's a one time pain to fix it for users. The upside is that compilation will be faster when not everything includes all of <algorithm>. I will volunteer to do the cleanup.)

[philnik]

• Add headers

libcxx/test/std/ranges/range.factories/range.iota.view/iterator/minus.pass.cpp

20

I didn't know that rule and nobody has said anything in my other PRs until now, so I'd like to know @ldionne's opinion on this. I don't see much point to it either.

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/deduct.pass.cpp

22 ↗

(On Diff #407528)

This is needed in deduction_guides_sfinare_checks.h for std::equal_to (I think). Including it directly in the header didn't work. I'm not sure if this is a compiler bug, or if it's intended.

[• Quuxplusone]

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/deduct.pass.cpp

22 ↗

(On Diff #407528)

Including <functional> from deduction_guides_sfinae_checks.h didn't work? Can you link to the CI results (or reupload to generate some new CI results)?
Or, since this is moot after the Hyrum's Law thing, IMHO you could just not touch this file and leave it for later investigation (at whatever time in the future we make <memory> stop providing a definition for std::equal_to).

[ldionne]

I'm fine with this in essence, and I'm looking to a follow-up PR where we remove unneeded includes (from the headers in this patch but also from other places, I'm sure we have a bunch).

libcxx/include/__functional_base

22–26

I agree -- if we want to remove unused transitive includes, it would be a pretty good time in the cycle to do it. As far as I'm concerned, this will leave plenty of time for vendors to shake out issues on their end before we cut the release. And if things break in unexpected and bad ways, we can always revert or tweak.

libcxx/test/std/ranges/range.factories/range.iota.view/iterator/minus.pass.cpp

20

That's true, we've always done this (rather blindly TBH). We may not have noticed on your other PRs.

The benefit of having the header first is that we make sure the header can be included when nothing else has been included before. I'm fine with not doing this anymore in our tests, however I think it would be nice to also have generated tests that simply include every single public header in an otherwise empty translation unit, just to make sure it works.

TLDR: Let's test this properly and then we can safely drop this weird custom we have. I'm fine with this patch not following the usual rule.

[ldionne]

(If that wasn't clear, I'll let @Quuxplusone give the final 🚢 it because he had comments)

[philnik]

libcxx/test/std/ranges/range.factories/range.iota.view/iterator/minus.pass.cpp

20

You mean something like test/libcxx/inclusions? (and extend it to generate tests for all headers)

[• Quuxplusone]

LGTM except that I'd like to see #include <functional> moved
from test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/deduct.pass.cpp
into test/support/deduction_guides_sfinae_checks.h before this lands. Since, if that doesn't work, then we don't understand something here, and we ought to understand it before we land it.

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.ctor/deduct.pass.cpp

22 ↗

(On Diff #407528)

Including <functional> from deduction_guides_sfinae_checks.h didn't work? Can you link to the CI results (or reupload to generate some new CI results)?

Let's figure out what's going on here, before landing this.

[ldionne]

libcxx/test/std/ranges/range.factories/range.iota.view/iterator/minus.pass.cpp

20

Yes, exactly, but I wouldn't necessarily extend generate_header_inclusion_tests.py, I would just create another one that includes literally every single header, regardless of whether they have mandated transitive includes or not.

libcxx/include/CMakeLists.txt

set(files
   __functional/pointer_to_binary_function.h
   __functional/pointer_to_unary_function.h
   __functional/ranges_operations.h
   __functional/reference_wrapper.h
   __functional/unary_function.h
   __functional/unary_negate.h
   __functional/unwrap_ref.h
   __functional/weak_result_type.h
-  __functional_base
   __hash_table
   __ios/fpos.h
   __iterator/access.h
   __iterator/advance.h
   __iterator/back_insert_iterator.h
   __iterator/common_iterator.h
   __iterator/concepts.h
   __iterator/counted_iterator.h

libcxx/include/__format/format_arg.h

 #ifndef _LIBCPP___FORMAT_FORMAT_ARG_H
 #define _LIBCPP___FORMAT_FORMAT_ARG_H
 
 #include <__concepts/arithmetic.h>
 #include <__config>
 #include <__format/format_error.h>
 #include <__format/format_fwd.h>
 #include <__format/format_parse_context.h>
-#include <__functional_base>
 #include <__memory/addressof.h>
 #include <__variant/monostate.h>
 #include <string>
 #include <string_view>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif

libcxx/include/__functional_base

-// -*- C++ -*-
-//===----------------------------------------------------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef _LIBCPP_FUNCTIONAL_BASE
-#define _LIBCPP_FUNCTIONAL_BASE
-
-#include <__config>
-#include <__functional/binary_function.h>
-#include <__functional/invoke.h>
-#include <__functional/operations.h>
-#include <__functional/reference_wrapper.h>
-#include <__functional/unary_function.h>
-#include <__functional/weak_result_type.h>
-#include <__memory/allocator_arg_t.h>
-#include <__memory/uses_allocator.h>
-#include <exception>
-#include <new>
-#include <type_traits>
-#include <typeinfo>
-#include <utility>
-
-#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
-#  pragma GCC system_header
-#endif
-
-#endif // _LIBCPP_FUNCTIONAL_BASE

libcxx/include/__memory/shared_ptr.h

 #ifndef _LIBCPP___MEMORY_SHARED_PTR_H
 #define _LIBCPP___MEMORY_SHARED_PTR_H
 
 #include <__availability>
 #include <__config>
 #include <__functional/binary_function.h>
 #include <__functional/operations.h>
 #include <__functional/reference_wrapper.h>
-#include <__functional_base>
 #include <__memory/addressof.h>
 #include <__memory/allocation_guard.h>
 #include <__memory/allocator.h>
 #include <__memory/allocator_traits.h>
 #include <__memory/compressed_pair.h>
 #include <__memory/pointer_traits.h>
 #include <__memory/unique_ptr.h>
 #include <__utility/forward.h>

libcxx/include/__memory/unique_ptr.h

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP___MEMORY_UNIQUE_PTR_H
 #define _LIBCPP___MEMORY_UNIQUE_PTR_H
 
 #include <__config>
 #include <__functional/hash.h>
 #include <__functional/operations.h>
-#include <__functional_base>
 #include <__memory/allocator_traits.h> // __pointer
 #include <__memory/compressed_pair.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
 #include <cstddef>
 #include <type_traits>
 
 #if _LIBCPP_STD_VER <= 14 || defined(_LIBCPP_ENABLE_CXX17_REMOVED_AUTO_PTR)

libcxx/include/bitset

 template <size_t N> struct hash<std::bitset<N>>;
 
 }  // std
 
 */
 
 #include <__bit_reference>
 #include <__config>
-#include <__functional_base>
 #include <climits>
 #include <cstddef>
 #include <iosfwd>
 #include <stdexcept>
 #include <string>
 #include <version>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)

libcxx/include/experimental/__memory

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP_EXPERIMENTAL___MEMORY
 #define _LIBCPP_EXPERIMENTAL___MEMORY
 
-#include <__functional_base>
 #include <__memory/allocator_arg_t.h>
 #include <__memory/uses_allocator.h>
 #include <experimental/__config>
 #include <experimental/utility> // for erased_type
 #include <type_traits>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header

libcxx/include/iterator

 template <class E> constexpr const E* data(initializer_list<E> il) noexcept;      // C++17
 
 }  // std
 
 */
 
 #include <__config>
 #include <__debug>
-#include <__functional_base>
 #include <__iterator/access.h>
 #include <__iterator/advance.h>
 #include <__iterator/back_insert_iterator.h>
 #include <__iterator/common_iterator.h>
 #include <__iterator/concepts.h>
 #include <__iterator/counted_iterator.h>
 #include <__iterator/data.h>
 #include <__iterator/default_sentinel.h>

 #include <compare>
 #include <concepts> // Mandated by the Standard.
 #include <cstddef>
 #include <initializer_list>
 #include <type_traits>
 #include <utility>
 #include <version>
 
+// TODO: remove these headers
+#include <__functional/binary_function.h>
+#include <__functional/invoke.h>
+#include <__functional/operations.h>
+#include <__functional/reference_wrapper.h>
+#include <__functional/unary_function.h>
+#include <__functional/weak_result_type.h>
+#include <__memory/allocator_arg_t.h>
+#include <__memory/uses_allocator.h>
+#include <exception>
+#include <new>
+#include <typeinfo>
+
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 #endif // _LIBCPP_ITERATOR

libcxx/include/memory

 
 void* align(size_t alignment, size_t size, void*& ptr, size_t& space);
 
 }  // std
 
 */
 
 #include <__config>
-#include <__functional_base>
 #include <__memory/addressof.h>
 #include <__memory/allocation_guard.h>
 #include <__memory/allocator.h>
 #include <__memory/allocator_arg_t.h>
 #include <__memory/allocator_traits.h>
 #include <__memory/compressed_pair.h>
 #include <__memory/concepts.h>
 #include <__memory/construct_at.h>

 #include <new>
 #include <stdexcept>
 #include <tuple>
 #include <type_traits>
 #include <typeinfo>
 #include <utility>
 #include <version>
 
+// TODO: remove these headers
+#include <__functional/binary_function.h>
+#include <__functional/invoke.h>
+#include <__functional/operations.h>
+#include <__functional/reference_wrapper.h>
+#include <__functional/unary_function.h>
+#include <__functional/weak_result_type.h>
+
 #if _LIBCPP_STD_VER <= 14 || defined(_LIBCPP_ENABLE_CXX17_REMOVED_AUTO_PTR)
 #   include <__memory/auto_ptr.h>
 #endif
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 

libcxx/include/optional

 } // namespace std
 
 */
 
 #include <__availability>
 #include <__concepts/invocable.h>
 #include <__config>
 #include <__debug>
-#include <__functional_base>
 #include <compare>
 #include <functional>
 #include <initializer_list>
 #include <new>
 #include <stdexcept>
 #include <type_traits>
 #include <utility>
 #include <version>
 
+// TODO: remove these headers
+#include <__memory/allocator_arg_t.h>
+#include <__memory/uses_allocator.h>
+#include <typeinfo>
+
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 namespace std  // purposefully not using versioning namespace
 {
 
 class _LIBCPP_EXCEPTION_ABI _LIBCPP_AVAILABILITY_BAD_OPTIONAL_ACCESS bad_optional_access

libcxx/include/string

 basic_string<char32_t> operator "" s( const char32_t *str, size_t len ); // C++14
 
 }  // std
 
 */
 
 #include <__config>
 #include <__debug>
-#include <__functional_base>
 #include <__ios/fpos.h>
 #include <__iterator/wrap_iter.h>
 #include <algorithm>
 #include <compare>
 #include <cstdio>  // EOF
 #include <cstdlib>
 #include <cstring>
 #include <initializer_list>
 #include <iosfwd>
 #include <iterator>
 #include <memory>
 #include <stdexcept>
 #include <string_view>
 #include <type_traits>
 #include <utility>
 #include <version>
 
+// TODO: remove these headers
+#include <__functional/binary_function.h>
+#include <__functional/invoke.h>
+#include <__functional/operations.h>
+#include <__functional/reference_wrapper.h>
+#include <__functional/unary_function.h>
+#include <__functional/weak_result_type.h>
+#include <new>
+#include <typeinfo>
+
 #ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
 #  include <cwchar>
 #endif
 
 #ifndef _LIBCPP_HAS_NO_UNICODE_CHARS
 #  include <cstdint>
 #endif
 

libcxx/include/system_error

 
 }  // std
 
 */
 
 #include <__config>
 #include <__errc>
 #include <__functional/unary_function.h>
-#include <__functional_base>
 #include <compare>
 #include <stdexcept>
 #include <string>
 #include <type_traits>
 #include <version>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header

libcxx/include/thread

 }  // this_thread
 
 }  // std
 
 */
 
 #include <__config>
 #include <__debug>
-#include <__functional_base>
 #include <__mutex_base>
 #include <__thread/poll_with_backoff.h>
 #include <__thread/timed_backoff_policy.h>
 #include <__threading_support>
 #include <__utility/forward.h>
 #include <chrono>
 #include <cstddef>
 #include <functional>

libcxx/include/tuple

 }  // std
 
 */
 
 #include <__compare/common_comparison_category.h>
 #include <__compare/synth_three_way.h>
 #include <__config>
 #include <__functional/unwrap_ref.h>
-#include <__functional_base>
 #include <__memory/allocator_arg_t.h>
 #include <__memory/uses_allocator.h>
 #include <__tuple>
 #include <__utility/forward.h>
 #include <__utility/integer_sequence.h>
 #include <__utility/move.h>
 #include <compare>
 #include <cstddef>
 #include <type_traits>
 #include <utility>
 #include <version>
 
+// TODO: remove these headers
+#include <__functional/binary_function.h>
+#include <__functional/invoke.h>
+#include <__functional/operations.h>
+#include <__functional/reference_wrapper.h>
+#include <__functional/unary_function.h>
+#include <__functional/weak_result_type.h>
+#include <exception>
+#include <new>
+#include <typeinfo>
+
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #ifndef _LIBCPP_CXX03_LANG
 

libcxx/include/typeindex

 };
 
 }  // std
 
 */
 
 #include <__config>
 #include <__functional/unary_function.h>
-#include <__functional_base>
 #include <compare>
 #include <typeinfo>
 #include <version>
 
+// TODO: remove these headers
+#include <__functional/binary_function.h>
+#include <__functional/invoke.h>
+#include <__functional/operations.h>
+#include <__functional/reference_wrapper.h>
+#include <__functional/weak_result_type.h>
+#include <__memory/allocator_arg_t.h>
+#include <__memory/uses_allocator.h>
+#include <new>
+#include <utility>
+
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 class _LIBCPP_TEMPLATE_VIS type_index
 {

libcxx/include/variant

 
 } // namespace std
 
 */
 
 #include <__availability>
 #include <__config>
 #include <__functional/hash.h>
+#include <__functional/operations.h>
+#include <__functional/unary_function.h>
 #include <__tuple>
 #include <__utility/forward.h>
 #include <__variant/monostate.h>
 #include <compare>
 #include <exception>
 #include <initializer_list>
 #include <limits>
 #include <new>
 #include <tuple>
 #include <type_traits>
 #include <utility>
 #include <version>
 
+// TODO: remove these headers
+#include <__functional/binary_function.h>
+#include <__functional/invoke.h>
+#include <__functional/reference_wrapper.h>
+#include <__functional/weak_result_type.h>
+#include <__memory/allocator_arg_t.h>
+#include <__memory/uses_allocator.h>
+#include <typeinfo>
+
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
 namespace std { // explicitly not using versioning namespace

libcxx/include/vector

 
 }  // std
 
 */
 
 #include <__bit_reference>
 #include <__config>
 #include <__debug>
-#include <__functional_base>
 #include <__iterator/iterator_traits.h>
 #include <__iterator/wrap_iter.h>
 #include <__split_buffer>
 #include <__utility/forward.h>
 #include <algorithm>
 #include <climits>
 #include <compare>
 #include <cstdlib>
 #include <cstring>
 #include <initializer_list>
 #include <iosfwd> // for forward declaration of vector
 #include <limits>
 #include <memory>
 #include <stdexcept>
 #include <type_traits>
 #include <version>
 
+// TODO: remove these headers
+#include <__functional/binary_function.h>
+#include <__functional/invoke.h>
+#include <__functional/operations.h>
+#include <__functional/reference_wrapper.h>
+#include <__functional/unary_function.h>
+#include <__functional/weak_result_type.h>
+#include <typeinfo>
+#include <utility>
+
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
 

libcxx/test/std/ranges/range.factories/range.iota.view/iterator/member_typedefs.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: libcpp-has-no-incomplete-ranges
 
 // Test iterator category and iterator concepts.
 
-#include <ranges>
 #include <cassert>
+#include <cstdint>
+#include <ranges>
 
 #include "test_macros.h"
 #include "../types.h"
 
 struct Decrementable {
   using difference_type = int;
 
   auto operator<=>(const Decrementable&) const = default;

libcxx/test/std/ranges/range.factories/range.iota.view/iterator/minus.pass.cpp

 // UNSUPPORTED: libcpp-no-concepts
 // UNSUPPORTED: libcpp-has-no-incomplete-ranges
 
 // friend constexpr iterator operator-(iterator i, difference_type n)
 //   requires advanceable<W>;
 // friend constexpr difference_type operator-(const iterator& x, const iterator& y)
 //   requires advanceable<W>;
 
-#include <ranges>
 #include <cassert>
+#include <cstdint>
+#include <ranges>
 
 #include "test_macros.h"
 #include "../types.h"
 
 // If we're compiling for 32 bit or windows, int and long are the same size, so long long is the correct difference type.
 #if INTPTR_MAX == INT32_MAX || defined(_WIN32)
 using IntDiffT = long long;
 #else

libcxx/utils/generate_private_header_tests.py

 def relative_path(path):
     return path.as_posix()[len(include_path + '/'):]
 
 
 def is_still_public(path):
     rp = relative_path(path)
     return not rp.startswith('__support') and rp not in [
         "__bsd_locale_defaults.h", "__bsd_locale_fallbacks.h", "__config",
-        "__config_site.in", "__debug", "__hash_table", "__functional_base",
+        "__config_site.in", "__debug", "__hash_table",
         "__libcpp_version", "__threading_support", "__tree", "__undef_macros"
     ]
 
 
 def main():
     paths = [
         relative_path(p) for p in Path(include_path).rglob('*')
         if relative_path(p).startswith('__') and not p.is_dir()

llvm/utils/gn/secondary/libcxx/include/BUILD.gn

sources = [
       "__functional/pointer_to_binary_function.h",
       "__functional/pointer_to_unary_function.h",
       "__functional/ranges_operations.h",
       "__functional/reference_wrapper.h",
       "__functional/unary_function.h",
       "__functional/unary_negate.h",
       "__functional/unwrap_ref.h",
       "__functional/weak_result_type.h",
-      "__functional_base",
       "__hash_table",
       "__ios/fpos.h",
       "__iterator/access.h",
       "__iterator/advance.h",
       "__iterator/back_insert_iterator.h",
       "__iterator/common_iterator.h",
       "__iterator/concepts.h",
       "__iterator/counted_iterator.h",