[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Med Ismail Bennani via Phabricator via cfe-commits]

mib added a comment.

@iana I believe this is affecting lldb macOS bot: 
https://green.lab.llvm.org/green/view/LLDB/job/as-lldb-cmake/6316/

Can you take a look ? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0ea3d88bdb16: [Modules] Add a flag to control builtin 
headers being in system modules (authored by iana).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

Files:
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json
  clang/test/Driver/darwin-builtin-modules.c
  clang/test/Modules/Werror-Wsystem-headers.m
  clang/test/Modules/context-hash.c
  clang/test/Modules/crash-vfs-include-pch.m
  clang/test/Modules/cstd.m
  clang/test/Modules/pch-used.m
  clang/test/Modules/shadowed-submodule.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,5 @@
 size_t getSize(void);
 
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -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 -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,5 +1,5 @@
 // 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
+// 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
 
 #include "ptrdiff_t.h"
 
Index: clang/test/Modules/shadowed-submodule.m
===
--- clang/test/Modules/shadowed-submodule.m
+++ clang/test/Modules/shadowed-submodule.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
 
 @import Foo; // expected-error {{module 'A' was built in directory}}
  // expected-note@shadowed-submodule.m:4 {{imported by module 'Foo'}}
Index: clang/test/Modules/pch-used.m
===
--- clang/test/Modules/pch-used.m
+++ clang/test/Modules/pch-used.m
@@ -1,8 +1,8 @@
 // UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // RUN: rm -rf %t
 // RUN: mkdir %t
-// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
-// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
+// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm -o - | FileCheck %s
 
 void f(void) { SPXTrace(); }
 void g(void) { double x = DBL_MAX; }
Index: clang/test/Modules/cstd.m
===
--- clang/test/Modules/cstd.m
+++ clang/test/Modules/cstd.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -internal-isystem %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -D__need_wint_t -Werror=implicit-function-declaration %s
+// RUN: %clang_cc1 -fsyntax-only -internal-isystem %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -D__need_wint_t -Werror=implicit-function-declaration %s
 
 @import uses_other_constants;
 const double other_value = DBL_MAX;
Index: clang/test/Modules/crash-vfs-include-pch.m
===
--- clang/test/Modules/crash-vfs-include-pch.m
+++ clang/test/Modules/crash-vfs-include-pch.m
@@ -5,14 +5,14 @@
 
 // RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h \
 // RUN: -o 

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[David Blaikie via Phabricator via cfe-commits]

dblaikie added subscribers: sammccall, dblaikie.
dblaikie added a comment.

@sammccall @rsmith - figure if this does impact us, we'll use the flag to 
opt-out in the short term while we figure out longer-term solution, yeah? Is 
there any pre-emptive testing you think is worth doing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

The implementation of this change looks good. I share the concern that changing 
the default may break other users, but that's easily remedied and this is the 
correct default. It would be good to hear from at least the Google folks before 
landing though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/LangOptions.def:176
 COMPATIBLE_LANGOPT(CPlusPlusModules, 1, 0, "C++ modules syntax")
+LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin headers belong to system 
modules, and _Builtin_ modules are ignored for cstdlib headers")
 BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,

vsapsai wrote:
> iana wrote:
> > vsapsai wrote:
> > > Why not to make the default value `true` to preserve the old behavior and 
> > > switch on the new behavior in the driver? It's not a veiled way to force 
> > > you to change it, I'm genuinely curious and want to consider pro/cons of 
> > > various alternatives.
> > I did it this way because the current behavior is nonintuitive and just 
> > kind of bizarre. I had no idea that `Darwin.C.stdint` also included the 
> > compiler's stdint.h, and that the OS's stdint.h was treated textually. I 
> > thought it would be better for the default behavior to be for modules to 
> > behave consistently and obviously, not have secret special cases, and be 
> > usable with C++.
> > 
> > The con of course is that if anyone besides Apple depends on the current 
> > behavior they're going to either have to fix their OS modules or add this 
> > flag. I'm not sure if other platforms have a convenient driver class where 
> > they can do that.
> I'm fine with the more aggressive approach as long as we test Swift on Linux. 
> That's the only project I know that is willing to keep using clang modules. 
> For other projects if things break and it is a motivation to move to C++ 
> modules, it is a positive direction, as for me.
> 
> Another reason I wanted to be more cautious is because of 
> https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213
>  change. Clang started enforcing 20+ years old rule and that has broken some 
> stuff. So I don't see "you should've known better before relying on this 
> behavior" as a good argument.
> 
> Anyway, for Swift testing here is what you can do (if you have your own plan, 
> go ahead):
> 1. Make PR for the same change as this one at 
> https://github.com/apple/llvm-project/
> 2. Create inconsequential PR at https://github.com/apple/swift/
> 3. Use [[ 
> https://github.com/apple/swift/blob/main/docs/ContinuousIntegration.md#cross-repository-testing
>  | Cross Repository Testing ]] to test clang change from swift on Linux.
If things break I think we would just add the flag to a few more drivers and 
fix it like that. I'm definitely not trying to drive anyone away from clang 
modules, I'm just hoping they don't need the strange current behavior. If it 
turns out to be too much of a mess then it can be on by default and we'll do an 
fno flag instead that we can turn off in the Darwin driver when we're ready.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:176
 COMPATIBLE_LANGOPT(CPlusPlusModules, 1, 0, "C++ modules syntax")
+LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin headers belong to system 
modules, and _Builtin_ modules are ignored for cstdlib headers")
 BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,

iana wrote:
> vsapsai wrote:
> > Why not to make the default value `true` to preserve the old behavior and 
> > switch on the new behavior in the driver? It's not a veiled way to force 
> > you to change it, I'm genuinely curious and want to consider pro/cons of 
> > various alternatives.
> I did it this way because the current behavior is nonintuitive and just kind 
> of bizarre. I had no idea that `Darwin.C.stdint` also included the compiler's 
> stdint.h, and that the OS's stdint.h was treated textually. I thought it 
> would be better for the default behavior to be for modules to behave 
> consistently and obviously, not have secret special cases, and be usable with 
> C++.
> 
> The con of course is that if anyone besides Apple depends on the current 
> behavior they're going to either have to fix their OS modules or add this 
> flag. I'm not sure if other platforms have a convenient driver class where 
> they can do that.
I'm fine with the more aggressive approach as long as we test Swift on Linux. 
That's the only project I know that is willing to keep using clang modules. For 
other projects if things break and it is a motivation to move to C++ modules, 
it is a positive direction, as for me.

Another reason I wanted to be more cautious is because of 
https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213
 change. Clang started enforcing 20+ years old rule and that has broken some 
stuff. So I don't see "you should've known better before relying on this 
behavior" as a good argument.

Anyway, for Swift testing here is what you can do (if you have your own plan, 
go ahead):
1. Make PR for the same change as this one at 
https://github.com/apple/llvm-project/
2. Create inconsequential PR at https://github.com/apple/swift/
3. Use [[ 
https://github.com/apple/swift/blob/main/docs/ContinuousIntegration.md#cross-repository-testing
 | Cross Repository Testing ]] to test clang change from swift on Linux.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2860-2861
+return SDKVersion >= VersionTuple(99U);
+  default:
+return true;
+  }

iana wrote:
> vsapsai wrote:
> > Another question regarding defaults. Doesn't look consistent with your 
> > position
> > > [...] I thought it safer to default to the current behavior which is 
> > > false.
> > 
> > Personally I gravitate towards `false` for extra safety but it's not a 
> > strong opinion.
> default only covers DriverKit, which can be true. Any future platforms should 
> also default to true.
Ok, that looks like a reasonable choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

iana marked an inline comment as done.
iana added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:176
 COMPATIBLE_LANGOPT(CPlusPlusModules, 1, 0, "C++ modules syntax")
+LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin headers belong to system 
modules, and _Builtin_ modules are ignored for cstdlib headers")
 BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,

vsapsai wrote:
> Why not to make the default value `true` to preserve the old behavior and 
> switch on the new behavior in the driver? It's not a veiled way to force you 
> to change it, I'm genuinely curious and want to consider pro/cons of various 
> alternatives.
I did it this way because the current behavior is nonintuitive and just kind of 
bizarre. I had no idea that `Darwin.C.stdint` also included the compiler's 
stdint.h, and that the OS's stdint.h was treated textually. I thought it would 
be better for the default behavior to be for modules to behave consistently and 
obviously, not have secret special cases, and be usable with C++.

The con of course is that if anyone besides Apple depends on the current 
behavior they're going to either have to fix their OS modules or add this flag. 
I'm not sure if other platforms have a convenient driver class where they can 
do that.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2860-2861
+return SDKVersion >= VersionTuple(99U);
+  default:
+return true;
+  }

vsapsai wrote:
> Another question regarding defaults. Doesn't look consistent with your 
> position
> > [...] I thought it safer to default to the current behavior which is false.
> 
> Personally I gravitate towards `false` for extra safety but it's not a strong 
> opinion.
default only covers DriverKit, which can be true. Any future platforms should 
also default to true.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D159483#4641191 , @iana wrote:

> A big assumption this patch makes is that `ModuleMap::isBuiltinHeader` is 
> primarily to support Apple's unfortunate module needs. Thus this patch turns 
> that behavior off by default, which makes things work the way one would 
> expect. That is, when usr/include/module.modulemap references stdint.h, that 
> just means usr/include/stdint.h and it doesn't also pull in the clang builtin 
> stdint.h, it doesn't transform usr/include/stdint.h into a textual header, 
> etc. I'm hoping that's acceptable behavior on non-Apple platforms, but if 
> someone knows otherwise please let me know and we can rethink how the option 
> should be defined and set.

