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

[Med Ismail Bennani via Phabricator via cfe-commits]

mib added a comment.

Hey @iana, I think this broke the lldb bot 
https://green.lab.llvm.org/green/view/LLDB/job/as-lldb-cmake/6698/

Can you take a look ?


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

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

[Ian Anderson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9a7a6dd3c358: [Modules] Make clang modules for the C 
standard library headers (authored by iana).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_max_align_t.h
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/__stddef_offsetof.h
  clang/lib/Headers/__stddef_ptrdiff_t.h
  clang/lib/Headers/__stddef_rsize_t.h
  clang/lib/Headers/__stddef_size_t.h
  clang/lib/Headers/__stddef_unreachable.h
  clang/lib/Headers/__stddef_wchar_t.h
  clang/lib/Headers/__stddef_wint_t.h
  clang/lib/Headers/module.modulemap
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stddef.h
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/Inputs/System/usr/include/complex.h
  clang/test/Modules/Inputs/System/usr/include/inttypes.h
  clang/test/Modules/Inputs/System/usr/include/math.h
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/Modules/Inputs/System/usr/include/stdint.h
  clang/test/Modules/compiler_builtins.m
  clang/test/Modules/stddef.c
  clang/test/Modules/stddef.m

Index: clang/test/Modules/stddef.m
===
--- clang/test/Modules/stddef.m
+++ clang/test/Modules/stddef.m
@@ -4,4 +4,6 @@
 
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,13 +1,29 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
 
 #include "ptrdiff_t.h"
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
-// expected-note@__stddef_size_t.h:* {{here}}
+// size_t is declared in both size_t.h and __stddef_size_t.h, both of which are
+// modular headers. Regardless of whether stddef.h joins the StdDef test module
+// or is in its _Builtin_stddef module, __stddef_size_t.h will be in
+// _Builtin_stddef.size_t. It's not defined which module will win as the expected
+// provider of size_t. For the purposes of this test it doesn't matter which header
+// gets reported, just as long as it isn't other.h or include_again.h.
+size_t st; // expected-error-re {{missing '#include "{{size_t|__stddef_size_t}}.h"'; 'size_t' must be declared before it is used}}
+// expected-note@size_t.h:* 0+ {{here}}
+// expected-note@__stddef_size_t.h:* 0+ {{here}}
 
 #include "include_again.h"
+// Includes  which includes <__stddef_size_t.h> which imports the
+// _Builtin_stddef.size_t module.
 
 size_t st2;
+
+#include "size_t.h"
+// Redeclares size_t, but the type merger should figure it out.
+
+size_t st3;
Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
 // expected-no-diagnostics
 
 #ifdef __SSE__
@@ -11,3 +11,19 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+#ifndef NO_SYSTEM_MODULES
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import 

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

[Ian Anderson via Phabricator via cfe-commits]

iana marked 5 inline comments as done.
iana added inline comments.



Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2
 typedef int my_awesome_nonstandard_integer_type;
+
+/* C99 7.18.1.1 Exact-width integer types.

dexonsmith wrote:
> Bigcheese wrote:
> > iana wrote:
> > > dexonsmith wrote:
> > > > benlangmuir wrote:
> > > > > iana wrote:
> > > > > > iana wrote:
> > > > > > > benlangmuir wrote:
> > > > > > > > Why do we need all this code now (I assume this is copied from 
> > > > > > > > the real header)?
> > > > > > > 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?
> > > > > > (and yes I just copied it from the builtin header)
> > > > > 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
> > > > 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).
> > > 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.
> > Looking at stdatomic.h I think this approach would be fine. It just wants 
> > the types to exist.
> SGTM.
Mostly copied clang/test/Modules/Inputs/libc-libcxx/sysroot/usr/include/stdint.h


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

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 557549.
iana added a comment.

Minimize the testing stdint.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_max_align_t.h
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/__stddef_offsetof.h
  clang/lib/Headers/__stddef_ptrdiff_t.h
  clang/lib/Headers/__stddef_rsize_t.h
  clang/lib/Headers/__stddef_size_t.h
  clang/lib/Headers/__stddef_unreachable.h
  clang/lib/Headers/__stddef_wchar_t.h
  clang/lib/Headers/__stddef_wint_t.h
  clang/lib/Headers/module.modulemap
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stddef.h
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/Inputs/System/usr/include/complex.h
  clang/test/Modules/Inputs/System/usr/include/inttypes.h
  clang/test/Modules/Inputs/System/usr/include/math.h
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/Modules/Inputs/System/usr/include/stdint.h
  clang/test/Modules/compiler_builtins.m
  clang/test/Modules/stddef.c
  clang/test/Modules/stddef.m

Index: clang/test/Modules/stddef.m
===
--- clang/test/Modules/stddef.m
+++ clang/test/Modules/stddef.m
@@ -4,4 +4,6 @@
 
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,13 +1,29 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
 
 #include "ptrdiff_t.h"
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
-// expected-note@__stddef_size_t.h:* {{here}}
+// size_t is declared in both size_t.h and __stddef_size_t.h, both of which are
+// modular headers. Regardless of whether stddef.h joins the StdDef test module
+// or is in its _Builtin_stddef module, __stddef_size_t.h will be in
+// _Builtin_stddef.size_t. It's not defined which module will win as the expected
+// provider of size_t. For the purposes of this test it doesn't matter which header
+// gets reported, just as long as it isn't other.h or include_again.h.
+size_t st; // expected-error-re {{missing '#include "{{size_t|__stddef_size_t}}.h"'; 'size_t' must be declared before it is used}}
+// expected-note@size_t.h:* 0+ {{here}}
+// expected-note@__stddef_size_t.h:* 0+ {{here}}
 
 #include "include_again.h"
+// Includes  which includes <__stddef_size_t.h> which imports the
+// _Builtin_stddef.size_t module.
 
 size_t st2;
+
+#include "size_t.h"
+// Redeclares size_t, but the type merger should figure it out.
+
+size_t st3;
Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
 // expected-no-diagnostics
 
 #ifdef __SSE__
@@ -11,3 +11,19 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+#ifndef NO_SYSTEM_MODULES
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;
+@import _Builtin_stdbool;

[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/test/Modules/Inputs/System/usr/include/stdint.h:2
 typedef int my_awesome_nonstandard_integer_type;
+
+/* C99 7.18.1.1 Exact-width integer types.

Bigcheese wrote:
> iana wrote:
> > dexonsmith wrote:
> > > benlangmuir wrote:
> > > > iana wrote:
> > > > > iana wrote:
> > > > > > benlangmuir wrote:
> > > > > > > Why do we need all this code now (I assume this is copied from 
> > > > > > > the real header)?
> > > > > > 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?
> > > > > (and yes I just copied it from the builtin header)
> > > > 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
> > > 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).
> > 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.
> Looking at stdatomic.h I think this approach would be fine. It just wants the 
> types to exist.
SGTM.


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

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

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added a comment.

This change looks good with the stdint.h comments resolved.




Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2
 typedef int my_awesome_nonstandard_integer_type;
+
+/* C99 7.18.1.1 Exact-width integer types.

iana wrote:
> dexonsmith wrote:
> > benlangmuir wrote:
> > > iana wrote:
> > > > iana wrote:
> > > > > benlangmuir wrote:
> > > > > > Why do we need all this code now (I assume this is copied from the 
> > > > > > real header)?
> > > > > 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?
> > > > (and yes I just copied it from the builtin header)
> > > 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
> > 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).
> 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.
Looking at stdatomic.h I think this approach would be fine. It just wants the 
types to exist.


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

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

[Ian Anderson via Phabricator via cfe-commits]

iana added a comment.

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.


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

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

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

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.


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

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

[Ian Anderson via Phabricator via cfe-commits]

iana added inline comments.



Comment at: clang/lib/Headers/module.modulemap:269
+  explicit module wint_t {
+header "__stddef_wint_t.h"
+export *

iana wrote:
> 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?
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.


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

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 556630.
iana added a comment.

Actually allow the stdarg module


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_max_align_t.h
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/__stddef_offsetof.h
  clang/lib/Headers/__stddef_ptrdiff_t.h
  clang/lib/Headers/__stddef_rsize_t.h
  clang/lib/Headers/__stddef_size_t.h
  clang/lib/Headers/__stddef_unreachable.h
  clang/lib/Headers/__stddef_wchar_t.h
  clang/lib/Headers/__stddef_wint_t.h
  clang/lib/Headers/module.modulemap
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stddef.h
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/Inputs/System/usr/include/complex.h
  clang/test/Modules/Inputs/System/usr/include/inttypes.h
  clang/test/Modules/Inputs/System/usr/include/math.h
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/Modules/Inputs/System/usr/include/stdint.h
  clang/test/Modules/compiler_builtins.m
  clang/test/Modules/stddef.c
  clang/test/Modules/stddef.m

Index: clang/test/Modules/stddef.m
===
--- clang/test/Modules/stddef.m
+++ clang/test/Modules/stddef.m
@@ -4,4 +4,6 @@
 
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,12 +1,22 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=no-stddef-module -fno-modules-error-recovery
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=yes-stddef-module -fno-modules-error-recovery
 
 #include "ptrdiff_t.h"
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
-// expected-note@__stddef_size_t.h:* {{here}}
+// size_t is declared in both size_t.h and __stddef_size_t.h, both of which are
+// modular headers. Regardless of whether stddef.h joins the StdDef test module
+// or is in its _Builtin_stddef module, __stddef_size_t.h will be in
+// _Builtin_stddef.size_t. It's not defined which module will win as the expected
+// provider of size_t. For the purposes of this test it doesn't matter which header
+// gets reported, just as long as it isn't other.h or include_again.h.
+size_t st; // no-stddef-module-error {{missing '#include "size_t.h"'; 'size_t' must be declared before it is used}} \
+  yes-stddef-module-error {{missing '#include "size_t.h"'; 'size_t' must be declared before it is used}}
+// no-stddef-module-note@size_t.h:* {{here}}
+// yes-stddef-module-note@size_t.h:* {{here}}
 
 #include "include_again.h"
 
Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
 // expected-no-diagnostics
 
 #ifdef __SSE__
@@ -11,3 +11,19 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+#ifndef NO_SYSTEM_MODULES
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import 

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 556629.
iana added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_max_align_t.h
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/__stddef_offsetof.h
  clang/lib/Headers/__stddef_ptrdiff_t.h
  clang/lib/Headers/__stddef_rsize_t.h
  clang/lib/Headers/__stddef_size_t.h
  clang/lib/Headers/__stddef_unreachable.h
  clang/lib/Headers/__stddef_wchar_t.h
  clang/lib/Headers/__stddef_wint_t.h
  clang/lib/Headers/module.modulemap
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stddef.h
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/Inputs/System/usr/include/complex.h
  clang/test/Modules/Inputs/System/usr/include/inttypes.h
  clang/test/Modules/Inputs/System/usr/include/math.h
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/Modules/Inputs/System/usr/include/stdint.h
  clang/test/Modules/compiler_builtins.m
  clang/test/Modules/stddef.c
  clang/test/Modules/stddef.m

Index: clang/test/Modules/stddef.m
===
--- clang/test/Modules/stddef.m
+++ clang/test/Modules/stddef.m
@@ -4,4 +4,6 @@
 
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,12 +1,22 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=no-stddef-module -fno-modules-error-recovery
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=yes-stddef-module -fno-modules-error-recovery
 
 #include "ptrdiff_t.h"
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
-// expected-note@__stddef_size_t.h:* {{here}}
+// size_t is declared in both size_t.h and __stddef_size_t.h, both of which are
+// modular headers. Regardless of whether stddef.h joins the StdDef test module
+// or is in its _Builtin_stddef module, __stddef_size_t.h will be in
+// _Builtin_stddef.size_t. It's not defined which module will win as the expected
+// provider of size_t. For the purposes of this test it doesn't matter which header
+// gets reported, just as long as it isn't other.h or include_again.h.
+size_t st; // no-stddef-module-error {{missing '#include "size_t.h"'; 'size_t' must be declared before it is used}} \
+  yes-stddef-module-error {{missing '#include "size_t.h"'; 'size_t' must be declared before it is used}}
+// no-stddef-module-note@size_t.h:* {{here}}
+// yes-stddef-module-note@size_t.h:* {{here}}
 
 #include "include_again.h"
 
Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
 // 

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 556356.
iana added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/module.modulemap
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/Inputs/System/usr/include/complex.h
  clang/test/Modules/Inputs/System/usr/include/inttypes.h
  clang/test/Modules/Inputs/System/usr/include/math.h
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/Modules/Inputs/System/usr/include/stdint.h
  clang/test/Modules/compiler_builtins.m
  clang/test/Modules/stddef.c
  clang/test/Modules/stddef.m

Index: clang/test/Modules/stddef.m
===
--- clang/test/Modules/stddef.m
+++ clang/test/Modules/stddef.m
@@ -3,5 +3,7 @@
 size_t getSize(void);
 
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/StdDef %s
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,12 +1,24 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -fno-modules-error-recovery
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -fno-modules-error-recovery
 
 #include "ptrdiff_t.h"
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
-// expected-note@__stddef_size_t.h:* {{here}}
+// When builtin headers join system modules, stddef.h and its sub-headers have no
+// header guards, and so are seen last by include_again.h, which takes all of their
+// declarations including size_t even though size_t.h previously declared it.
+// When builtin headers don't join the system modules and instead get their own
+// modules, none of the stddef.h declarations go in the StdDef test module. size_t
+// is then declared in both StdDef.SizeT and _Builtin_stddef.size_t. For the
+// purposes of this test it doesn't matter which one gets reported, just as long
+// as it isn't other.h or include_again.h.
+size_t st; // no-stddef-module-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}} \
+  yes-stddef-module-error {{missing '#include "size_t.h"'; 'size_t' must be declared before it is used}}
+// no-stddef-module-note@__stddef_size_t.h:* {{here}}
+// yes-stddef-module-note@size_t.h:* {{here}}
 
 #include "include_again.h"
 
Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
 // expected-no-diagnostics
 
 #ifdef __SSE__
@@ -11,3 +11,19 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+#ifndef NO_SYSTEM_MODULES
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 556355.
iana added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/module.modulemap
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/Inputs/System/usr/include/complex.h
  clang/test/Modules/Inputs/System/usr/include/inttypes.h
  clang/test/Modules/Inputs/System/usr/include/math.h
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/Modules/Inputs/System/usr/include/stdint.h
  clang/test/Modules/compiler_builtins.m
  clang/test/Modules/stddef.c
  clang/test/Modules/stddef.m

Index: clang/test/Modules/stddef.m
===
--- clang/test/Modules/stddef.m
+++ clang/test/Modules/stddef.m
@@ -4,4 +4,6 @@
 
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/StdDef %s
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,12 +1,24 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=no-stddef-module -fno-modules-error-recovery
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -fno-modules-error-recovery
 
 #include "ptrdiff_t.h"
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
-// expected-note@__stddef_size_t.h:* {{here}}
+// When builtin headers join system modules, stddef.h and its sub-headers have no
+// header guards, and so are seen last by include_again.h, which takes all of their
+// declarations including size_t even though size_t.h previously declared it.
+// When builtin headers don't join the system modules and instead get their own
+// modules, none of the stddef.h declarations go in the StdDef test module. size_t
+// is then declared in both StdDef.SizeT and _Builtin_stddef.size_t. For the
+// purposes of this test it doesn't matter which one gets reported, just as long
+// as it isn't other.h or include_again.h.
+size_t st; // no-stddef-module-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}} \
+  yes-stddef-module-error {{missing '#include "size_t.h"'; 'size_t' must be declared before it is used}}
+// no-stddef-module-note@__stddef_size_t.h:* {{here}}
+// yes-stddef-module-note@size_t.h:* {{here}}
 
 #include "include_again.h"
 
Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
 // expected-no-diagnostics
 
 #ifdef __SSE__
@@ -11,3 +11,19 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+#ifndef NO_SYSTEM_MODULES
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;
+@import _Builtin_stdbool;
+@import _Builtin_stddef;
+@import _Builtin_stdint;
+@import _Builtin_stdnoreturn;
+@import _Builtin_tgmath;
+@import 

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 556313.
iana added a comment.

Add the header guard comment to __stddef_null.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/module.modulemap
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/Inputs/System/usr/include/complex.h
  clang/test/Modules/Inputs/System/usr/include/inttypes.h
  clang/test/Modules/Inputs/System/usr/include/math.h
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/Modules/Inputs/System/usr/include/stdint.h
  clang/test/Modules/compiler_builtins.m
  clang/test/Modules/stddef.c
  clang/test/Modules/stddef.m

Index: clang/test/Modules/stddef.m
===
--- clang/test/Modules/stddef.m
+++ clang/test/Modules/stddef.m
@@ -4,4 +4,6 @@
 
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,12 +1,24 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=no-stddef-module -fno-modules-error-recovery
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=yes-stddef-module -fno-modules-error-recovery
 
 #include "ptrdiff_t.h"
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
-// expected-note@__stddef_size_t.h:* {{here}}
+// When builtin headers join system modules, stddef.h and its sub-headers have no
+// header guards, and so are seen last by include_again.h, which takes all of their
+// declarations including size_t even though size_t.h previously declared it.
+// When builtin headers don't join the system modules and instead get their own
+// modules, none of the stddef.h declarations go in the StdDef test module. size_t
+// is then declared in both StdDef.SizeT and _Builtin_stddef.size_t. For the
+// purposes of this test it doesn't matter which one gets reported, just as long
+// as it isn't other.h or include_again.h.
+size_t st; // no-stddef-module-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}} \
+  yes-stddef-module-error {{missing '#include "size_t.h"'; 'size_t' must be declared before it is used}}
+// no-stddef-module-note@__stddef_size_t.h:* {{here}}
+// yes-stddef-module-note@size_t.h:* {{here}}
 
 #include "include_again.h"
 
Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
 // expected-no-diagnostics
 
 #ifdef __SSE__
@@ -11,3 +11,19 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+#ifndef NO_SYSTEM_MODULES
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;
+@import _Builtin_stdbool;
+@import _Builtin_stddef;
+@import _Builtin_stdint;
+@import _Builtin_stdnoreturn;
+@import 

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 556311.
iana added a comment.

Try deleting the module cache to fix the failing tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/module.modulemap
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/Inputs/System/usr/include/complex.h
  clang/test/Modules/Inputs/System/usr/include/inttypes.h
  clang/test/Modules/Inputs/System/usr/include/math.h
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/Modules/Inputs/System/usr/include/stdint.h
  clang/test/Modules/compiler_builtins.m
  clang/test/Modules/stddef.c
  clang/test/Modules/stddef.m

Index: clang/test/Modules/stddef.m
===
--- clang/test/Modules/stddef.m
+++ clang/test/Modules/stddef.m
@@ -4,4 +4,6 @@
 
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,12 +1,24 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=no-stddef-module -fno-modules-error-recovery
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=yes-stddef-module -fno-modules-error-recovery
 
 #include "ptrdiff_t.h"
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
-// expected-note@__stddef_size_t.h:* {{here}}
+// When builtin headers join system modules, stddef.h and its sub-headers have no
+// header guards, and so are seen last by include_again.h, which takes all of their
+// declarations including size_t even though size_t.h previously declared it.
+// When builtin headers don't join the system modules and instead get their own
+// modules, none of the stddef.h declarations go in the StdDef test module. size_t
+// is then declared in both StdDef.SizeT and _Builtin_stddef.size_t. For the
+// purposes of this test it doesn't matter which one gets reported, just as long
+// as it isn't other.h or include_again.h.
+size_t st; // no-stddef-module-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}} \
+  yes-stddef-module-error {{missing '#include "size_t.h"'; 'size_t' must be declared before it is used}}
+// no-stddef-module-note@__stddef_size_t.h:* {{here}}
+// yes-stddef-module-note@size_t.h:* {{here}}
 
 #include "include_again.h"
 
Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
 // expected-no-diagnostics
 
 #ifdef __SSE__
@@ -11,3 +11,19 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+#ifndef NO_SYSTEM_MODULES
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;
+@import _Builtin_stdbool;
+@import _Builtin_stddef;
+@import _Builtin_stdint;
+@import _Builtin_stdnoreturn;
+@import 

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

[Ian Anderson via Phabricator via cfe-commits]

iana added inline comments.



Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2
 typedef int my_awesome_nonstandard_integer_type;
+
+/* C99 7.18.1.1 Exact-width integer types.

dexonsmith wrote:
> benlangmuir wrote:
> > iana wrote:
> > > iana wrote:
> > > > benlangmuir wrote:
> > > > > Why do we need all this code now (I assume this is copied from the 
> > > > > real header)?
> > > > 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?
> > > (and yes I just copied it from the builtin header)
> > 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
> 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).
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.


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

[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/test/Modules/Inputs/System/usr/include/stdint.h:2
 typedef int my_awesome_nonstandard_integer_type;
+
+/* C99 7.18.1.1 Exact-width integer types.

benlangmuir wrote:
> iana wrote:
> > iana wrote:
> > > benlangmuir wrote:
> > > > Why do we need all this code now (I assume this is copied from the real 
> > > > header)?
> > > 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?
> > (and yes I just copied it from the builtin header)
> 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
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).


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

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

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added a comment.

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




Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2
 typedef int my_awesome_nonstandard_integer_type;
+
+/* C99 7.18.1.1 Exact-width integer types.

iana wrote:
> iana wrote:
> > benlangmuir wrote:
> > > Why do we need all this code now (I assume this is copied from the real 
> > > header)?
> > 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?
> (and yes I just copied it from the builtin header)
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


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

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

[Ian Anderson via Phabricator via cfe-commits]

iana added inline comments.



Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2
 typedef int my_awesome_nonstandard_integer_type;
+
+/* C99 7.18.1.1 Exact-width integer types.

iana wrote:
> benlangmuir wrote:
> > Why do we need all this code now (I assume this is copied from the real 
> > header)?
> 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?
(and yes I just copied it from the builtin header)


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

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

[Ian Anderson via Phabricator via cfe-commits]

iana added inline comments.



Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2
 typedef int my_awesome_nonstandard_integer_type;
+
+/* C99 7.18.1.1 Exact-width integer types.

benlangmuir wrote:
> Why do we need all this code now (I assume this is copied from the real 
> header)?
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?


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

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

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added inline comments.



Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2
 typedef int my_awesome_nonstandard_integer_type;
+
+/* C99 7.18.1.1 Exact-width integer types.

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


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

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

[Louis Dionne via Phabricator via cfe-commits]

ldionne resigned from this revision.
ldionne added a comment.

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


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

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 556280.
iana added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/module.modulemap
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/Inputs/System/usr/include/complex.h
  clang/test/Modules/Inputs/System/usr/include/inttypes.h
  clang/test/Modules/Inputs/System/usr/include/math.h
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/Modules/Inputs/System/usr/include/stdint.h
  clang/test/Modules/compiler_builtins.m
  clang/test/Modules/stddef.c
  clang/test/Modules/stddef.m

Index: clang/test/Modules/stddef.m
===
--- clang/test/Modules/stddef.m
+++ clang/test/Modules/stddef.m
@@ -4,4 +4,5 @@
 
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,12 +1,23 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=no-stddef-module -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=yes-stddef-module -fno-modules-error-recovery
 
 #include "ptrdiff_t.h"
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
-// expected-note@__stddef_size_t.h:* {{here}}
+// When builtin headers join system modules, stddef.h and its sub-headers have no
+// header guards, and so are seen last by include_again.h, which takes all of their
+// declarations including size_t even though size_t.h previously declared it.
+// When builtin headers don't join the system modules and instead get their own
+// modules, none of the stddef.h declarations go in the StdDef test module. size_t
+// is then declared in both StdDef.SizeT and _Builtin_stddef.size_t. For the
+// purposes of this test it doesn't matter which one gets reported, just as long
+// as it isn't other.h or include_again.h.
+size_t st; // no-stddef-module-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}} \
+  yes-stddef-module-error {{missing '#include "size_t.h"'; 'size_t' must be declared before it is used}}
+// no-stddef-module-note@__stddef_size_t.h:* {{here}}
+// yes-stddef-module-note@size_t.h:* {{here}}
 
 #include "include_again.h"
 
Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
 // expected-no-diagnostics
 
 #ifdef __SSE__
@@ -11,3 +11,19 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+#ifndef NO_SYSTEM_MODULES
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;
+@import _Builtin_stdbool;
+@import _Builtin_stddef;
+@import _Builtin_stdint;
+@import _Builtin_stdnoreturn;
+@import _Builtin_tgmath;
+@import _Builtin_unwind;
+#endif
Index: 

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 556231.
iana added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/module.modulemap
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/Inputs/System/usr/include/complex.h
  clang/test/Modules/Inputs/System/usr/include/inttypes.h
  clang/test/Modules/Inputs/System/usr/include/math.h
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/Modules/Inputs/System/usr/include/stdint.h
  clang/test/Modules/compiler_builtins.m
  clang/test/Modules/stddef.c
  clang/test/Modules/stddef.m

Index: clang/test/Modules/stddef.m
===
--- clang/test/Modules/stddef.m
+++ clang/test/Modules/stddef.m
@@ -4,4 +4,5 @@
 
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,12 +1,23 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=no-stddef-module -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=yes-stddef-module -fno-modules-error-recovery
 
 #include "ptrdiff_t.h"
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
-// expected-note@__stddef_size_t.h:* {{here}}
+// When builtin headers join system modules, stddef.h and its sub-headers have no
+// header guards, and so are seen last by include_again.h, which takes all of their
+// declarations including size_t even though size_t.h previously declared it.
+// When builtin headers don't join the system modules and instead get their own
+// modules, none of the stddef.h declarations go in the StdDef test module. size_t
+// is then declared in both StdDef.SizeT and _Builtin_stddef.size_t. For the
+// purposes of this test it doesn't matter which one gets reported, just as long
+// as it isn't other.h or include_again.h.
+size_t st; // no-stddef-module-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}} \
+  yes-stddef-module-error {{missing '#include "size_t.h"'; 'size_t' must be declared before it is used}}
+// no-stddef-module-note@__stddef_size_t.h:* {{here}}
+// yes-stddef-module-note@size_t.h:* {{here}}
 
 #include "include_again.h"
 
Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
 // expected-no-diagnostics
 
 #ifdef __SSE__
@@ -11,3 +11,19 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+#ifndef NO_SYSTEM_MODULES
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;
+@import _Builtin_stdbool;
+@import _Builtin_stddef;
+@import _Builtin_stdint;
+@import _Builtin_stdnoreturn;
+@import _Builtin_tgmath;
+@import _Builtin_unwind;
+#endif
Index: 

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 556208.
iana added a comment.

Fix formatting issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/module.modulemap
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/Inputs/System/usr/include/complex.h
  clang/test/Modules/Inputs/System/usr/include/inttypes.h
  clang/test/Modules/Inputs/System/usr/include/math.h
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/Modules/Inputs/System/usr/include/stdint.h
  clang/test/Modules/compiler_builtins.m
  clang/test/Modules/stddef.c
  clang/test/Modules/stddef.m

Index: clang/test/Modules/stddef.m
===
--- clang/test/Modules/stddef.m
+++ clang/test/Modules/stddef.m
@@ -4,4 +4,5 @@
 
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,12 +1,23 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=no-stddef-module -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=yes-stddef-module -fno-modules-error-recovery
 
 #include "ptrdiff_t.h"
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
-// expected-note@__stddef_size_t.h:* {{here}}
+// When builtin headers join system modules, stddef.h and its sub-headers have no
+// header guards, and so are seen last by include_again.h, which takes all of their
+// declarations including size_t even though size_t.h previously declared it.
+// When builtin headers don't join the system modules and instead get their own
+// modules, none of the stddef.h declarations go in the StdDef test module. size_t
+// is then declared in both StdDef.SizeT and _Builtin_stddef.size_t. For the
+// purposes of this test it doesn't matter which one gets reported, just as long
+// as it isn't other.h or include_again.h.
+size_t st; // no-stddef-module-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}} \
+  yes-stddef-module-error {{missing '#include "size_t.h"'; 'size_t' must be declared before it is used}}
+// no-stddef-module-note@__stddef_size_t.h:* {{here}}
+// yes-stddef-module-note@size_t.h:* {{here}}
 
 #include "include_again.h"
 
Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
 // expected-no-diagnostics
 
 #ifdef __SSE__
@@ -11,3 +11,19 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+#ifndef NO_SYSTEM_MODULES
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;
+@import _Builtin_stdbool;
+@import _Builtin_stddef;
+@import _Builtin_stdint;
+@import _Builtin_stdnoreturn;
+@import _Builtin_tgmath;
+@import _Builtin_unwind;
+#endif
Index: 

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

[Ian Anderson via Phabricator via cfe-commits]

iana marked 2 inline comments as done.
iana added inline comments.



Comment at: clang/lib/Headers/module.modulemap:269
+  explicit module wint_t {
+header "__stddef_wint_t.h"
+export *

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?


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

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 556192.
iana added a comment.

Rebase on top of D159483 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/module.modulemap
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/Inputs/System/usr/include/complex.h
  clang/test/Modules/Inputs/System/usr/include/inttypes.h
  clang/test/Modules/Inputs/System/usr/include/math.h
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/Modules/Inputs/System/usr/include/stdint.h
  clang/test/Modules/compiler_builtins.m
  clang/test/Modules/stddef.c
  clang/test/Modules/stddef.m

Index: clang/test/Modules/stddef.m
===
--- clang/test/Modules/stddef.m
+++ clang/test/Modules/stddef.m
@@ -4,4 +4,5 @@
 
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/StdDef %s -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,12 +1,23 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=no-stddef-module -fno-modules-error-recovery
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=yes-stddef-module -fno-modules-error-recovery
 
 #include "ptrdiff_t.h"
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
-// expected-note@__stddef_size_t.h:* {{here}}
+// When builtin headers join system modules, stddef.h and its sub-headers have no
+// header guards, and so are seen last by include_again.h, which takes all of their
+// declarations including size_t even though size_t.h previously declared it.
+// When builtin headers don't join the system modules and instead get their own
+// modules, none of the stddef.h declarations go in the StdDef test module. size_t
+// is then declared in both StdDef.SizeT and _Builtin_stddef.size_t. For the
+// purposes of this test it doesn't matter which one gets reported, just as long
+// as it isn't other.h or include_again.h.
+size_t st; // no-stddef-module-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}} \
+  yes-stddef-module-error {{missing '#include "size_t.h"'; 'size_t' must be declared before it is used}}
+// no-stddef-module-note@__stddef_size_t.h:* {{here}}
+// yes-stddef-module-note@size_t.h:* {{here}}
 
 #include "include_again.h"
 
Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
 // expected-no-diagnostics
 
 #ifdef __SSE__
@@ -11,3 +11,19 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+#ifndef NO_SYSTEM_MODULES
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;
+@import _Builtin_stdbool;
+@import _Builtin_stddef;
+@import _Builtin_stdint;
+@import _Builtin_stdnoreturn;
+@import _Builtin_tgmath;
+@import 

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

[Ian Anderson via Phabricator via cfe-commits]

iana added a comment.

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.


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

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 554882.
iana added a comment.

Update Module::directlyUses


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Basic/Module.cpp
  clang/lib/Headers/__stddef_max_align_t.h
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/__stddef_offsetof.h
  clang/lib/Headers/__stddef_ptrdiff_t.h
  clang/lib/Headers/__stddef_rsize_t.h
  clang/lib/Headers/__stddef_size_t.h
  clang/lib/Headers/__stddef_unreachable.h
  clang/lib/Headers/__stddef_wchar_t.h
  clang/lib/Headers/__stddef_wint_t.h
  clang/lib/Headers/module.modulemap
  clang/lib/Headers/stddef.h
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/compiler_builtins.m

Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -11,3 +11,17 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;
+@import _Builtin_stdbool;
+@import _Builtin_stddef;
+@import _Builtin_stdint;
+@import _Builtin_stdnoreturn;
+@import _Builtin_tgmath;
+@import _Builtin_unwind;
Index: clang/test/Headers/stddefneeds.c
===
--- clang/test/Headers/stddefneeds.c
+++ clang/test/Headers/stddefneeds.c
@@ -1,36 +1,73 @@
+// RUN: rm -fR %t
 // RUN: %clang_cc1 -fsyntax-only -verify=c99 -std=c99 %s
 // RUN: %clang_cc1 -fsyntax-only -verify=c23 -std=c23 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c99-modules -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c23-modules -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -std=c23 %s
 
 // Use C99 to verify that __need_ can be used to get types that wouldn't normally be available.
 
 struct astruct { char member; };
 
-ptrdiff_t p0; // c99-error{{unknown type name 'ptrdiff_t'}} c23-error{{unknown type}}
-size_t s0; // c99-error{{unknown type name 'size_t'}} c23-error{{unknown type}}
-rsize_t r0; // c99-error{{unknown type name 'rsize_t'}} c23-error{{unknown type}}
-wchar_t wc0; // c99-error{{unknown type name 'wchar_t'}} c23-error{{unknown type}}
-void *v0 = NULL; // c99-error{{use of undeclared identifier 'NULL'}} c23-error{{undeclared identifier}}
-nullptr_t n0; // c99-error{{unknown type name 'nullptr_t'}} c23-error{{unknown type}}
-static void f0(void) { unreachable(); } // c99-error{{call to undeclared function 'unreachable'}} c23-error{{undeclared identifier 'unreachable'}}
-max_align_t m0; // c99-error{{unknown type name 'max_align_t'}} c23-error{{unknown type}}
+ptrdiff_t p0; // c99-error{{unknown type name 'ptrdiff_t'}} c23-error{{unknown type}} \
+ c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+size_t s0; // c99-error{{unknown type name 'size_t'}} c23-error{{unknown type}} \
+  c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+rsize_t r0; // c99-error{{unknown type name 'rsize_t'}} c23-error{{unknown type}} \
+   c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+wchar_t wc0; // c99-error{{unknown type name 'wchar_t'}} c23-error{{unknown type}} \
+c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+void *v0 = NULL; // c99-error{{use of undeclared identifier 'NULL'}} c23-error{{undeclared identifier}} \
+c99-modules-error{{undeclared identifier}} c23-modules-error{{undeclared identifier}}
+nullptr_t n0; // c99-error{{unknown type name 'nullptr_t'}} c23-error{{unknown type}} \
+ c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+static void f0(void) { unreachable(); } // c99-error{{call to undeclared function 'unreachable'}} c23-error{{undeclared identifier 'unreachable'}} \
+   c99-modules-error{{undeclared function}} c23-modules-error{{undeclared identifier}}
+max_align_t m0; // c99-error{{unknown type name 'max_align_t'}} c23-error{{unknown type}} \
+   c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
 size_t o0 = offsetof(struct astruct, member); // c99-error{{unknown type name 'size_t'}} c99-error{{call to undeclared function 'offsetof'}} c99-error{{expected expression}} c99-error{{use of undeclared identifier 'member'}} \
- c23-error{{unknown type name 'size_t'}} c23-error{{undeclared identifier 'offsetof'}} c23-error{{expected expression}} c23-error{{use of undeclared identifier 

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 554832.
iana marked an inline comment as done.
iana added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Headers/__stddef_max_align_t.h
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/__stddef_offsetof.h
  clang/lib/Headers/__stddef_ptrdiff_t.h
  clang/lib/Headers/__stddef_rsize_t.h
  clang/lib/Headers/__stddef_size_t.h
  clang/lib/Headers/__stddef_unreachable.h
  clang/lib/Headers/__stddef_wchar_t.h
  clang/lib/Headers/__stddef_wint_t.h
  clang/lib/Headers/module.modulemap
  clang/lib/Headers/stddef.h
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/compiler_builtins.m

Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -11,3 +11,17 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;
+@import _Builtin_stdbool;
+@import _Builtin_stddef;
+@import _Builtin_stdint;
+@import _Builtin_stdnoreturn;
+@import _Builtin_tgmath;
+@import _Builtin_unwind;
Index: clang/test/Headers/stddefneeds.c
===
--- clang/test/Headers/stddefneeds.c
+++ clang/test/Headers/stddefneeds.c
@@ -1,36 +1,73 @@
+// RUN: rm -fR %t
 // RUN: %clang_cc1 -fsyntax-only -verify=c99 -std=c99 %s
 // RUN: %clang_cc1 -fsyntax-only -verify=c23 -std=c23 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c99-modules -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c23-modules -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -std=c23 %s
 
 // Use C99 to verify that __need_ can be used to get types that wouldn't normally be available.
 
 struct astruct { char member; };
 
-ptrdiff_t p0; // c99-error{{unknown type name 'ptrdiff_t'}} c23-error{{unknown type}}
-size_t s0; // c99-error{{unknown type name 'size_t'}} c23-error{{unknown type}}
-rsize_t r0; // c99-error{{unknown type name 'rsize_t'}} c23-error{{unknown type}}
-wchar_t wc0; // c99-error{{unknown type name 'wchar_t'}} c23-error{{unknown type}}
-void *v0 = NULL; // c99-error{{use of undeclared identifier 'NULL'}} c23-error{{undeclared identifier}}
-nullptr_t n0; // c99-error{{unknown type name 'nullptr_t'}} c23-error{{unknown type}}
-static void f0(void) { unreachable(); } // c99-error{{call to undeclared function 'unreachable'}} c23-error{{undeclared identifier 'unreachable'}}
-max_align_t m0; // c99-error{{unknown type name 'max_align_t'}} c23-error{{unknown type}}
+ptrdiff_t p0; // c99-error{{unknown type name 'ptrdiff_t'}} c23-error{{unknown type}} \
+ c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+size_t s0; // c99-error{{unknown type name 'size_t'}} c23-error{{unknown type}} \
+  c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+rsize_t r0; // c99-error{{unknown type name 'rsize_t'}} c23-error{{unknown type}} \
+   c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+wchar_t wc0; // c99-error{{unknown type name 'wchar_t'}} c23-error{{unknown type}} \
+c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+void *v0 = NULL; // c99-error{{use of undeclared identifier 'NULL'}} c23-error{{undeclared identifier}} \
+c99-modules-error{{undeclared identifier}} c23-modules-error{{undeclared identifier}}
+nullptr_t n0; // c99-error{{unknown type name 'nullptr_t'}} c23-error{{unknown type}} \
+ c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+static void f0(void) { unreachable(); } // c99-error{{call to undeclared function 'unreachable'}} c23-error{{undeclared identifier 'unreachable'}} \
+   c99-modules-error{{undeclared function}} c23-modules-error{{undeclared identifier}}
+max_align_t m0; // c99-error{{unknown type name 'max_align_t'}} c23-error{{unknown type}} \
+   c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
 size_t o0 = offsetof(struct astruct, member); // c99-error{{unknown type name 'size_t'}} c99-error{{call to undeclared function 'offsetof'}} c99-error{{expected expression}} c99-error{{use of undeclared identifier 'member'}} \
- 

[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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ian Anderson via Phabricator via cfe-commits]

iana marked an inline comment as done.
iana 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.

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.



Comment at: clang/lib/Headers/module.modulemap:156
 
-module _Builtin_stddef_max_align_t [system] [extern_c] {
-  header "__stddef_max_align_t.h"

benlangmuir wrote:
> 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?
Yes that is the assumption, but if someone knows different please let me know!


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

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

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added inline comments.



Comment at: clang/lib/Headers/module.modulemap:156
 
-module _Builtin_stddef_max_align_t [system] [extern_c] {
-  header "__stddef_max_align_t.h"

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?


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

[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:
> 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.


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

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

[Ian Anderson via Phabricator via cfe-commits]

iana 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.

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.



Comment at: clang/test/Headers/stdarg.c:4
 // RUN: %clang_cc1 -fsyntax-only -verify=c99 
-Werror=implicit-function-declaration -std=c99 %t/stdarg0.c
+// RUN: %clang_cc1 -fsyntax-only -verify=c89-modules -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t 
-Werror=implicit-function-declaration -std=c89 %t/stdarg0.c
+// RUN: %clang_cc1 -fsyntax-only -verify=c99-modules -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t 
-Werror=implicit-function-declaration -std=c99 %t/stdarg0.c

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?


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

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 554193.
iana added a comment.

Add `@import` tests for all of the new modules


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Headers/__stddef_max_align_t.h
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/__stddef_offsetof.h
  clang/lib/Headers/__stddef_ptrdiff_t.h
  clang/lib/Headers/__stddef_rsize_t.h
  clang/lib/Headers/__stddef_size_t.h
  clang/lib/Headers/__stddef_unreachable.h
  clang/lib/Headers/__stddef_wchar_t.h
  clang/lib/Headers/__stddef_wint_t.h
  clang/lib/Headers/module.modulemap
  clang/lib/Headers/stddef.h
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c
  clang/test/Modules/compiler_builtins.m

Index: clang/test/Modules/compiler_builtins.m
===
--- clang/test/Modules/compiler_builtins.m
+++ clang/test/Modules/compiler_builtins.m
@@ -11,3 +11,17 @@
 #ifdef __AVX2__
 @import _Builtin_intrinsics.intel.avx2;
 #endif
+
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;
+@import _Builtin_stdbool;
+@import _Builtin_stddef;
+@import _Builtin_stdint;
+@import _Builtin_stdnoreturn;
+@import _Builtin_tgmath;
+@import _Builtin_unwind;
Index: clang/test/Headers/stddefneeds.c
===
--- clang/test/Headers/stddefneeds.c
+++ clang/test/Headers/stddefneeds.c
@@ -1,36 +1,73 @@
+// RUN: rm -fR %t
 // RUN: %clang_cc1 -fsyntax-only -verify=c99 -std=c99 %s
 // RUN: %clang_cc1 -fsyntax-only -verify=c23 -std=c23 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c99-modules -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c23-modules -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -std=c23 %s
 
 // Use C99 to verify that __need_ can be used to get types that wouldn't normally be available.
 
 struct astruct { char member; };
 
-ptrdiff_t p0; // c99-error{{unknown type name 'ptrdiff_t'}} c23-error{{unknown type}}
-size_t s0; // c99-error{{unknown type name 'size_t'}} c23-error{{unknown type}}
-rsize_t r0; // c99-error{{unknown type name 'rsize_t'}} c23-error{{unknown type}}
-wchar_t wc0; // c99-error{{unknown type name 'wchar_t'}} c23-error{{unknown type}}
-void *v0 = NULL; // c99-error{{use of undeclared identifier 'NULL'}} c23-error{{undeclared identifier}}
-nullptr_t n0; // c99-error{{unknown type name 'nullptr_t'}} c23-error{{unknown type}}
-static void f0(void) { unreachable(); } // c99-error{{call to undeclared function 'unreachable'}} c23-error{{undeclared identifier 'unreachable'}}
-max_align_t m0; // c99-error{{unknown type name 'max_align_t'}} c23-error{{unknown type}}
+ptrdiff_t p0; // c99-error{{unknown type name 'ptrdiff_t'}} c23-error{{unknown type}} \
+ c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+size_t s0; // c99-error{{unknown type name 'size_t'}} c23-error{{unknown type}} \
+  c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+rsize_t r0; // c99-error{{unknown type name 'rsize_t'}} c23-error{{unknown type}} \
+   c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+wchar_t wc0; // c99-error{{unknown type name 'wchar_t'}} c23-error{{unknown type}} \
+c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+void *v0 = NULL; // c99-error{{use of undeclared identifier 'NULL'}} c23-error{{undeclared identifier}} \
+c99-modules-error{{undeclared identifier}} c23-modules-error{{undeclared identifier}}
+nullptr_t n0; // c99-error{{unknown type name 'nullptr_t'}} c23-error{{unknown type}} \
+ c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+static void f0(void) { unreachable(); } // c99-error{{call to undeclared function 'unreachable'}} c23-error{{undeclared identifier 'unreachable'}} \
+   c99-modules-error{{undeclared function}} c23-modules-error{{undeclared identifier}}
+max_align_t m0; // c99-error{{unknown type name 'max_align_t'}} c23-error{{unknown type}} \
+   c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
 size_t o0 = offsetof(struct astruct, member); // c99-error{{unknown type name 'size_t'}} c99-error{{call to undeclared function 'offsetof'}} c99-error{{expected expression}} c99-error{{use of undeclared identifier 'member'}} \
- c23-error{{unknown type name 'size_t'}} c23-error{{undeclared identifier 'offsetof'}} c23-error{{expected expression}} c23-error{{use of undeclared identifier 'member'}}

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

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 554183.
iana added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159064

Files:
  clang/lib/Headers/__stddef_max_align_t.h
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/__stddef_offsetof.h
  clang/lib/Headers/__stddef_ptrdiff_t.h
  clang/lib/Headers/__stddef_rsize_t.h
  clang/lib/Headers/__stddef_size_t.h
  clang/lib/Headers/__stddef_unreachable.h
  clang/lib/Headers/__stddef_wchar_t.h
  clang/lib/Headers/__stddef_wint_t.h
  clang/lib/Headers/module.modulemap
  clang/lib/Headers/stddef.h
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c

Index: clang/test/Headers/stddefneeds.c
===
--- clang/test/Headers/stddefneeds.c
+++ clang/test/Headers/stddefneeds.c
@@ -1,36 +1,73 @@
+// RUN: rm -fR %t
 // RUN: %clang_cc1 -fsyntax-only -verify=c99 -std=c99 %s
 // RUN: %clang_cc1 -fsyntax-only -verify=c23 -std=c23 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c99-modules -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c23-modules -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -std=c23 %s
 
 // Use C99 to verify that __need_ can be used to get types that wouldn't normally be available.
 
 struct astruct { char member; };
 
-ptrdiff_t p0; // c99-error{{unknown type name 'ptrdiff_t'}} c23-error{{unknown type}}
-size_t s0; // c99-error{{unknown type name 'size_t'}} c23-error{{unknown type}}
-rsize_t r0; // c99-error{{unknown type name 'rsize_t'}} c23-error{{unknown type}}
-wchar_t wc0; // c99-error{{unknown type name 'wchar_t'}} c23-error{{unknown type}}
-void *v0 = NULL; // c99-error{{use of undeclared identifier 'NULL'}} c23-error{{undeclared identifier}}
-nullptr_t n0; // c99-error{{unknown type name 'nullptr_t'}} c23-error{{unknown type}}
-static void f0(void) { unreachable(); } // c99-error{{call to undeclared function 'unreachable'}} c23-error{{undeclared identifier 'unreachable'}}
-max_align_t m0; // c99-error{{unknown type name 'max_align_t'}} c23-error{{unknown type}}
+ptrdiff_t p0; // c99-error{{unknown type name 'ptrdiff_t'}} c23-error{{unknown type}} \
+ c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+size_t s0; // c99-error{{unknown type name 'size_t'}} c23-error{{unknown type}} \
+  c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+rsize_t r0; // c99-error{{unknown type name 'rsize_t'}} c23-error{{unknown type}} \
+   c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+wchar_t wc0; // c99-error{{unknown type name 'wchar_t'}} c23-error{{unknown type}} \
+c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+void *v0 = NULL; // c99-error{{use of undeclared identifier 'NULL'}} c23-error{{undeclared identifier}} \
+c99-modules-error{{undeclared identifier}} c23-modules-error{{undeclared identifier}}
+nullptr_t n0; // c99-error{{unknown type name 'nullptr_t'}} c23-error{{unknown type}} \
+ c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+static void f0(void) { unreachable(); } // c99-error{{call to undeclared function 'unreachable'}} c23-error{{undeclared identifier 'unreachable'}} \
+   c99-modules-error{{undeclared function}} c23-modules-error{{undeclared identifier}}
+max_align_t m0; // c99-error{{unknown type name 'max_align_t'}} c23-error{{unknown type}} \
+   c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
 size_t o0 = offsetof(struct astruct, member); // c99-error{{unknown type name 'size_t'}} c99-error{{call to undeclared function 'offsetof'}} c99-error{{expected expression}} c99-error{{use of undeclared identifier 'member'}} \
- c23-error{{unknown type name 'size_t'}} c23-error{{undeclared identifier 'offsetof'}} c23-error{{expected expression}} c23-error{{use of undeclared identifier 'member'}}
-wint_t wi0; // c99-error{{unknown type name 'wint_t'}} c23-error{{unknown type}}
+ c23-error{{unknown type name 'size_t'}} c23-error{{undeclared identifier 'offsetof'}} c23-error{{expected expression}} c23-error{{use of undeclared identifier 'member'}} \
+ c99-modules-error{{unknown type}} c99-modules-error{{undeclared function}} c99-modules-error{{expected expression}} c99-modules-error{{undeclared identifier}} \
+ c23-modules-error{{unknown type}} c23-modules-error{{undeclared identifier}} 

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

[Ian Anderson via Phabricator via cfe-commits]

iana created this revision.
iana added reviewers: aaron.ballman, rsmith, efriedma, ldionne, ChuanqiXu, 
Bigcheese, vsapsai, benlangmuir, dexonsmith.
Herald added a subscriber: ributzka.
Herald added a project: All.
iana requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Make top level modules for all the C standard library headers. Now that the 
stddef headers are modular, they can lose all of the __has_feature(modules) 
checks and declare their include guards.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159064

Files:
  clang/lib/Headers/__stddef_max_align_t.h
  clang/lib/Headers/__stddef_null.h
  clang/lib/Headers/__stddef_nullptr_t.h
  clang/lib/Headers/__stddef_offsetof.h
  clang/lib/Headers/__stddef_ptrdiff_t.h
  clang/lib/Headers/__stddef_rsize_t.h
  clang/lib/Headers/__stddef_size_t.h
  clang/lib/Headers/__stddef_unreachable.h
  clang/lib/Headers/__stddef_wchar_t.h
  clang/lib/Headers/__stddef_wint_t.h
  clang/lib/Headers/module.modulemap
  clang/lib/Headers/stddef.h
  clang/test/Headers/stdarg.c
  clang/test/Headers/stdargneeds.c
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c

Index: clang/test/Headers/stddefneeds.c
===
--- clang/test/Headers/stddefneeds.c
+++ clang/test/Headers/stddefneeds.c
@@ -1,36 +1,72 @@
 // RUN: %clang_cc1 -fsyntax-only -verify=c99 -std=c99 %s
 // RUN: %clang_cc1 -fsyntax-only -verify=c23 -std=c23 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c99-modules -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c23-modules -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -std=c23 %s
 
 // Use C99 to verify that __need_ can be used to get types that wouldn't normally be available.
 
 struct astruct { char member; };
 
-ptrdiff_t p0; // c99-error{{unknown type name 'ptrdiff_t'}} c23-error{{unknown type}}
-size_t s0; // c99-error{{unknown type name 'size_t'}} c23-error{{unknown type}}
-rsize_t r0; // c99-error{{unknown type name 'rsize_t'}} c23-error{{unknown type}}
-wchar_t wc0; // c99-error{{unknown type name 'wchar_t'}} c23-error{{unknown type}}
-void *v0 = NULL; // c99-error{{use of undeclared identifier 'NULL'}} c23-error{{undeclared identifier}}
-nullptr_t n0; // c99-error{{unknown type name 'nullptr_t'}} c23-error{{unknown type}}
-static void f0(void) { unreachable(); } // c99-error{{call to undeclared function 'unreachable'}} c23-error{{undeclared identifier 'unreachable'}}
-max_align_t m0; // c99-error{{unknown type name 'max_align_t'}} c23-error{{unknown type}}
+ptrdiff_t p0; // c99-error{{unknown type name 'ptrdiff_t'}} c23-error{{unknown type}} \
+ c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+size_t s0; // c99-error{{unknown type name 'size_t'}} c23-error{{unknown type}} \
+  c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+rsize_t r0; // c99-error{{unknown type name 'rsize_t'}} c23-error{{unknown type}} \
+   c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+wchar_t wc0; // c99-error{{unknown type name 'wchar_t'}} c23-error{{unknown type}} \
+c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+void *v0 = NULL; // c99-error{{use of undeclared identifier 'NULL'}} c23-error{{undeclared identifier}} \
+c99-modules-error{{undeclared identifier}} c23-modules-error{{undeclared identifier}}
+nullptr_t n0; // c99-error{{unknown type name 'nullptr_t'}} c23-error{{unknown type}} \
+ c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
+static void f0(void) { unreachable(); } // c99-error{{call to undeclared function 'unreachable'}} c23-error{{undeclared identifier 'unreachable'}} \
+   c99-modules-error{{undeclared function}} c23-modules-error{{undeclared identifier}}
+max_align_t m0; // c99-error{{unknown type name 'max_align_t'}} c23-error{{unknown type}} \
+   c99-modules-error{{unknown type}} c23-modules-error{{unknown type}}
 size_t o0 = offsetof(struct astruct, member); // c99-error{{unknown type name 'size_t'}} c99-error{{call to undeclared function 'offsetof'}} c99-error{{expected expression}} c99-error{{use of undeclared identifier 'member'}} \
- c23-error{{unknown type name 'size_t'}} c23-error{{undeclared identifier 'offsetof'}} c23-error{{expected expression}} c23-error{{use of undeclared identifier 'member'}}
-wint_t wi0; // c99-error{{unknown type name 'wint_t'}} c23-error{{unknown type}}
+ c23-error{{unknown type name 'size_t'}} c23-error{{undeclared identifier 'offsetof'}} c23-error{{expected expression}} c23-error{{use of undeclared identifier 'member'}} \
+