[Modules] Make clang modules for the C standard library headers

[iana]

Summary

Make top level modules for all the C standard library headers.

The __stddef implementation headers need header guards now that they're all modular. stdarg.h and stddef.h will be textual headers in the builtin modules, and so need to be repeatedly included in both the system and builtin module case. Define their header guards for consistency, but ignore them when building with modules.

__stddef_null.h needs to ignore its header guard when modules aren't being used to fulfill its redefinition obligation.
__stddef_nullptr_t.h needs to add a guard for C23 so that _Builtin_stddef can compile in C17 and earlier modes. _Builtin_stddef.nullptr_t can't require C23 because it also needs to be usable from C++.

[iana]

Remove the module cache before running the stdarg/stddef unit tests in module mode

[iana]

Add @import tests for all of the new modules

clang/lib/Headers/__stddef_null.h

14

I couldn't come up with anything clever to support this in modules. If I get rid of the guard, NULL still doesn't get redefined in modules. If I move the undef to stddef.h then NULL gets undefined, but not redefined. So I think precompiled modules must inherently have #pragma once semantics. If we really had to, we could move this back into stddef.h and let it be textual, but it's distasteful that NULL gets defined in every single module that includes stddef.h and has to be merged later.

clang/test/Headers/stdarg.c

5

I think it's fine to use these same tests to validate that modules mode makes the same things visible as non-modules mode depending on the standard and set __need_ macros?

[dexonsmith]

clang/lib/Headers/__stddef_null.h

14

Modules are, in a way, stronger than #pragma once. Each module is a separate translation unit. If there's a module around __stddef_null.h, then it will only be compiled once, and any definitions imported from there. Other importing TUs get those definitions per the context the module was compiled in (they don't re-compile the code in their own context...).

It seems like a regression for NULL to degrade from ((void *)0) to 0 since it loses type safety, which is rather rare and precious in C code. Maybe this header should remain textual when deployed to platforms that rely on this dance.

[benlangmuir]

clang/lib/Headers/module.modulemap

156

Is the assumption here that directly @import _Builtin_stddef_max_align_t is unsupported so we don't need to provide a shim to keep this working?

[iana]

clang/lib/Headers/__stddef_null.h

14

Sure, but I was a little surprised that this doesn't work.

@import _Builtin_stddef.null;
undef NULL
@import _Builtin_stddef.null;
// NULL is undefined

It makes perfect sense that the module doesn't get re-compiled on the second import (and maybe not even the first), and that when it does get precompiled it doesn't see anything that the importing code did with NULL. But I am a bit surprised that re-importing the module doesn't re-expose everything from it.

I guess if we really need to, we could unifdef the module map in case some platforms need this to be textual, and then lose the guard.

clang/lib/Headers/module.modulemap

156

Yes that is the assumption, but if someone knows different please let me know!

[dexonsmith]

clang/lib/Headers/__stddef_null.h

14

Yeah, subsequent imports have no effect. I can see how it's confusing in that situation, but it's important that they are redundant.

I guess if we really need to, we could unifdef the module map in case some platforms need this to be textual, and then lose the guard.

This makes sense to me, have it be either textual or a submodule depending on configuration for the platform. Maybe someone involved in supporting clang modules on Linux has an opinion though.

[iana]

The nullptr_t submodule can't require c23 because it needs to be used in C++ in some cases too. Remove the requires from the module map and add a C23 guard to the header.

[iana]

Update Module::directlyUses

[iana]

Ah, the tests are failing because some of them are putting the builtin headers in other modules. This is going to need the "optionally ignore the builtin modules" change first then. I almost have that one finished.

[iana]

Rebase on top of D159483

[iana]

clang/lib/Headers/module.modulemap

269

Arguably this should be textual since stddef.h shouldn't own wint_t and it's just here for compatibility. Otherwise I think the ODR would force wchar.h to define need_wint and include stddef.h which is kind of weird?

[iana]

Fix formatting issue

[iana]

Rebase

[iana]

Rebase

[ldionne]

Unfortunately I am too backed up with libc++ stuff to review with the PR transition, so I'm resigning from this one.

[benlangmuir]

clang/test/Modules/Inputs/System/usr/include/stdint.h

2

Why do we need all this code now (I assume this is copied from the real header)?

[iana]

clang/test/Modules/Inputs/System/usr/include/stdint.h

2

It's to support the tests I added to Modules/compiler_builtins.m. stdatomic.h and inttypes.h rely on stdint.h contents from the C library, and on complex.h and inttypes.h and math.h being present. I could just test the modules with -ffreestanding I think, would that be better?

2

(and yes I just copied it from the builtin header)

[benlangmuir]

Other than the giant header in the test we're still discussing, this basically LGTM.

clang/test/Modules/Inputs/System/usr/include/stdint.h

2

I'm not sure what the best approach is here, but this seems heavy for a test. Do we expect to need to keep this up to date? If so maybe ffreestanding for the specific test cases this affects would be a better tradeoff. Maybe someone else has a suggestion here

[dexonsmith]

clang/test/Modules/Inputs/System/usr/include/stdint.h

2

This reflects *most* of the content in stdint.h.

One idea would be to copy the full header over during the test setup and fix it up, either:

• strip out the header/footer or
• sed -e "s/__STDC_HOSTED__/0/g"

so that it always provides content (even when not freestanding).

[iana]

clang/test/Modules/Inputs/System/usr/include/stdint.h

2

IMO it doesn't need to be that sophisticated, the types just have to exist for stdtomic.h to compile. I could chop all the comments and define everything to int and I think that'd be fine too.

[iana]

Try deleting the module cache to fix the failing tests

[iana]

Add the header guard comment to __stddef_null.h

[iana]

• Drop the -verify off the new tests so that I can get the full errors

[iana]

• Drop the -verify off the stddef tests so that I can get the full errors

[iana]

Adjust for the changes in D159483.

Mostly the header guards that were added in that review have moved to this one, the stdarg module needs to always be allowed, and a few tests needed to adjust for the stdarg/stddef implemenetation header being always modular.

[iana]

Actually allow the stdarg module

[iana]

clang/lib/Headers/module.modulemap

269

wint_t doesn't need to be textual, it can just be split out of stddef so that it doesn't get precompiled, and won't be see at all normally.

[iana]

Update clang/test/Modules/stddef.c: it doesn't need a different verify label for -fbuiltin-headers-in-system-modules, allow the diagnostic to suggest either definer of size_t, add another test to make sure that size_t still works when both declarations are seen

[vsapsai]

Still going through the patch and through the discussion. But wanted to ask if there are any complications in reverting this change? For libc++ we've discussed that we don't really know the perf impact of multiple small modules. So I want to make sure we aren't making decisions that make it impossible to revert the change later.

[iana]

In D159064#4647709, @vsapsai wrote:

Still going through the patch and through the discussion. But wanted to ask if there are any complications in reverting this change? For libc++ we've discussed that we don't really know the perf impact of multiple small modules. So I want to make sure we aren't making decisions that make it impossible to revert the change later.

I don't think we can really revert the C stdlib headers each being in their own module; there isn't any other way around the module layering maze. libc++ is a bit different, it's not just the C stdlib headers that are each in their own module, it's every one of the ~1000 headers right now. I have misgivings about that, I made D155141 to reduce that closer to 150 modules instead of 1000 but it was poorly received. I might try to address some of the concerns after I get this one worked through.