Good way to test the assumptions can be checking how Swift works with this 
change on Linux. There are not as many module maps there as in Apple SDKs but 
there are some.




Comment at: clang/include/clang/Basic/LangOptions.def:176
 COMPATIBLE_LANGOPT(CPlusPlusModules, 1, 0, "C++ modules syntax")
+LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin headers belong to system 
modules, and _Builtin_ modules are ignored for cstdlib headers")
 BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,

Why not to make the default value `true` to preserve the old behavior and 
switch on the new behavior in the driver? It's not a veiled way to force you to 
change it, I'm genuinely curious and want to consider pro/cons of various 
alternatives.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2860-2861
+return SDKVersion >= VersionTuple(99U);
+  default:
+return true;
+  }

Another question regarding defaults. Doesn't look consistent with your position
> [...] I thought it safer to default to the current behavior which is false.

Personally I gravitate towards `false` for extra safety but it's not a strong 
opinion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

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

__stddef_max_align_t.h always needs a header guard, because the type merger 
can't handle struct's. If it has a header guard then it always needs to have a 
module too, even if `-fbuiltin-headers-in-system-modules` is passed.

Change ModuleMap to not ignore the builtin modules completely, but rather to 
just skip their top level headers (which is isBuiltinHeaderName + inttypes.h 
and stdnoreturn.h). With that change, the implementation headers are always 
modular, and `__has_feature(builtin_headers_in_system_modules)` is no longer 
necessary anywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

Files:
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json
  clang/test/Driver/darwin-builtin-modules.c
  clang/test/Modules/Werror-Wsystem-headers.m
  clang/test/Modules/context-hash.c
  clang/test/Modules/crash-vfs-include-pch.m
  clang/test/Modules/cstd.m
  clang/test/Modules/pch-used.m
  clang/test/Modules/shadowed-submodule.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,5 @@
 size_t getSize(void);
 
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -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 -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,5 +1,5 @@
 // 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
+// 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
 
 #include "ptrdiff_t.h"
 
Index: clang/test/Modules/shadowed-submodule.m
===
--- clang/test/Modules/shadowed-submodule.m
+++ clang/test/Modules/shadowed-submodule.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
 
 @import Foo; // expected-error {{module 'A' was built in directory}}
  // expected-note@shadowed-submodule.m:4 {{imported by module 'Foo'}}
Index: clang/test/Modules/pch-used.m
===
--- clang/test/Modules/pch-used.m
+++ clang/test/Modules/pch-used.m
@@ -1,8 +1,8 @@
 // UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // RUN: rm -rf %t
 // RUN: mkdir %t
-// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
-// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
+// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm -o - | FileCheck %s
 
 void f(void) { SPXTrace(); }
 void g(void) { double x = DBL_MAX; }
Index: clang/test/Modules/cstd.m
===
--- clang/test/Modules/cstd.m
+++ clang/test/Modules/cstd.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -internal-isystem %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -D__need_wint_t -Werror=implicit-function-declaration %s
+// RUN: %clang_cc1 -fsyntax-only -internal-isystem %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -D__need_wint_t 

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

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

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

Files:
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Headers/__stdarg___gnuc_va_list.h
  clang/lib/Headers/__stdarg___va_copy.h
  clang/lib/Headers/__stdarg_va_arg.h
  clang/lib/Headers/__stdarg_va_copy.h
  clang/lib/Headers/__stdarg_va_list.h
  clang/lib/Headers/__stddef_max_align_t.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/stdarg.h
  clang/lib/Headers/stddef.h
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json
  clang/test/Driver/darwin-builtin-modules.c
  clang/test/Modules/Werror-Wsystem-headers.m
  clang/test/Modules/context-hash.c
  clang/test/Modules/crash-vfs-include-pch.m
  clang/test/Modules/cstd.m
  clang/test/Modules/pch-used.m
  clang/test/Modules/shadowed-submodule.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,5 @@
 size_t getSize(void);
 
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -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 -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,5 +1,5 @@
 // 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
+// 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
 
 #include "ptrdiff_t.h"
 
Index: clang/test/Modules/shadowed-submodule.m
===
--- clang/test/Modules/shadowed-submodule.m
+++ clang/test/Modules/shadowed-submodule.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
 
 @import Foo; // expected-error {{module 'A' was built in directory}}
  // expected-note@shadowed-submodule.m:4 {{imported by module 'Foo'}}
Index: clang/test/Modules/pch-used.m
===
--- clang/test/Modules/pch-used.m
+++ clang/test/Modules/pch-used.m
@@ -1,8 +1,8 @@
 // UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // RUN: rm -rf %t
 // RUN: mkdir %t
-// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
-// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
+// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm -o - | FileCheck %s
 
 void f(void) { SPXTrace(); }
 void g(void) { double x = DBL_MAX; }
Index: clang/test/Modules/cstd.m
===
--- clang/test/Modules/cstd.m
+++ clang/test/Modules/cstd.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -internal-isystem %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -D__need_wint_t -Werror=implicit-function-declaration %s
+// RUN: %clang_cc1 -fsyntax-only -internal-isystem %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps 

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

iana updated this revision to Diff 556230.
iana marked 3 inline comments as done.
iana added a comment.

Add comments to stdarg.h and stddef.h explaining their interesting header guard 
setup.
Ignore -fbuiltin-headers-in-system-modules if -fmodules isn't passed.
DriverKit doesn't need to pass -fbuiltin-headers-in-system-modules since it 
doesn't have a module map at all.
Add a test for 
-fbuiltin-headers-in-system-modules/__has_feature(builtin_headers_in_system_modules)
Miscellaneous review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

Files:
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Headers/__stdarg___gnuc_va_list.h
  clang/lib/Headers/__stdarg___va_copy.h
  clang/lib/Headers/__stdarg_va_arg.h
  clang/lib/Headers/__stdarg_va_copy.h
  clang/lib/Headers/__stdarg_va_list.h
  clang/lib/Headers/__stddef_max_align_t.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/stdarg.h
  clang/lib/Headers/stddef.h
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json
  clang/test/Driver/darwin-builtin-modules.c
  clang/test/Modules/Werror-Wsystem-headers.m
  clang/test/Modules/context-hash.c
  clang/test/Modules/crash-vfs-include-pch.m
  clang/test/Modules/cstd.m
  clang/test/Modules/pch-used.m
  clang/test/Modules/shadowed-submodule.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,5 @@
 size_t getSize(void);
 
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -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 -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,5 +1,5 @@
 // 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
+// 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
 
 #include "ptrdiff_t.h"
 
Index: clang/test/Modules/shadowed-submodule.m
===
--- clang/test/Modules/shadowed-submodule.m
+++ clang/test/Modules/shadowed-submodule.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
 
 @import Foo; // expected-error {{module 'A' was built in directory}}
  // expected-note@shadowed-submodule.m:4 {{imported by module 'Foo'}}
Index: clang/test/Modules/pch-used.m
===
--- clang/test/Modules/pch-used.m
+++ clang/test/Modules/pch-used.m
@@ -1,8 +1,8 @@
 // UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // RUN: rm -rf %t
 // RUN: mkdir %t
-// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
-// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
+// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm -o - | FileCheck %s
 
 void f(void) { SPXTrace(); }
 void g(void) { double x = DBL_MAX; }
Index: clang/test/Modules/cstd.m

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

iana marked an inline comment as done.
iana added inline comments.



Comment at: clang/include/clang/Basic/Features.def:233
 FEATURE(modules, LangOpts.Modules)
+FEATURE(builtin_headers_in_system_modules, 
LangOpts.BuiltinHeadersInSystemModules)
 FEATURE(safe_stack, LangOpts.Sanitize.has(SanitizerKind::SafeStack))

benlangmuir wrote:
> iana wrote:
> > benlangmuir wrote:
> > > This appears to be in a list of `// C++ TSes`.  Near the bottom there is 
> > > 'Miscellaneous language extensions' which might be a better fit.
> > I thought it went with `FEATURE(modules, LangOpts.Modules)`, but I can put 
> > it down in 'Miscellaneous language extensions' if that's better.
> I think misc is probably better.
 



Comment at: clang/include/clang/Driver/Options.td:2944
   [NoXarchOption], [ClangOption, CLOption]>>;
+def fbuiltin_headers_in_system_modules : Flag <["-"], 
"fbuiltin-headers-in-system-modules">,
+  Group,

benlangmuir wrote:
> iana wrote:
> > I don't love this flag name, but I couldn't come up with anything more 
> > succinct. It actually does two things.
> > 
> >   # Turns on `ModuleMap::isBuiltinHeader` behavior, that is the builtin 
> > headers get added into the system modules.
> >   # Causes the module map parser to ignore all of the new builtin modules 
> > added in D159064. This is needed before the modules are added to assist 
> > with Apple internal staging with the Swift compiler.
> > 
> I think the name is good enough for a -cc1 option, unless someone else has a 
> better suggestion. I suggest adding a HelpText - you could basically copy the 
> description you added to LangOptions.def
Sure



Comment at: clang/lib/Lex/ModuleMap.cpp:1540
+/// or added to Map's Modules/ModuleScopeIDs).
+bool IgnoreModules = false;
+

