[libcxx-commits] [PATCH] D156177: [libc++][Modules] Recreate the top level `std` clang module

[Ian Anderson via Phabricator via libcxx-commits]

iana added a comment.

In D156177#4532769 <https://reviews.llvm.org/D156177#4532769>, @Mordante wrote:

> In D156177#4532637 <https://reviews.llvm.org/D156177#4532637>, @iana wrote:
>
>> In D156177#4532451 <https://reviews.llvm.org/D156177#4532451>, @Mordante wrote:
>>
>>> In D156177#4532349 <https://reviews.llvm.org/D156177#4532349>, @aprantl wrote:
>>>
>>>>> ! In D156177#4532335 <https://reviews.llvm.org/D156177#4532335>, @Mordante wrote:
>>>>>  I'm not entirely fond of this approach.
>>>>
>>>> Would you mind clarifying what your concerns are?
>>>
>>> My main concern is shipping a header to all our users that includes everything without me understanding why. Especially since that was not needed before the changes to modules. Hence my question why this is needed by LLDB.
>>>
>>> Smaller concerns, which are fixable
>>>
>>> - Something else we need to maintain. We don't often add new top-level headers and I don't see tooling to make sure we don't forget to add it.
>>> - Using a module named `std` which is a C++23 feature
>>
>> The goal is to have a single clang module that lldb can import to get all of libc++. And maybe for anyone else that's currently importing the shipping `std` clang module, although they're probably better off importing the individual modules they use like `std_algorithm`. clang modules require header files, they can't do anything without them. So for the `std` clang module to provide everything from libc++, it needs a header that includes everything from libc++.
>
> Does this mean the current module changes may be a breaking change for clang module users?
> I know this is used at Google.

Only if they import `std` by name. If they just build with modules then no*

> How did this work before your change? IIRC we didn't have a header to include at that time.
> I really would like to understand why we need a header now, when it could be done without a header before.

Before the change, every libc++ header was in the `std` mega-module as submodules.

>> We're shipping a new header that includes everything, but if anyone includes it they'll get an error. (That's what the `__building_module` check does - it makes the header error for everything _except_ doing `import std`.)
>
> Thanks for the info.
>
>> clang modules are getting to be kind of a pain to support.
>
> +1
>
>> I'm in favor of scripting that pain away, but @ldionne felt that I was just exchanging manual header/module map editing pain for debugging Python pain.
>
> Unlike D155141 <https://reviews.llvm.org/D155141> a script to generate this header should be trivial. The file `libcxx/utils/libcxx/test/header_information.py` already has the logic to get all `public_headers` in libc++. This script is used for our testing so this script will get updated when the rules for our public headers change. I feel here scripting will make things less error prone. We already have a CMake step to build generated files. So I think this would be a natural fit to add there.
>
>> The module name `std` is what the current (llvm 16) clang module name is for libc++. So that part isn't a change, it's just restoring the behavior from a few weeks ago.
>
> I see, then we can't change that.



- One thing I'm worried about with the change is that it may break the libc++ clang modules on platforms that don't support modules. See my comment at https://discord.com/channels/636084430946959380/636732894974312448/1131736544387014767


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156177/new/

https://reviews.llvm.org/D156177