This is an archive of the discontinued LLVM Phabricator instance.

[libc++][Modules] Add missing includes and exports
AbandonedPublic

Authored by iana on May 25 2023, 11:19 AM.

Details

Reviewers
ldionne
Mordante
Bigcheese
philnik
Group Reviewers
Restricted Project
Summary

When the std mega module is broken up (D144322), it exposes several missing includes and exports.

Diff Detail

Event Timeline

iana created this revision.May 25 2023, 11:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2023, 11:20 AM
iana requested review of this revision.May 25 2023, 11:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2023, 11:20 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik requested changes to this revision.May 25 2023, 11:30 AM

There seem to be a lot of false-positives from the module map change.

libcxx/include/__algorithm/copy.h
16 ↗(On Diff #525709)

This include is wrong. __iterator_with_data isn't used in this file.

libcxx/include/__algorithm/copy_backward.h
16 ↗(On Diff #525709)

Same here.

libcxx/include/__algorithm/pstl_any_all_none_of.h
20

Same here.

libcxx/include/__algorithm/pstl_backend.h
13 ↗(On Diff #525709)

This should be included through <__algorithm/pstl_backends/cpu_backend.h>.

libcxx/include/__algorithm/pstl_fill.h
21

Same here.

libcxx/include/__algorithm/pstl_find.h
21

and here.

libcxx/include/__algorithm/sort.h
16 ↗(On Diff #525709)

This is also never used in sort.h.

libcxx/include/__locale_dir/locale_base_api/bsd_locale_fallbacks.h
16

bsd_locale_fallbacks.h should probably not be part of the module. This might not compile on all platforms.

libcxx/include/__mdspan/extents.h
22 ↗(On Diff #525709)

<span> is already included. There shouldn't be a need to include a forward declaration.

libcxx/include/__random/discrete_distribution.h
14

This is also never used here.

This revision now requires changes to proceed.May 25 2023, 11:30 AM
iana added a comment.May 25 2023, 11:32 AM

There seem to be a lot of false-positives from the module map change.

A lot of these were added last week, it's possible things have changed. Let me go back through the ones you pointed out.

iana updated this revision to Diff 525847.May 25 2023, 3:41 PM
iana marked 10 inline comments as done.

Review feedback, add a module for pstl_frontend_dispatch.h so that it can export __utility/forward.h

iana added inline comments.May 25 2023, 3:45 PM
libcxx/include/__locale_dir/locale_base_api/bsd_locale_fallbacks.h
16

It has to be part of the module because it's included by <locale>. But it doesn't compile on all platforms, so it's a textual header so that it only gets used if <locale> actually includes it.

libcxx/include/__mdspan/extents.h
22 ↗(On Diff #525709)

That's what I thought, but I was still getting an error without it. Let me try again, I found an issue where some modules were exporting modules that they didn't include in the first place, and that seems to make clang mad.

iana added inline comments.May 25 2023, 9:51 PM
libcxx/include/__algorithm/pstl_backend.h
13 ↗(On Diff #525709)

Oh, <__algorithm/pstl_backends/cpu_backend.h> doesn't include <__algorithm/pstl_backends/cpu_backends/backend.h>. Nothing does. I'll ad it there.

iana updated this revision to Diff 525957.May 25 2023, 11:33 PM

Remove a few invalid exports and make a few more fixes.

iana planned changes to this revision.May 26 2023, 10:02 PM

Still seeing some test failures.

libcxx/include/__algorithm/pstl_backend.h
13 ↗(On Diff #525709)

Err, typo in my find command. Each of the individual backends include <__algorithm/pstl_backends/cpu_backends/backend.h> but none of them export it. I guess they all need to.

libcxx/include/__algorithm/sort.h
16 ↗(On Diff #525709)

I think it is.

/Volumes/SoftwareDevelopment/Projects/llvm-project/build/generic-modules/include/c++/v1/__algorithm/sort.h:715:11: error: definition of '_ProjectedPred' must be imported from module 'std_private_algorithm_1.make_projected' before it is required
      if (__comp(*--__last, *__first))
          ^
/Volumes/SoftwareDevelopment/Projects/llvm-project/build/generic-modules/include/c++/v1/__algorithm/sort.h:855:8: note: in instantiation of function template specialization 'std::__introsort<std::_RangeAlgPolicy, std::_ProjectedPred<Less, Proj> &, T *, false>' requested here
  std::__introsort<_AlgPolicy,
       ^
/Volumes/SoftwareDevelopment/Projects/llvm-project/build/generic-modules/include/c++/v1/__algorithm/make_projected.h:31:8: note: definition here is not reachable
struct _ProjectedPred {
       ^
iana abandoned this revision.Jun 7 2023, 12:34 PM

I'm going to split this one up into a couple of reviews. Mostly it isn't includes that need to be added, but module exports.