benlangmuir wrote:
> iana wrote:
> > This an everything under it is gross, but the only other thing I could 
> > think of was to duplicate the module map and decided to load a special one 
> > for BuiltinHeadersInSystemModules. That seemed weirder.
> Would it be possible to get the right semantics using a `requires` decl in 
> the modules plus a new module feature?
Maybe? I guess e.g. _Builtin_stdint would be seen, the new requires flag would 
be seen, and if it's not satisfied then its headers wouldn't be added to the 
module? That seems like a really weird use of requires though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added a comment.

I suggest we add a comment explaining the weird has_feature checks being added 
here (even if they're less weird than what they're replacing).  Maybe just a 
comment in stdarg.h and/or stddef.h and then the __std(arg|def) headers could 
just add a one-liner reference  "see comment in std(arg|def).h".




Comment at: clang/include/clang/Basic/Features.def:233
 FEATURE(modules, LangOpts.Modules)
+FEATURE(builtin_headers_in_system_modules, 
LangOpts.BuiltinHeadersInSystemModules)
 FEATURE(safe_stack, LangOpts.Sanitize.has(SanitizerKind::SafeStack))

iana wrote:
> benlangmuir wrote:
> > This appears to be in a list of `// C++ TSes`.  Near the bottom there is 
> > 'Miscellaneous language extensions' which might be a better fit.
> I thought it went with `FEATURE(modules, LangOpts.Modules)`, but I can put it 
> down in 'Miscellaneous language extensions' if that's better.
I think misc is probably better.



Comment at: clang/include/clang/Driver/Options.td:2944
   [NoXarchOption], [ClangOption, CLOption]>>;
+def fbuiltin_headers_in_system_modules : Flag <["-"], 
"fbuiltin-headers-in-system-modules">,
+  Group,

iana wrote:
> I don't love this flag name, but I couldn't come up with anything more 
> succinct. It actually does two things.
> 
>   # Turns on `ModuleMap::isBuiltinHeader` behavior, that is the builtin 
> headers get added into the system modules.
>   # Causes the module map parser to ignore all of the new builtin modules 
> added in D159064. This is needed before the modules are added to assist with 
> Apple internal staging with the Swift compiler.
> 
I think the name is good enough for a -cc1 option, unless someone else has a 
better suggestion. I suggest adding a HelpText - you could basically copy the 
description you added to LangOptions.def



Comment at: clang/include/clang/Driver/Options.td:2945-2946
+def fbuiltin_headers_in_system_modules : Flag <["-"], 
"fbuiltin-headers-in-system-modules">,
+  Group,
+  Flags<[NoXarchOption]>,
+  Visibility<[CC1Option]>,

iana wrote:
> I don't fully understand what these two do, but I think they apply here?
I agree they apply. `f_Group` affects a `ClaimAllArgs` call (via inheritance 
from `CompileOnly_Group`) but mostly puts it under a group in documentation.  
`NoXarchOption` means you can't modify this option per-arch in a multi-arch 
compilation.  That seems like what you want.



Comment at: clang/lib/Lex/ModuleMap.cpp:1540
+/// or added to Map's Modules/ModuleScopeIDs).
+bool IgnoreModules = false;
+

iana wrote:
> This an everything under it is gross, but the only other thing I could think 
> of was to duplicate the module map and decided to load a special one for 
> BuiltinHeadersInSystemModules. That seemed weirder.
Would it be possible to get the right semantics using a `requires` decl in the 
modules plus a new module feature?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

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

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

Files:
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Headers/__stdarg___gnuc_va_list.h
  clang/lib/Headers/__stdarg___va_copy.h
  clang/lib/Headers/__stdarg_va_arg.h
  clang/lib/Headers/__stdarg_va_copy.h
  clang/lib/Headers/__stdarg_va_list.h
  clang/lib/Headers/__stddef_max_align_t.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/stdarg.h
  clang/lib/Headers/stddef.h
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Modules/Werror-Wsystem-headers.m
  clang/test/Modules/context-hash.c
  clang/test/Modules/crash-vfs-include-pch.m
  clang/test/Modules/cstd.m
  clang/test/Modules/pch-used.m
  clang/test/Modules/shadowed-submodule.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,5 @@
 size_t getSize(void);
 
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -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 -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,5 +1,5 @@
 // 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
+// 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
 
 #include "ptrdiff_t.h"
 
Index: clang/test/Modules/shadowed-submodule.m
===
--- clang/test/Modules/shadowed-submodule.m
+++ clang/test/Modules/shadowed-submodule.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
 
 @import Foo; // expected-error {{module 'A' was built in directory}}
  // expected-note@shadowed-submodule.m:4 {{imported by module 'Foo'}}
Index: clang/test/Modules/pch-used.m
===
--- clang/test/Modules/pch-used.m
+++ clang/test/Modules/pch-used.m
@@ -1,8 +1,8 @@
 // UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // RUN: rm -rf %t
 // RUN: mkdir %t
-// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
-// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
+// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm -o - | FileCheck %s
 
 void f(void) { SPXTrace(); }
 void g(void) { double x = DBL_MAX; }
Index: clang/test/Modules/cstd.m
===
--- clang/test/Modules/cstd.m
+++ clang/test/Modules/cstd.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -internal-isystem %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -D__need_wint_t -Werror=implicit-function-declaration %s
+// RUN: %clang_cc1 -fsyntax-only -internal-isystem %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -D__need_wint_t -Werror=implicit-function-declaration %s
 
 

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

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

Fix formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

Files:
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Headers/__stdarg___gnuc_va_list.h
  clang/lib/Headers/__stdarg___va_copy.h
  clang/lib/Headers/__stdarg_va_arg.h
  clang/lib/Headers/__stdarg_va_copy.h
  clang/lib/Headers/__stdarg_va_list.h
  clang/lib/Headers/__stddef_max_align_t.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/stdarg.h
  clang/lib/Headers/stddef.h
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Modules/Werror-Wsystem-headers.m
  clang/test/Modules/context-hash.c
  clang/test/Modules/crash-vfs-include-pch.m
  clang/test/Modules/cstd.m
  clang/test/Modules/pch-used.m
  clang/test/Modules/shadowed-submodule.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,5 @@
 size_t getSize(void);
 
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -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 -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,5 +1,5 @@
 // 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
+// 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
 
 #include "ptrdiff_t.h"
 
Index: clang/test/Modules/shadowed-submodule.m
===
--- clang/test/Modules/shadowed-submodule.m
+++ clang/test/Modules/shadowed-submodule.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
 
 @import Foo; // expected-error {{module 'A' was built in directory}}
  // expected-note@shadowed-submodule.m:4 {{imported by module 'Foo'}}
Index: clang/test/Modules/pch-used.m
===
--- clang/test/Modules/pch-used.m
+++ clang/test/Modules/pch-used.m
@@ -1,8 +1,8 @@
 // UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // RUN: rm -rf %t
 // RUN: mkdir %t
-// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
-// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
+// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm -o - | FileCheck %s
 
 void f(void) { SPXTrace(); }
 void g(void) { double x = DBL_MAX; }
Index: clang/test/Modules/cstd.m
===
--- clang/test/Modules/cstd.m
+++ clang/test/Modules/cstd.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -internal-isystem %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -D__need_wint_t -Werror=implicit-function-declaration %s
+// RUN: %clang_cc1 -fsyntax-only -internal-isystem %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -D__need_wint_t -Werror=implicit-function-declaration 

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

iana marked an inline comment as done.
iana added a comment.

In D159483#4641289 , @benlangmuir 
wrote:

>>> How does this work today? Wouldn't the include guard prevent this?
>>
>> Today they don't define their include guard when building with modules.
>
> Thanks - I can see some headers behave that way, or in other cases there are 
> other sorts of has_feature(modules) checks that I can see you're modifying, 
> but what about all the *va_* headers that currently just seem to have normal 
> header guards?

I think the stdarg ones didn't always work properly in modules and we just got 
lucky.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added a comment.

>> How does this work today? Wouldn't the include guard prevent this?
>
> Today they don't define their include guard when building with modules.

Thanks - I can see some headers behave that way, or in other cases there are 
other sorts of has_feature(modules) checks that I can see you're modifying, but 
what about all the *va_* headers that currently just seem to have normal header 
guards?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

iana marked an inline comment as done.
iana added inline comments.



Comment at: clang/include/clang/Basic/Features.def:233
 FEATURE(modules, LangOpts.Modules)
+FEATURE(builtin_headers_in_system_modules, 
LangOpts.BuiltinHeadersInSystemModules)
 FEATURE(safe_stack, LangOpts.Sanitize.has(SanitizerKind::SafeStack))

benlangmuir wrote:
> This appears to be in a list of `// C++ TSes`.  Near the bottom there is 
> 'Miscellaneous language extensions' which might be a better fit.
I thought it went with `FEATURE(modules, LangOpts.Modules)`, but I can put it 
down in 'Miscellaneous language extensions' if that's better.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2851
+  if (!SDKInfo)
+return false;
+  

benlangmuir wrote:
> I would have expected the default to be true; do old SDKs not have SDKInfo or 
> something?
I'm not sure if we ever expect there to not be SDKInfo, but I thought it safer 
to default to the current behavior which is false.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2856
+  case Darwin::MacOS:
+return SDKVersion >= VersionTuple(100U);
+  case Darwin::IPhoneOS:

benlangmuir wrote:
> Are these really supposed to be 100 or is this a placeholder?
Placeholder until we actually update the SDKs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

iana added a comment.

In D159483#4641231 , @benlangmuir 
wrote:

