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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
