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

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.


================
Comment at: clang/lib/Headers/__stddef_null.h:14
+ * __need_NULL and rely on stddef.h to redefine NULL to the correct value again.
+ * Modules don't support redefining macros like that, but support that pattern
+ * in the non-modules case.
----------------
iana wrote:
> dexonsmith wrote:
> > iana wrote:
> > > 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.
> > 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.
> 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.
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064