>> but need to be repeatedly included when they're used in system modules
>
> How does this work today? Wouldn't the include guard prevent this?

Today they don't define their include guard when building with modules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

iana added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2944
   [NoXarchOption], [ClangOption, CLOption]>>;
+def fbuiltin_headers_in_system_modules : Flag <["-"], 
"fbuiltin-headers-in-system-modules">,
+  Group,

I don't love this flag name, but I couldn't come up with anything more 
succinct. It actually does two things.

  # Turns on `ModuleMap::isBuiltinHeader` behavior, that is the builtin headers 
get added into the system modules.
  # Causes the module map parser to ignore all of the new builtin modules added 
in D159064. This is needed before the modules are added to assist with Apple 
internal staging with the Swift compiler.




Comment at: clang/include/clang/Driver/Options.td:2945-2946
+def fbuiltin_headers_in_system_modules : Flag <["-"], 
"fbuiltin-headers-in-system-modules">,
+  Group,
+  Flags<[NoXarchOption]>,
+  Visibility<[CC1Option]>,

I don't fully understand what these two do, but I think they apply here?



Comment at: clang/lib/Headers/__stddef_ptrdiff_t.h:10
 
-#if !defined(_PTRDIFF_T) || __has_feature(modules)
-/* Always define ptrdiff_t when modules are available. */
-#if !__has_feature(modules)
+#if !defined(_PTRDIFF_T) || __has_feature(builtin_headers_in_system_modules)
 #define _PTRDIFF_T

This is to support patterns like the Modules/stddef.c test. The module map 
looks like this.

```
module StdDef {
  module Other {
header "other.h" // includes stddef.h
  }
  module PtrDiffT {
header "ptrdiff_t.h" // defines __needs_ptrdiff_t and includes stddef.h
  }
  module IncludeAgain {
header "include_again.h" // includes stddef.h
  }
}
```
With builtin_headers_in_system_modules, __stddef_ptrdiff_t.h will not be in a 
module. It will first be seen in other.h and define its header guard. When it's 
seen again in ptrdiff_t.h, it needs to declare ptrdiff_t again, so it needs to 
ignore its header guard.

Without builtin_headers_in_system_modules, __stddef_ptrdiff_t.h will be in the 
`_Builtin_stddef` module. It needs its header guard there so that it's only 
included once when building the module. Even though ptrdiff_t.h will see the 
header guard, it will still import `_Builtin_stddef.ptrdiff_t` and get the 
declaration.



Comment at: clang/lib/Headers/stddef.h:10
 
-#if !defined(__STDDEF_H) || defined(__need_ptrdiff_t) ||   
\
-defined(__need_size_t) || defined(__need_rsize_t) ||   
\
-defined(__need_wchar_t) || defined(__need_NULL) || 
\
-defined(__need_nullptr_t) || defined(__need_unreachable) ||
\
-defined(__need_max_align_t) || defined(__need_offsetof) || 
\
-defined(__need_wint_t) ||  
\
-(defined(__STDC_WANT_LIB_EXT1__) && __STDC_WANT_LIB_EXT1__ >= 1)
+#if !defined(__STDDEF_H) || __has_feature(modules) ||  
\
+(defined(__STDC_WANT_LIB_EXT1__) && __STDC_WANT_LIB_EXT1__ >= 1) ||
\

As a `textual` header, this needs to ignore its header guard in module mode, 
even without builtin_headers_in_system_modules. Going back to the 
Modules/stddef.c test.

```
module StdDef {
  module Other {
header "other.h" // includes stddef.h
export *
  }
  module PtrDiffT {
header "ptrdiff_t.h" // defines __needs_ptrdiff_t and includes stddef.h
  }
  module IncludeAgain {
header "include_again.h" // includes stddef.h
export *
  }
}
```

When other.h compiles, __STDDEF_H will get defined and 
`_Builtin_stddef.ptrdiff_t` will be imported. But when include_again.h 
compiles, __STDDEF_H will already be defined and `_Builtin_stddef.ptrdiff_t` 
won't be imported because the include will be skipped in stddef.h. That's ok 
when include_again.h is building because it will see ptrdiff_t from when 
other.h included it. But a client that only includes include_again.h won't see 
any types because `StdDef.IncludeAgain` skipped the contents of stddef.h and 
skipped importing `_Builtin_stddef.ptrdiff_t`, and so there's nothing for 
`export *` to export.

The other solution is to never define __STDDEF_H in module mode, but I think 
that's strange behavior. Several headers in the Apple SDKs test if a header was 
included by checking for its header guard (questionable, but it's what they 
do). stddef.h isn't one of those headers, but it still seems like it should 
define everything in modules mode that it defines without modules.



