Revert "[libc++][PSTL] Add missing includes to PSTL headers"

[iana]

Summary

It doesn't help modules to add extra includes for things that are already transitively included. What's needed is additional module exports.

This reverts commit 5ac0c1af8215ae5b084235c34be60a50dbe4e92e.

[iana]

Reorder in the stack

[iana]

Rebase

[iana]

Rebase

[iana]

Reordering in the stack

[ldionne]

I don't understand this patch. We follow include-what-you-use in libc++. These headers are using the entities declared by the includes you're removing, I think we should keep them. In other words, those files should have been including these headers since they were written, regardless of whether it helps with disentangling the Clang modules story or not. Do I misunderstand something? If not, I think we can abandon this.

[iana]

Rebase

[iana]

In D153209#4438890, @ldionne wrote:

I don't understand this patch. We follow include-what-you-use in libc++. These headers are using the entities declared by the includes you're removing, I think we should keep them. In other words, those files should have been including these headers since they were written, regardless of whether it helps with disentangling the Clang modules story or not. Do I misunderstand something? If not, I think we can abandon this.

I guess it doesn't hurt to have these extra includes, but they mask the missing module exports which are important to have. Without the exports, many unit tests have compiler errors, and even linker errors.

[iana]

Rebase

[iana]

Rebase

[iana]

Rebase

[iana]

Rebase

[iana]

Rebase

[iana]

Rebase

[iana]

Rebase

[iana]

Rebase, fix merge conflict

[ldionne]

I guess it doesn't hurt to have these extra includes, but they mask the missing module exports which are important to have. Without the exports, many unit tests have compiler errors, and even linker errors.

We follow include-what-you-use in libc++ -- that's the baseline rule. This patch moves us away from include-what-you-use, so I don't think we want to move forward with it. The stuff we export from the modulemap is not what we consider the baseline -- we tweak it however it needs to be tweaked to fix stuff (a rather poor strategy TBH).

If it's not needed for your stack of patches (I think it's not), then I would drop this.

[iana]

In D153209#4472107, @ldionne wrote:

I guess it doesn't hurt to have these extra includes, but they mask the missing module exports which are important to have. Without the exports, many unit tests have compiler errors, and even linker errors.

We follow include-what-you-use in libc++ -- that's the baseline rule. This patch moves us away from include-what-you-use, so I don't think we want to move forward with it. The stuff we export from the modulemap is not what we consider the baseline -- we tweak it however it needs to be tweaked to fix stuff (a rather poor strategy TBH).

If it's not needed for your stack of patches (I think it's not), then I would drop this.

Alright, I'll drop these reviews. They ended up being the wrong fix for the errors I was seeing in D144322, the exports were needed for the lots of linker errors I was seeing, But I agree it's correct to have these includes.

libcxx/include/__algorithm/pstl_find.h

 #define _LIBCPP___ALGORITHM_PSTL_FIND_H
 
 #include <__algorithm/comp.h>
 #include <__algorithm/find.h>
 #include <__algorithm/pstl_backend.h>
 #include <__algorithm/pstl_frontend_dispatch.h>
 #include <__config>
 #include <__iterator/cpp17_iterator_concepts.h>
-#include <__iterator/iterator_traits.h>
 #include <__type_traits/enable_if.h>
 #include <__type_traits/is_execution_policy.h>
 #include <__type_traits/remove_cvref.h>
 #include <__utility/move.h>
 #include <__utility/terminate_on_exception.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header

libcxx/include/__pstl/internal/execution_impl.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 _PSTL_EXECUTION_IMPL_H
 #define _PSTL_EXECUTION_IMPL_H
 
 #include <__config>
 #include <__iterator/iterator_traits.h>
 #include <__type_traits/conditional.h>
 #include <__type_traits/conjunction.h>
-#include <__type_traits/decay.h>
-#include <__type_traits/integral_constant.h>
 #include <__type_traits/is_base_of.h>
 
-#include <__pstl/internal/execution_defs.h>
+#include "execution_defs.h"
 
 #if !defined(_LIBCPP_HAS_NO_INCOMPLETE_PSTL) && _LIBCPP_STD_VER >= 17
 
 namespace __pstl {
 namespace __internal {
 
 template <typename _IteratorTag, typename... _IteratorTypes>
 using __are_iterators_of = std::conjunction<

libcxx/include/__pstl/internal/parallel_backend_serial.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 _PSTL_PARALLEL_BACKEND_SERIAL_H
 #define _PSTL_PARALLEL_BACKEND_SERIAL_H
 
 #include <__config>
 #include <__memory/allocator.h>
 #include <__pstl/internal/execution_impl.h>
 #include <__utility/forward.h>
-#include <cstddef>
 
 #if !defined(_LIBCPP_HAS_NO_INCOMPLETE_PSTL) && _LIBCPP_STD_VER >= 17
 
 namespace __pstl
 {
 namespace __serial_backend
 {
 

libcxx/include/__pstl/internal/unseq_backend_simd.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 _PSTL_UNSEQ_BACKEND_SIMD_H
 #define _PSTL_UNSEQ_BACKEND_SIMD_H
 
 #include <__config>
 #include <__functional/operations.h>
-#include <__iterator/iterator_traits.h>
 #include <__type_traits/is_arithmetic.h>
-#include <__type_traits/is_same.h>
-#include <__utility/move.h>
 #include <__utility/pair.h>
 #include <cstddef>
 #include <cstdint>
 
-#include <__pstl/internal/utils.h>
+#include "utils.h"
 
 // This header defines the minimum set of vector routines required
 // to support parallel STL.
 
 #if !defined(_LIBCPP_HAS_NO_INCOMPLETE_PSTL) && _LIBCPP_STD_VER >= 17
 
 namespace __pstl
 {