[libc++][Modules] Make module exports consistent with header includes

[iana]

Summary

Some modules export modules that they don't import (i.e. that their header doesn't directly include). That sometimes works when the exported submodule is in the same module, but when the std mega module is broken up (D144322), some of the exports stop working. Make the exports and includes consistent, either by adding includes for the exports, or by removing exports for missing includes.

The concepts.equality_comparable export in std.iterator.__iterator.concepts isn't doing anything because 1) it's resolved as std.iterator.__iterator.concepts.equality_comparable and 2) there's a __concepts submodule in between std.concepts and equality_comparable. Fix it to be std.concepts.__concepts.equality_comparable.

<span> is listed in both std.span and std.experimental.span. Delete the latter module.
There is no __errc module or header, so remove that export from std.system_error.

[Mordante]

libcxx/include/__format/format_context.h

24 ↗

(On Diff #532438)

I doubt this is needed, this is part of <locale> and should be exported by <locale>. When this is really needed it should be moved to line 31 in the _LIBCPP_HAS_NO_LOCALIZATION. Otherwise the CI will fail.

libcxx/include/cwchar

113

Is this really needed, it looks weird; C headers should be independent.

libcxx/include/mdspan

64–65 ↗

(On Diff #532438)

Can you move these to the __mdspan/extents.h where the are really needed?

libcxx/include/string_view

230

The header does not use initializer_list why is it needed?

libcxx/utils/libcxx/test/header_information.py

76 ↗

(On Diff #532438)

I really like to see comment why this file is in this list.

[iana]

Reordered in the stack since the exports are now needed to revert some of the added includes that don't work.

[iana]

libcxx/include/__format/format_context.h

24 ↗

(On Diff #532438)

I added this because std.format.__format.format_context exports __locale (aka std.__locale). This currently works because they're in the same top level module, but when __format/format_context.h and __locale belong to different top level modules, the export can have no effect. (Sometimes it works and sometimes it doesn't. I think if the export is transitively included by one of the module's headers then it does work, but that's fragile.) Most of the includes added in this review are due to similar circumstances: the module for the header was exporting the module for the include. Maybe not all of the exports need to be there, but it's too difficult to track down which ones are necessary after the mega module breakup.

[iana]

Fix unit tests

[iana]

Fix unit tests, reorder in the stack

[iana]

Re-order the stack again

[iana]

More CI fixes

[iana]

Add a comment to header_information.py explaining why __iterator/readable_traits.h is a private textual header.

[ldionne]

libcxx/include/__algorithm/iterator_operations.h

23 ↗

(On Diff #533365)

Let's move the definition of iter_value_t from libcxx/include/__iterator/readable_traits.h to libcxx/include/__iterator/iterator_traits.h and fix up the existing includes of <__iterator/readable_traits.h>. This will remove the dependency of readable_traits.h on iterator_traits.h. This can be done in a separate patch.

libcxx/include/__algorithm/ranges_partial_sort.h

12 ↗

(On Diff #533365)

I think we want to *not* include this header and instead remove export algorithm.__algorithm.in_out_result from ranges_partial_sort (but not for ranges_partial_sort_copy and others).

Basically I think adding that export was a mistake that was made in https://reviews.llvm.org/D136711.

libcxx/include/__algorithm/ranges_partial_sort_copy.h

12 ↗

(On Diff #533365)

I think this one shouldn't be added, but instead we should change export algorithm.__algorithm.in_out_out_result to export algorithm.__algorithm.in_out_result in module ranges_partial_sort_copy.

libcxx/include/__chrono/high_resolution_clock.h

15 ↗

(On Diff #533365)

I think we don't want to add this one, but instead we want to

diff --git a/libcxx/include/module.modulemap.in b/libcxx/include/module.modulemap.in
index d49e339f2fbb..bac76a70ff4f 100644
--- a/libcxx/include/module.modulemap.in
+++ b/libcxx/include/module.modulemap.in
@@ -763,7 +763,6 @@ module std [system] {
         private header "__chrono/high_resolution_clock.h"
         export steady_clock
         export system_clock
-        export time_point
       }
       module literals               { private header "__chrono/literals.h" }
       module month                  { private header "__chrono/month.h" }
@@ -778,8 +777,14 @@ module std [system] {
         private header "__chrono/parser_std_format_spec.h"
       }
       module statically_widen       { private header "__chrono/statically_widen.h" }
-      module steady_clock           { private header "__chrono/steady_clock.h" }
-      module system_clock           { private header "__chrono/system_clock.h" }
+      module steady_clock           {
+        private header "__chrono/steady_clock.h"
+        export time_point
+      }
+      module system_clock           {
+        private header "__chrono/system_clock.h"
+        export time_point
+      }
       module time_point             { private header "__chrono/time_point.h" }
       module weekday                { private header "__chrono/weekday.h" }
       module year                   { private header "__chrono/year.h" }

libcxx/include/__filesystem/path.h

17

This one is definitely correct, it was a missing include.

libcxx/include/__format/format_context.h

24 ↗

(On Diff #532438)

Instead, I think locale should export __locale and format_context should only export locale, not export __locale.

libcxx/include/__ranges/all.h

14–15

Those don't make sense to me but I can't figure out what's wrong. The solution *should* be to remove the exports from module all but it doesn't work for some reason.

libcxx/include/__system_error/error_condition.h

15

This is adding a missing include -- obviously correct.

libcxx/utils/libcxx/test/header_information.py

149–152 ↗

(On Diff #533365)

Is this still going to be required after we make __iterator/readable_traits.h a standalone header?

[ldionne]

libcxx/utils/find_transitive_includes.py

1 ↗

(On Diff #533365)

This script doesn't seem to be used in this review, is there any reason why it's being added (or added right now)?

[iana]

Review feedback, split the review into 3

[iana]

libcxx/include/cwchar

113

I think it was transitively included, but cwchar explicitly exports the module, so it needs to include the header.

libcxx/include/string_view

230

string_view's module explicitly exports it, and this one I'm pretty sure wasn't being transitively included. So this export would definitely break without the include being added.

[iana]

Fix the std.concepts.__concepts.equality_comparable in std.iterator.__iterator.concepts

[iana]

Rebase

[ldionne]

libcxx/include/__memory/concepts.h

22

Why is this one needed?

libcxx/include/cwchar

113

Instead, what happens if we remove export depr.stdio_h from module cwchar? This seems like a better approach to me?

libcxx/include/string_view

230

I think we want to remove the include and also remove the export initializer_list from module string_view. Some headers are specified to include <initializer_list>, but <string_view> isn't. I suspect the export was added incorrectly.

libcxx/include/tgmath.h

27–28

What happens if we remove the exports instead? Our "C compatibility" headers are normally just #include_nexting the C library header, and we should strive to keep them as close as possible to that.

[iana]

libcxx/include/cwchar

113

I'll try that with the rest of these. I've been hesitant to remove the exports because their lack causes linker errors post-module-split that are sensitive to exactly what gets included and which templates are used. The CI catches a lot of them, but probably not all. Maybe world building everything at Apple will shake out the rest of them, I was just going for a more conservative approach and keeping what was there working.

libcxx/utils/libcxx/test/header_information.py

149–152 ↗

(On Diff #533365)

It will be required in a later change, but not this one anymore.

[iana]

Remove some of the added includes, and instead remove the associated exports

[iana]

The CI fails when __memory/concepts.h doesnt export std.type_traits.remove_reference, so add the include with a comment instead.

[ldionne]

LGTM assuming the CI passes

libcxx/include/__filesystem/path.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___FILESYSTEM_PATH_H
 #define _LIBCPP___FILESYSTEM_PATH_H
 
 #include <__algorithm/replace.h>
 #include <__algorithm/replace_copy.h>
 #include <__availability>
 #include <__config>
+#include <__functional/hash.h>
 #include <__functional/unary_function.h>
 #include <__fwd/hash.h>
 #include <__iterator/back_insert_iterator.h>
 #include <__iterator/iterator_traits.h>
 #include <__type_traits/decay.h>
 #include <__type_traits/is_pointer.h>
 #include <__type_traits/remove_const.h>
 #include <__type_traits/remove_pointer.h>

libcxx/include/__memory/concepts.h

 #include <__config>
 #include <__iterator/concepts.h>
 #include <__iterator/iterator_traits.h>
 #include <__iterator/readable_traits.h>
 #include <__ranges/access.h>
 #include <__ranges/concepts.h>
 #include <__type_traits/is_reference.h>
 #include <__type_traits/remove_cvref.h>
+#include <__type_traits/remove_reference.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if _LIBCPP_STD_VER >= 20

libcxx/include/__ranges/all.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___RANGES_ALL_H
 #define _LIBCPP___RANGES_ALL_H
 
 #include <__config>
+#include <__functional/compose.h>         // TODO(modules): Those should not be required
+#include <__functional/perfect_forward.h> //
 #include <__iterator/concepts.h>
 #include <__iterator/iterator_traits.h>
 #include <__ranges/access.h>
 #include <__ranges/concepts.h>
 #include <__ranges/owning_view.h>
 #include <__ranges/range_adaptor.h>
 #include <__ranges/ref_view.h>
 #include <__type_traits/decay.h>

libcxx/include/__ranges/transform_view.h

 #include <__concepts/convertible_to.h>
 #include <__concepts/copyable.h>
 #include <__concepts/derived_from.h>
 #include <__concepts/equality_comparable.h>
 #include <__concepts/invocable.h>
 #include <__config>
 #include <__functional/bind_back.h>
 #include <__functional/invoke.h>
+#include <__functional/perfect_forward.h>
 #include <__iterator/concepts.h>
 #include <__iterator/iterator_traits.h>
 #include <__memory/addressof.h>
 #include <__ranges/access.h>
 #include <__ranges/all.h>
 #include <__ranges/concepts.h>
 #include <__ranges/empty.h>
 #include <__ranges/movable_box.h>

libcxx/include/__system_error/error_condition.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___SYSTEM_ERROR_ERROR_CONDITION_H
 #define _LIBCPP___SYSTEM_ERROR_ERROR_CONDITION_H
 
 #include <__compare/ordering.h>
 #include <__config>
+#include <__functional/hash.h>
 #include <__functional/unary_function.h>
 #include <__system_error/errc.h>
 #include <__system_error/error_category.h>
 #include <cstddef>
 #include <string>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header

libcxx/include/cwchar

 #include <__assert> // all public C++ headers provide the assertion handler
 #include <__config>
 #include <__type_traits/apply_cv.h>
 #include <__type_traits/is_constant_evaluated.h>
 #include <__type_traits/is_equality_comparable.h>
 #include <__type_traits/is_same.h>
 #include <__type_traits/remove_cv.h>
 #include <cwctype>
+#include <stdio.h>
 
 #include <wchar.h>
 
 #ifndef _LIBCPP_WCHAR_H
 #   error <cwchar> tried including <wchar.h> but didn't find libc++'s <wchar.h> header. \
           This usually means that your header search paths are not configured properly. \
           The header search paths should contain the C++ Standard Library headers before \
           any C Standard Library, and you are probably using compiler flags that make that \

libcxx/include/module.modulemap.in

module __algorithm {
       }
       module ranges_none_of                  { private header "__algorithm/ranges_none_of.h" }
       module ranges_nth_element {
         private header "__algorithm/ranges_nth_element.h"
         export functional.__functional.ranges_operations
       }
       module ranges_partial_sort {
         private header "__algorithm/ranges_partial_sort.h"
-        export algorithm.__algorithm.in_out_result
         export functional.__functional.ranges_operations
       }
       module ranges_partial_sort_copy {
         private header "__algorithm/ranges_partial_sort_copy.h"
-        export algorithm.__algorithm.in_out_out_result
+        export algorithm.__algorithm.in_out_result
         export functional.__functional.ranges_operations
       }
       module ranges_partition                { private header "__algorithm/ranges_partition.h" }
       module ranges_partition_copy           { private header "__algorithm/ranges_partition_copy.h" }
       module ranges_partition_point          { private header "__algorithm/ranges_partition_point.h" }
       module ranges_pop_heap {
         private header "__algorithm/ranges_pop_heap.h"
         export functional.__functional.ranges_operations
module __chrono {
         @requires_LIBCXX_ENABLE_LOCALIZATION@
         private header "__chrono/formatter.h"
       }
       module hh_mm_ss               { private header "__chrono/hh_mm_ss.h" }
       module high_resolution_clock  {
         private header "__chrono/high_resolution_clock.h"
         export steady_clock
         export system_clock
-        export time_point
       }
       module literals               { private header "__chrono/literals.h" }
       module month                  { private header "__chrono/month.h" }
       module month_weekday          { private header "__chrono/month_weekday.h" }
       module monthday               { private header "__chrono/monthday.h" }
       module ostream                {
         @requires_LIBCXX_ENABLE_LOCALIZATION@
         private header "__chrono/ostream.h"
       }
       module parser_std_format_spec {
         @requires_LIBCXX_ENABLE_LOCALIZATION@
         private header "__chrono/parser_std_format_spec.h"
       }
       module statically_widen       { private header "__chrono/statically_widen.h" }
-      module steady_clock           { private header "__chrono/steady_clock.h" }
-      module system_clock           { private header "__chrono/system_clock.h" }
+      module steady_clock           {
+        private header "__chrono/steady_clock.h"
+        export time_point
+      }
+      module system_clock           {
+        private header "__chrono/system_clock.h"
+        export time_point
+      }
       module time_point             { private header "__chrono/time_point.h" }
       module weekday                { private header "__chrono/weekday.h" }
       module year                   { private header "__chrono/year.h" }
       module year_month             { private header "__chrono/year_month.h" }
       module year_month_day         { private header "__chrono/year_month_day.h" }
       module year_month_weekday     { private header "__chrono/year_month_weekday.h" }
     }
   }
module __format {
       module extended_grapheme_cluster_table { private header "__format/extended_grapheme_cluster_table.h" }
       module format_arg                      { private header "__format/format_arg.h" }
       module format_arg_store                { private header "__format/format_arg_store.h" }
       module format_args                     { private header "__format/format_args.h" }
       module format_context {
         private header "__format/format_context.h"
         export optional
         export locale
-        export __locale
       }
       module format_error                    { private header "__format/format_error.h" }
       module format_functions                {
         private header "__format/format_functions.h"
         export string
       }
       module format_fwd                      { private header "__format/format_fwd.h" }
       module format_parse_context            { private header "__format/format_parse_context.h" }
module __iterator {
       module access                  { private header "__iterator/access.h" }
       module advance                 { private header "__iterator/advance.h" }
       module back_insert_iterator    { private header "__iterator/back_insert_iterator.h" }
       module bounded_iter            { private header "__iterator/bounded_iter.h" }
       module common_iterator         { private header "__iterator/common_iterator.h" }
       module cpp17_iterator_concepts { private header "__iterator/cpp17_iterator_concepts.h" }
       module concepts {
         private header "__iterator/concepts.h"
-        export concepts.equality_comparable
+        export std.concepts.__concepts.equality_comparable
         export type_traits.common_reference
       }
       module counted_iterator      { private header "__iterator/counted_iterator.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 __iterator {
       module ostream_iterator      { private header "__iterator/ostream_iterator.h" }
       module ostreambuf_iterator   {
         private header "__iterator/ostreambuf_iterator.h"
         export iosfwd
       }
       module permutable            { private header "__iterator/permutable.h" }
       module prev                  { private header "__iterator/prev.h" }
       module projected             { private header "__iterator/projected.h" }
-      module readable_traits       {
-        private header "__iterator/readable_traits.h"
-        export __iterator.iterator_traits
-      }
+      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 segmented_iterator    { private header "__iterator/segmented_iterator.h" }
       module size                  { private header "__iterator/size.h" }
       module sortable {
         private header "__iterator/sortable.h"
         export functional.__functional.ranges_operations
       }

   }
   module map {
     header "map"
     export initializer_list
     export *
   }
   module mdspan {
     header "mdspan"
-    export array
-    export span
     export *
 
     module __mdspan {
-      module extents                         { private header "__mdspan/extents.h" }
+      module extents                         {
+        private header "__mdspan/extents.h"
+        export array
+        export span
+      }
     }
   }
   module memory {
     header "memory"
     export *
 
     module __memory {
       module addressof                       { private header "__memory/addressof.h" }

   }
   module strstream {
     @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "strstream"
     export *
   }
   module system_error {
     header "system_error"
-    export __errc
     export *
     module __system_error {
       module errc           { private header "__system_error/errc.h" }
       module error_category { private header "__system_error/error_category.h" }
       module error_code {
         private header "__system_error/error_code.h"
         export functional.__functional.hash
         export functional.__functional.unary_function
module experimental {
     module simd {
       header "experimental/simd"
       export *
     }
     module set {
       header "experimental/set"
       export *
     }
-    module span {
-      header "span"
-      export *
-    }
     module string {
       header "experimental/string"
       export *
     }
     module type_traits {
       header "experimental/type_traits"
       export *
     }

libcxx/include/string_view

 #include <__type_traits/is_array.h>
 #include <__type_traits/is_convertible.h>
 #include <__type_traits/is_same.h>
 #include <__type_traits/is_standard_layout.h>
 #include <__type_traits/is_trivial.h>
 #include <__type_traits/remove_cvref.h>
 #include <__type_traits/remove_reference.h>
 #include <__type_traits/type_identity.h>
 #include <cstddef>
+#include <initializer_list>
 #include <iosfwd>
 #include <limits>
 #include <stdexcept>
 #include <version>
 
 // standard-mandated includes
 
 // [iterator.range]

libcxx/include/tgmath.h

 
 #include <__config>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
 #ifdef __cplusplus
+#  include <ccomplex>
+#  include <cmath>
 #  include <ctgmath>
 #else
 #  if __has_include_next(<tgmath.h>)
 #    include_next <tgmath.h>
 #  endif
 #endif
 
 #endif // _LIBCPP_TGMATH_H

libcxx/test/libcxx/transitive_includes/cxx03.csv

 experimental/utility utility
 experimental/vector experimental/memory_resource
 experimental/vector vector
 filesystem compare
 filesystem concepts
 filesystem cstddef
 filesystem cstdint
 filesystem cstdlib
+filesystem cstring
 filesystem ctime
 filesystem iomanip
 filesystem iosfwd
 filesystem limits
 filesystem locale
 filesystem new
 filesystem ratio
 filesystem string

libcxx/test/libcxx/transitive_includes/cxx11.csv

 experimental/utility utility
 experimental/vector experimental/memory_resource
 experimental/vector vector
 filesystem compare
 filesystem concepts
 filesystem cstddef
 filesystem cstdint
 filesystem cstdlib
+filesystem cstring
 filesystem ctime
 filesystem iomanip
 filesystem iosfwd
 filesystem limits
 filesystem locale
 filesystem new
 filesystem ratio
 filesystem string

libcxx/test/libcxx/transitive_includes/cxx14.csv

 experimental/utility utility
 experimental/vector experimental/memory_resource
 experimental/vector vector
 filesystem compare
 filesystem concepts
 filesystem cstddef
 filesystem cstdint
 filesystem cstdlib
+filesystem cstring
 filesystem ctime
 filesystem iomanip
 filesystem iosfwd
 filesystem limits
 filesystem locale
 filesystem new
 filesystem ratio
 filesystem string

libcxx/test/libcxx/transitive_includes/cxx17.csv

 experimental/utility utility
 experimental/vector experimental/memory_resource
 experimental/vector vector
 filesystem compare
 filesystem concepts
 filesystem cstddef
 filesystem cstdint
 filesystem cstdlib
+filesystem cstring
 filesystem ctime
 filesystem iomanip
 filesystem iosfwd
 filesystem limits
 filesystem locale
 filesystem new
 filesystem ratio
 filesystem string

libcxx/test/libcxx/transitive_includes/cxx20.csv

 experimental/utility utility
 experimental/vector experimental/memory_resource
 experimental/vector vector
 filesystem compare
 filesystem concepts
 filesystem cstddef
 filesystem cstdint
 filesystem cstdlib
+filesystem cstring
 filesystem ctime
 filesystem iomanip
 filesystem iosfwd
 filesystem limits
 filesystem locale
 filesystem new
 filesystem ratio
 filesystem string

libcxx/test/libcxx/transitive_includes/cxx23.csv

 experimental/unordered_set experimental/memory_resource
 experimental/unordered_set unordered_set
 experimental/utility utility
 experimental/vector experimental/memory_resource
 experimental/vector vector
 filesystem compare
 filesystem cstddef
 filesystem cstdint
+filesystem cstring
 filesystem ctime
 filesystem iomanip
 filesystem iosfwd
 filesystem limits
 filesystem locale
 filesystem new
 filesystem ratio
 filesystem string

libcxx/test/libcxx/transitive_includes/cxx26.csv

 experimental/unordered_set experimental/memory_resource
 experimental/unordered_set unordered_set
 experimental/utility utility
 experimental/vector experimental/memory_resource
 experimental/vector vector
 filesystem compare
 filesystem cstddef
 filesystem cstdint
+filesystem cstring
 filesystem ctime
 filesystem iomanip
 filesystem iosfwd
 filesystem limits
 filesystem locale
 filesystem new
 filesystem ratio
 filesystem string