Comment at: clang/lib/Lex/ModuleMap.cpp:259
+/// headers.
+static bool isBuiltinHeaderName(StringRef FileName) {
+  return llvm::StringSwitch(FileName)

This had to be moved up just because it's used by methods under here.



Comment at: clang/lib/Lex/ModuleMap.cpp:1540
+/// or added to Map's Modules/ModuleScopeIDs).
+bool IgnoreModules = false;
+

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added a comment.

> but need to be repeatedly included when they're used in system modules

How does this work today? Wouldn't the include guard prevent this?




Comment at: clang/include/clang/Basic/Features.def:233
 FEATURE(modules, LangOpts.Modules)
+FEATURE(builtin_headers_in_system_modules, 
LangOpts.BuiltinHeadersInSystemModules)
 FEATURE(safe_stack, LangOpts.Sanitize.has(SanitizerKind::SafeStack))

This appears to be in a list of `// C++ TSes`.  Near the bottom there is 
'Miscellaneous language extensions' which might be a better fit.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2851
+  if (!SDKInfo)
+return false;
+  

I would have expected the default to be true; do old SDKs not have SDKInfo or 
something?



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2856
+  case Darwin::MacOS:
+return SDKVersion >= VersionTuple(100U);
+  case Darwin::IPhoneOS:

Are these really supposed to be 100 or is this a placeholder?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

iana added a comment.

A big assumption this patch makes is that `ModuleMap::isBuiltinHeader` is 
primarily to support Apple's unfortunate module needs. Thus this patch turns 
that behavior off by default, which makes things work the way one would expect. 
That is, when usr/include/module.modulemap references stdint.h, that just means 
usr/include/stdint.h and it doesn't also pull in the clang builtin stdint.h, it 
doesn't transform usr/include/stdint.h into a textual header, etc. I'm hoping 
that's acceptable behavior on non-Apple platforms, but if someone knows 
otherwise please let me know and we can rethink how the option should be 
defined and set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159483

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

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

[Ian Anderson via Phabricator via cfe-commits]

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

Including select builtin headers in system modules is a workaround for module 
cycles, primarily in Apple's Darwin module that includes all of its C standard 
library headers. The workaround is problematic because it doesn't include all 
of the builtin headers (inttypes.h is notably absent), and it also doesn't 
include C++ headers. The straightforward for for this is to make top level 
modules for all of the C standard library headers and unwind.h in C++, clang, 
and the OS.

However, doing so in clang before the OS modules are ready re-introduces the 
module cycles. Add a -fbuiltin-headers-in-system-modules option to control if 
the special builtin headers belong to system modules or builtin modules. Pass 
the option by default for Apple.

The __stdarg and __stddef headers need to use header guards when they're 
precompiled into their builtin modules, but need to be repeatedly included when 
they're used in system modules. Add a builtin_headers_in_system_modules feature 
so those headers can conditionally ignore their header guards. Always define 
the header guards for consistency. stdarg.h and stddef.h will be textual 
headers in the builtin modules, and so need to be repeatedly included in both 
the system and builtin module case. Ignore their header guards when building 
with modules, but still define the guards.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159483

Files:
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Headers/__stdarg___gnuc_va_list.h
  clang/lib/Headers/__stdarg___va_copy.h
  clang/lib/Headers/__stdarg_va_arg.h
  clang/lib/Headers/__stdarg_va_copy.h
  clang/lib/Headers/__stdarg_va_list.h
  clang/lib/Headers/__stddef_max_align_t.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/stdarg.h
  clang/lib/Headers/stddef.h
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Modules/Werror-Wsystem-headers.m
  clang/test/Modules/context-hash.c
  clang/test/Modules/crash-vfs-include-pch.m
  clang/test/Modules/cstd.m
  clang/test/Modules/pch-used.m
  clang/test/Modules/shadowed-submodule.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,5 @@
 size_t getSize(void);
 
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -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 -verify
 // expected-no-diagnostics
Index: clang/test/Modules/stddef.c
===
--- clang/test/Modules/stddef.c
+++ clang/test/Modules/stddef.c
@@ -1,5 +1,5 @@
 // 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
+// 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
 
 #include "ptrdiff_t.h"
 
Index: clang/test/Modules/shadowed-submodule.m
===
--- clang/test/Modules/shadowed-submodule.m
+++ clang/test/Modules/shadowed-submodule.m
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I %S/Inputs/shadowed-submodule/Foo -I %S/Inputs/shadowed-submodule/A2 %s -verify
 
 @import Foo; // expected-error {{module 'A' was built in directory}}
  // expected-note@shadowed-submodule.m:4 {{imported by module 'Foo'}}
Index: clang/test/Modules/pch-used.m
===
--- clang/test/Modules/pch-used.m
+++ clang/test/Modules/pch-used.m
@@ -1,8 +1,8 @@
 // UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // RUN: rm -rf %t
 // RUN: mkdir %t
-// RUN: %clang_cc1 -x