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 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 %t/out/pch-used.h.pch -fmodules -fimplicit-module-maps \
-// RUN:     -fmodules-cache-path=%t/cache -O0 \
+// RUN:     -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t/cache -O0 \
 // RUN:     -isystem %S/Inputs/System/usr/include
 
 // RUN: env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
 // RUN: not %clang %s -E -include-pch %t/out/pch-used.h.pch -fmodules -nostdlibinc \
-// RUN:     -fimplicit-module-maps -fmodules-cache-path=%t/cache -O0 \
-// RUN:     -Xclang -fno-validate-pch -isystem %S/Inputs/System/usr/include \
-// RUN:     -o %t/output.E 2>&1 | FileCheck %s
+// RUN:     -fimplicit-module-maps -fbuiltin-headers-in-system-modules \
+// RUN:     -fmodules-cache-path=%t/cache -O0 -Xclang -fno-validate-pch \
+// RUN:     -isystem %S/Inputs/System/usr/include -o %t/output.E 2>&1 | FileCheck %s
 
 // RUN: FileCheck --check-prefix=CHECKSH %s -input-file %t/crash-vfs-*.sh
 // RUN: FileCheck --check-prefix=CHECKYAML %s -input-file \
Index: clang/test/Modules/context-hash.c
===================================================================
--- clang/test/Modules/context-hash.c
+++ clang/test/Modules/context-hash.c
@@ -4,22 +4,24 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fsyntax-only -internal-isystem \
 // RUN:   %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps \
-// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t1
+// RUN:   -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t %s \
+// RUN:   -Rmodule-build 2> %t1
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fsyntax-only -internal-isystem \
 // RUN:   %S/Inputs/System/usr/include -internal-isystem %S -fmodules \
-// RUN:   -fimplicit-module-maps -fmodules-cache-path=%t %s -Rmodule-build 2> \
-// RUN:   %t2
+// RUN:   -fbuiltin-headers-in-system-modules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t2
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fsyntax-only -internal-isystem \
 // RUN:   %S/Inputs/System/usr/include -internal-isystem %S -fmodules \
-// RUN:   -fimplicit-module-maps -fmodules-cache-path=%t %s \
-// RUN:   -fmodules-strict-context-hash -Rmodule-build 2> %t3
+// RUN:   -fbuiltin-headers-in-system-modules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t %s -fmodules-strict-context-hash \
+// RUN:   -Rmodule-build 2> %t3
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fsyntax-only -Weverything -internal-isystem \
 // RUN:   %S/Inputs/System/usr/include -fmodules -fmodules-strict-context-hash \
-// RUN:   -fimplicit-module-maps -fmodules-cache-path=%t %s -Rmodule-build 2> \
-// RUN:   %t4
+// RUN:   -fbuiltin-headers-in-system-modules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t4
 // RUN: echo %t > %t.path
 // RUN: cat %t.path %t1 %t2 %t3 %t4 | FileCheck %s
 
@@ -29,16 +31,17 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fsyntax-only -internal-isystem \
 // RUN:   %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps \
-// RUN:   -fmodules-cache-path=%t -x objective-c %s -Rmodule-build 2> %t1
+// RUN:   -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t \
+// RUN:   -x objective-c %s -Rmodule-build 2> %t1
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fsyntax-only -internal-isystem \
 // RUN:   %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps \
-// RUN:   -fobjc-runtime=macosx-1.0.0.0 \
+// RUN:   -fbuiltin-headers-in-system-modules -fobjc-runtime=macosx-1.0.0.0 \
 // RUN:   -fmodules-cache-path=%t -x objective-c %s -Rmodule-build 2> %t2
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fsyntax-only -internal-isystem \
 // RUN:   %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps \
-// RUN:   -fcomment-block-commands=lp,bj \
+// RUN:   -fbuiltin-headers-in-system-modules -fcomment-block-commands=lp,bj \
 // RUN:   -fmodules-cache-path=%t -x objective-c %s -Rmodule-build 2> %t3
 // RUN: echo %t > %t.path
 // RUN: cat %t.path %t1 %t2 %t3 | FileCheck --check-prefix=LANGOPTS %s
Index: clang/test/Modules/Werror-Wsystem-headers.m
===================================================================
--- clang/test/Modules/Werror-Wsystem-headers.m
+++ clang/test/Modules/Werror-Wsystem-headers.m
@@ -3,19 +3,21 @@
 // RUN: mkdir %t-saved
 
 // Initial module build
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash \
-// RUN:     -isystem %S/Inputs/System/usr/include -fsyntax-only %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules \
+// RUN:     -fmodules-cache-path=%t -fdisable-module-hash -isystem %S/Inputs/System/usr/include \
+// RUN:     -fsyntax-only %s -verify
 // RUN: cp %t/cstd.pcm %t-saved/cstd.pcm
 
 // Even with -Werror don't rebuild a system module
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash \
-// RUN:     -isystem %S/Inputs/System/usr/include -fsyntax-only %s -verify -Werror
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules \
+// RUN:     -fmodules-cache-path=%t -fdisable-module-hash -isystem %S/Inputs/System/usr/include \
+// RUN:     -fsyntax-only %s -verify -Werror
 // RUN: diff %t/cstd.pcm %t-saved/cstd.pcm
 
 // Unless -Wsystem-headers is on
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fdisable-module-hash \
-// RUN:     -isystem %S/Inputs/System/usr/include -fsyntax-only %s -verify \
-// RUN:     -Werror=unused -Wsystem-headers
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules \
+// RUN:     -fmodules-cache-path=%t -fdisable-module-hash -isystem %S/Inputs/System/usr/include \
+// RUN:     -fsyntax-only %s -verify -Werror=unused -Wsystem-headers
 // RUN: not diff %t/cstd.pcm %t-saved/cstd.pcm
 
 // expected-no-diagnostics
Index: clang/lib/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -253,6 +253,45 @@
   return NormalHdrFile;
 }
 
+/// Determine whether the given file name is the name of a builtin
+/// header, supplied by Clang to replace, override, or augment existing system
+/// headers.
+static bool isBuiltinHeaderName(StringRef FileName) {
+  return llvm::StringSwitch<bool>(FileName)
+           .Case("float.h", true)
+           .Case("iso646.h", true)
+           .Case("limits.h", true)
+           .Case("stdalign.h", true)
+           .Case("stdarg.h", true)
+           .Case("stdatomic.h", true)
+           .Case("stdbool.h", true)
+           .Case("stddef.h", true)
+           .Case("stdint.h", true)
+           .Case("tgmath.h", true)
+           .Case("unwind.h", true)
+           .Default(false);
+}
+
+/// Determine whether the given module name is the name of a builtin
+/// module that is cyclic with a system module.
+static bool isBuiltInModuleName(StringRef ModuleName) {
+  return llvm::StringSwitch<bool>(ModuleName)
+           .Case("_Builtin_float", true)
+           .Case("_Builtin_inttypes", true)
+           .Case("_Builtin_iso646", true)
+           .Case("_Builtin_limits", true)
+           .Case("_Builtin_stdalign", true)
+           .Case("_Builtin_stdarg", true)
+           .Case("_Builtin_stdatomic", true)
+           .Case("_Builtin_stdbool", true)
+           .Case("_Builtin_stddef", true)
+           .Case("_Builtin_stdint", true)
+           .Case("_Builtin_stdnoreturn", true)
+           .Case("_Builtin_tgmath", true)
+           .Case("_Builtin_unwind", true)
+           .Default(false);
+}
+
 void ModuleMap::resolveHeader(Module *Mod,
                               const Module::UnresolvedHeaderDirective &Header,
                               bool &NeedsFramework) {
@@ -297,7 +336,7 @@
       llvm::sys::path::is_absolute(Header.FileName) ||
       Mod->isPartOfFramework() || !Mod->IsSystem || Header.IsUmbrella ||
       !BuiltinIncludeDir || BuiltinIncludeDir == Mod->Directory ||
-      !isBuiltinHeader(Header.FileName))
+      !LangOpts.BuiltinHeadersInSystemModules || !isBuiltinHeaderName(Header.FileName))
     return false;
 
   // This is a system module with a top-level header. This header
@@ -373,28 +412,9 @@
   return Name;
 }
 
-/// Determine whether the given file name is the name of a builtin
-/// header, supplied by Clang to replace, override, or augment existing system
-/// headers.
-bool ModuleMap::isBuiltinHeader(StringRef FileName) {
-  return llvm::StringSwitch<bool>(FileName)
-           .Case("float.h", true)
-           .Case("iso646.h", true)
-           .Case("limits.h", true)
-           .Case("stdalign.h", true)
-           .Case("stdarg.h", true)
-           .Case("stdatomic.h", true)
-           .Case("stdbool.h", true)
-           .Case("stddef.h", true)
-           .Case("stdint.h", true)
-           .Case("tgmath.h", true)
-           .Case("unwind.h", true)
-           .Default(false);
-}
-
 bool ModuleMap::isBuiltinHeader(const FileEntry *File) {
-  return File->getDir() == BuiltinIncludeDir &&
-         ModuleMap::isBuiltinHeader(llvm::sys::path::filename(File->getName()));
+  return File->getDir() == BuiltinIncludeDir && LangOpts.BuiltinHeadersInSystemModules &&
+         isBuiltinHeaderName(llvm::sys::path::filename(File->getName()));
 }
 
 ModuleMap::HeadersMap::iterator
@@ -1515,6 +1535,10 @@
     /// The active module.
     Module *ActiveModule = nullptr;
 
+    /// Whether parsed modules should be ignored (not counted in Map's NumCreatedModules
+    /// or added to Map's Modules/ModuleScopeIDs).
+    bool IgnoreModules = false;
+
     /// Whether a module uses the 'requires excluded' hack to mark its
     /// contents as 'textual'.
     ///
@@ -1947,6 +1971,7 @@
     return;
   }
 
+  bool SetIgnoreModules = false;
   if (ActiveModule) {
     if (Id.size() > 1) {
       Diags.Report(Id.front().second, diag::err_mmap_nested_submodule_id)
@@ -1955,12 +1980,21 @@
       HadError = true;
       return;
     }
-  } else if (Id.size() == 1 && Explicit) {
-    // Top-level modules can't be explicit.
-    Diags.Report(ExplicitLoc, diag::err_mmap_explicit_top_level);
-    Explicit = false;
-    ExplicitLoc = SourceLocation();
-    HadError = true;
+  } else if (Id.size() == 1) {
+    if (Explicit) {
+      // Top-level modules can't be explicit.
+      Diags.Report(ExplicitLoc, diag::err_mmap_explicit_top_level);
+      Explicit = false;
+      ExplicitLoc = SourceLocation();
+      HadError = true;
+    } else if (ModuleMapFile->getDir() == Map.BuiltinIncludeDir &&
+               Map.LangOpts.BuiltinHeadersInSystemModules &&
+               isBuiltInModuleName(Id[0].first)) {
+      // If the builtin headers belong to system modules, ignore
+      // their builtin modules.
+      IgnoreModules = true;
+      SetIgnoreModules = true;
+    }
   }
 
   Module *PreviousActiveModule = ActiveModule;
@@ -2078,8 +2112,12 @@
 
   // Start defining this module.
   if (ShadowingModule) {
+    assert(!IgnoreModules);
     ActiveModule =
         Map.createShadowedModule(ModuleName, Framework, ShadowingModule);
+  } else if (IgnoreModules) {
+    ActiveModule = new Module(ModuleName, SourceLocation(), ActiveModule,
+                              Framework, Explicit, 0);
   } else {
     ActiveModule =
         Map.findOrCreateModule(ModuleName, ActiveModule, Framework, Explicit)
@@ -2214,6 +2252,8 @@
 
   // We're done parsing this module. Pop back to the previous module.
   ActiveModule = PreviousActiveModule;
+  if (SetIgnoreModules)
+    IgnoreModules = false;
 }
 
 /// Parse an extern module declaration.
@@ -2249,20 +2289,22 @@
   std::string FileName = std::string(Tok.getString());
   consumeToken(); // filename
 
-  StringRef FileNameRef = FileName;
-  SmallString<128> ModuleMapFileName;
-  if (llvm::sys::path::is_relative(FileNameRef)) {
-    ModuleMapFileName += Directory.getName();
-    llvm::sys::path::append(ModuleMapFileName, FileName);
-    FileNameRef = ModuleMapFileName;
+  if (!IgnoreModules) {
+    StringRef FileNameRef = FileName;
+    SmallString<128> ModuleMapFileName;
+    if (llvm::sys::path::is_relative(FileNameRef)) {
+      ModuleMapFileName += Directory.getName();
+      llvm::sys::path::append(ModuleMapFileName, FileName);
+      FileNameRef = ModuleMapFileName;
+    }
+    if (auto File = SourceMgr.getFileManager().getOptionalFileRef(FileNameRef))
+      Map.parseModuleMapFile(
+          *File, IsSystem,
+          Map.HeaderInfo.getHeaderSearchOpts().ModuleMapFileHomeIsCwd
+              ? Directory
+              : File->getDir(),
+          FileID(), nullptr, ExternLoc);
   }
-  if (auto File = SourceMgr.getFileManager().getOptionalFileRef(FileNameRef))
-    Map.parseModuleMapFile(
-        *File, IsSystem,
-        Map.HeaderInfo.getHeaderSearchOpts().ModuleMapFileHomeIsCwd
-            ? Directory
-            : File->getDir(),
-        FileID(), nullptr, ExternLoc);
 }
 
 /// Whether to add the requirement \p Feature to the module \p M.
@@ -2472,7 +2514,8 @@
   }
 
   bool NeedsFramework = false;
-  Map.addUnresolvedHeader(ActiveModule, std::move(Header), NeedsFramework);
+  if (!IgnoreModules)
+    Map.addUnresolvedHeader(ActiveModule, std::move(Header), NeedsFramework);
 
   if (NeedsFramework)
     Diags.Report(CurrModuleDeclLoc, diag::note_mmap_add_framework_keyword)
@@ -2547,8 +2590,9 @@
     // Sort header paths so that the pcm doesn't depend on iteration order.
     llvm::array_pod_sort(Headers.begin(), Headers.end(), compareModuleHeaders);
 
-    for (auto &Header : Headers)
-      Map.addHeader(ActiveModule, std::move(Header), ModuleMap::TextualHeader);
+    if (!IgnoreModules)
+      for (auto &Header : Headers)
+        Map.addHeader(ActiveModule, std::move(Header), ModuleMap::TextualHeader);
     return;
   }
 
@@ -2559,8 +2603,9 @@
     return;
   }
 
-  // Record this umbrella directory.
-  Map.setUmbrellaDirAsWritten(ActiveModule, *Dir, DirNameAsWritten, DirName);
+  if (!IgnoreModules)
+    // Record this umbrella directory.
+    Map.setUmbrellaDirAsWritten(ActiveModule, *Dir, DirNameAsWritten, DirName);
 }
 
 /// Parse a module export declaration.
@@ -2643,7 +2688,8 @@
   }
 
   ActiveModule->ExportAsModule = std::string(Tok.getString());
-  Map.addLinkAsDependency(ActiveModule);
+  if (!IgnoreModules)
+    Map.addLinkAsDependency(ActiveModule);
 
   consumeToken();
 }
@@ -2871,6 +2917,7 @@
     ActiveModule->InferredSubmoduleLoc = StarLoc;
     ActiveModule->InferExplicitSubmodules = Explicit;
   } else {
+    assert(!IgnoreModules);
     // We'll be inferring framework modules for this directory.
     Map.InferredDirectories[Directory].InferModules = true;
     Map.InferredDirectories[Directory].Attrs = Attrs;
@@ -2910,8 +2957,9 @@
         break;
       }
 
-      Map.InferredDirectories[Directory].ExcludedModules.push_back(
-          std::string(Tok.getString()));
+      if (!IgnoreModules)
+        Map.InferredDirectories[Directory].ExcludedModules.push_back(
+            std::string(Tok.getString()));
       consumeToken();
       break;
 
Index: clang/lib/Headers/stddef.h
===================================================================
--- clang/lib/Headers/stddef.h
+++ clang/lib/Headers/stddef.h
@@ -7,23 +7,20 @@
  *===-----------------------------------------------------------------------===
  */
 
-#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) ||        \
+    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)
 
 #if !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)
-/* Always define miscellaneous pieces when modules are available. */
-#if !__has_feature(modules)
 #define __STDDEF_H
-#endif
 #define __need_ptrdiff_t
 #define __need_size_t
 /* ISO9899:2011 7.20 (C11 Annex K): Define rsize_t if __STDC_WANT_LIB_EXT1__ is
Index: clang/lib/Headers/stdarg.h
===================================================================
--- clang/lib/Headers/stdarg.h
+++ clang/lib/Headers/stdarg.h
@@ -7,9 +7,10 @@
  *===-----------------------------------------------------------------------===
  */
 
-#if !defined(__STDARG_H) || defined(__need___va_list) ||                       \
-    defined(__need_va_list) || defined(__need_va_arg) ||                       \
-    defined(__need___va_copy) || defined(__need_va_copy)
+#if !defined(__STDARG_H) || __has_feature(modules) ||                          \
+    defined(__need___va_list) || defined(__need_va_list) ||                    \
+    defined(__need_va_arg) || defined(__need___va_copy) ||                     \
+    defined(__need_va_copy)
 
 #if !defined(__need___va_list) && !defined(__need_va_list) &&                  \
     !defined(__need_va_arg) && !defined(__need___va_copy) &&                   \
Index: clang/lib/Headers/__stddef_wint_t.h
===================================================================
--- clang/lib/Headers/__stddef_wint_t.h
+++ clang/lib/Headers/__stddef_wint_t.h
@@ -7,10 +7,9 @@
  *===-----------------------------------------------------------------------===
  */
 
-/* Always define wint_t when modules are available. */
-#if !defined(_WINT_T) || __has_feature(modules)
-#if !__has_feature(modules)
+#if !defined(_WINT_T) || __has_feature(builtin_headers_in_system_modules)
 #define _WINT_T
-#endif
+
 typedef __WINT_TYPE__ wint_t;
+
 #endif
Index: clang/lib/Headers/__stddef_wchar_t.h
===================================================================
--- clang/lib/Headers/__stddef_wchar_t.h
+++ clang/lib/Headers/__stddef_wchar_t.h
@@ -8,14 +8,16 @@
  */
 
 #if !defined(__cplusplus) || (defined(_MSC_VER) && !_NATIVE_WCHAR_T_DEFINED)
-/* Always define wchar_t when modules are available. */
-#if !defined(_WCHAR_T) || __has_feature(modules)
-#if !__has_feature(modules)
+
+#if !defined(_WCHAR_T) || __has_feature(builtin_headers_in_system_modules)
 #define _WCHAR_T
-#if defined(_MSC_EXTENSIONS)
+
+#ifdef _MSC_EXTENSIONS
 #define _WCHAR_T_DEFINED
 #endif
-#endif
+
 typedef __WCHAR_TYPE__ wchar_t;
+
 #endif
+
 #endif
Index: clang/lib/Headers/__stddef_unreachable.h
===================================================================
--- clang/lib/Headers/__stddef_unreachable.h
+++ clang/lib/Headers/__stddef_unreachable.h
@@ -7,7 +7,6 @@
  *===-----------------------------------------------------------------------===
  */
 
-#if !defined(unreachable) || __has_feature(modules)
-/* Always define unreachable when modules are available. */
+#if !defined(unreachable) || __has_feature(builtin_headers_in_system_modules)
 #define unreachable() __builtin_unreachable()
 #endif
Index: clang/lib/Headers/__stddef_size_t.h
===================================================================
--- clang/lib/Headers/__stddef_size_t.h
+++ clang/lib/Headers/__stddef_size_t.h
@@ -7,10 +7,9 @@
  *===-----------------------------------------------------------------------===
  */
 
-#if !defined(_SIZE_T) || __has_feature(modules)
-/* Always define size_t when modules are available. */
-#if !__has_feature(modules)
+#if !defined(_SIZE_T) || __has_feature(builtin_headers_in_system_modules)
 #define _SIZE_T
-#endif
+
 typedef __SIZE_TYPE__ size_t;
+
 #endif
Index: clang/lib/Headers/__stddef_rsize_t.h
===================================================================
--- clang/lib/Headers/__stddef_rsize_t.h
+++ clang/lib/Headers/__stddef_rsize_t.h
@@ -7,10 +7,9 @@
  *===-----------------------------------------------------------------------===
  */
 
-#if !defined(_RSIZE_T) || __has_feature(modules)
-/* Always define rsize_t when modules are available. */
-#if !__has_feature(modules)
+#if !defined(_RSIZE_T) || __has_feature(builtin_headers_in_system_modules)
 #define _RSIZE_T
-#endif
+
 typedef __SIZE_TYPE__ rsize_t;
+
 #endif
Index: clang/lib/Headers/__stddef_ptrdiff_t.h
===================================================================
--- clang/lib/Headers/__stddef_ptrdiff_t.h
+++ clang/lib/Headers/__stddef_ptrdiff_t.h
@@ -7,10 +7,9 @@
  *===-----------------------------------------------------------------------===
  */
 
-#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
-#endif
+
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
+
 #endif
Index: clang/lib/Headers/__stddef_offsetof.h
===================================================================
--- clang/lib/Headers/__stddef_offsetof.h
+++ clang/lib/Headers/__stddef_offsetof.h
@@ -7,7 +7,6 @@
  *===-----------------------------------------------------------------------===
  */
 
-#if !defined(offsetof) || __has_feature(modules)
-/* Always define offsetof when modules are available. */
+#if !defined(offsetof) || __has_feature(builtin_headers_in_system_modules)
 #define offsetof(t, d) __builtin_offsetof(t, d)
 #endif
Index: clang/lib/Headers/__stddef_nullptr_t.h
===================================================================
--- clang/lib/Headers/__stddef_nullptr_t.h
+++ clang/lib/Headers/__stddef_nullptr_t.h
@@ -7,11 +7,9 @@
  *===-----------------------------------------------------------------------===
  */
 
-#if !defined(_NULLPTR_T) || __has_feature(modules)
-/* Always define nullptr_t when modules are available. */
-#if !__has_feature(modules)
+#if !defined(_NULLPTR_T) || __has_feature(builtin_headers_in_system_modules)
 #define _NULLPTR_T
-#endif
+
 #ifdef __cplusplus
 #if defined(_MSC_EXTENSIONS) && defined(_NATIVE_NULLPTR_SUPPORTED)
 namespace std {
@@ -22,4 +20,5 @@
 #else
 typedef typeof(nullptr) nullptr_t;
 #endif
+
 #endif
Index: clang/lib/Headers/__stddef_max_align_t.h
===================================================================
--- clang/lib/Headers/__stddef_max_align_t.h
+++ clang/lib/Headers/__stddef_max_align_t.h
@@ -1,4 +1,4 @@
-/*===---- __stddef_max_align_t.h - Definition of max_align_t for modules ---===
+/*===---- __stddef_max_align_t.h - Definition of max_align_t ---------------===
  *
  * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
  * See https://llvm.org/LICENSE.txt for license information.
@@ -7,7 +7,8 @@
  *===-----------------------------------------------------------------------===
  */
 
-#ifndef __CLANG_MAX_ALIGN_T_DEFINED
+#if !defined(__CLANG_MAX_ALIGN_T_DEFINED) ||                                   \
+    __has_feature(builtin_headers_in_system_modules)
 #define __CLANG_MAX_ALIGN_T_DEFINED
 
 #if defined(_MSC_VER)
Index: clang/lib/Headers/__stdarg_va_list.h
===================================================================
--- clang/lib/Headers/__stdarg_va_list.h
+++ clang/lib/Headers/__stdarg_va_list.h
@@ -7,7 +7,7 @@
  *===-----------------------------------------------------------------------===
  */
 
-#ifndef _VA_LIST
+#if !defined(_VA_LIST) || __has_feature(builtin_headers_in_system_modules)
 #define _VA_LIST
 typedef __builtin_va_list va_list;
 #endif
Index: clang/lib/Headers/__stdarg_va_copy.h
===================================================================
--- clang/lib/Headers/__stdarg_va_copy.h
+++ clang/lib/Headers/__stdarg_va_copy.h
@@ -7,6 +7,6 @@
  *===-----------------------------------------------------------------------===
  */
 
-#ifndef va_copy
+#if !defined(va_copy) || __has_feature(builtin_headers_in_system_modules)
 #define va_copy(dest, src) __builtin_va_copy(dest, src)
 #endif
Index: clang/lib/Headers/__stdarg_va_arg.h
===================================================================
--- clang/lib/Headers/__stdarg_va_arg.h
+++ clang/lib/Headers/__stdarg_va_arg.h
@@ -7,7 +7,7 @@
  *===-----------------------------------------------------------------------===
  */
 
-#ifndef va_arg
+#if !defined(va_arg) || __has_feature(builtin_headers_in_system_modules)
 
 #if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L
 /* C23 does not require the second parameter for va_start. */
Index: clang/lib/Headers/__stdarg___va_copy.h
===================================================================
--- clang/lib/Headers/__stdarg___va_copy.h
+++ clang/lib/Headers/__stdarg___va_copy.h
@@ -7,6 +7,6 @@
  *===-----------------------------------------------------------------------===
  */
 
-#ifndef __va_copy
+#if !defined(__va_copy) || __has_feature(builtin_headers_in_system_modules)
 #define __va_copy(d, s) __builtin_va_copy(d, s)
 #endif
Index: clang/lib/Headers/__stdarg___gnuc_va_list.h
===================================================================
--- clang/lib/Headers/__stdarg___gnuc_va_list.h
+++ clang/lib/Headers/__stdarg___gnuc_va_list.h
@@ -7,7 +7,7 @@
  *===-----------------------------------------------------------------------===
  */
 
-#ifndef __GNUC_VA_LIST
+#if !defined(__GNUC_VA_LIST) || __has_feature(builtin_headers_in_system_modules)
 #define __GNUC_VA_LIST
 typedef __builtin_va_list __gnuc_va_list;
 #endif
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2846,6 +2846,25 @@
   return TargetVersion < alignedAllocMinVersion(OS);
 }
 
+static bool sdkSupportsBuiltinModules(const Darwin::DarwinPlatformKind &TargetPlatform, const std::optional<DarwinSDKInfo> &SDKInfo) {
+  if (!SDKInfo)
+    return false;
+  
+  VersionTuple SDKVersion = SDKInfo->getVersion();
+  switch (TargetPlatform) {
+  case Darwin::MacOS:
+    return SDKVersion >= VersionTuple(100U);
+  case Darwin::IPhoneOS:
+    return SDKVersion >= VersionTuple(100U);
+  case Darwin::TvOS:
+    return SDKVersion >= VersionTuple(100U);
+  case Darwin::WatchOS:
+    return SDKVersion >= VersionTuple(100U);
+  case Darwin::DriverKit:
+    return SDKVersion >= VersionTuple(100U);
+  }
+}
+
 void Darwin::addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
                                    llvm::opt::ArgStringList &CC1Args,
                                    Action::OffloadKind DeviceOffloadKind) const {
@@ -2868,6 +2887,20 @@
           options::OPT_fvisibility_inlines_hidden_static_local_var,
           options::OPT_fno_visibility_inlines_hidden_static_local_var))
     CC1Args.push_back("-fvisibility-inlines-hidden-static-local-var");
+  
+  // Earlier versions of the darwin SDK have the C standard library headers
+  // all together in the Darwin module. That leads to module cycles with
+  // the _Builtin_ modules. e.g. <inttypes.h> on darwin includes <stdint.h>.
+  // The builtin <stdint.h> include-nexts <stdint.h>. When both of those
+  // darwin headers are in the Darwin module, there's a module cycle Darwin ->
+  // _Builtin_stdint -> Darwin (i.e. inttypes.h (darwin) -> stdint.h (builtin) ->
+  // stdint.h (darwin)). This is fixed in later versions of the darwin SDK,
+  // but until then, the builtin headers need to join the system modules.
+  // i.e. when the builtin stdint.h is in the Darwin module too, the cycle
+  // goes away. Note that -fbuiltin-headers-in-system-modules does nothing
+  // to fix the same problem with C++ headers, and is generally fragile.
+  if (!sdkSupportsBuiltinModules(TargetPlatform, SDKInfo))
+    CC1Args.push_back("-fbuiltin-headers-in-system-modules");
 }
 
 void Darwin::addClangCC1ASTargetOptions(
Index: clang/include/clang/Lex/ModuleMap.h
===================================================================
--- clang/include/clang/Lex/ModuleMap.h
+++ clang/include/clang/Lex/ModuleMap.h
@@ -415,7 +415,6 @@
   }
 
   /// Is this a compiler builtin header?
-  static bool isBuiltinHeader(StringRef FileName);
   bool isBuiltinHeader(const FileEntry *File);
 
   /// Add a module map callback.
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2941,6 +2941,11 @@
           "Enable the 'modules' language feature">,
   NegFlag<SetFalse>, BothFlags<
           [NoXarchOption], [ClangOption, CLOption]>>;
+def fbuiltin_headers_in_system_modules : Flag <["-"], "fbuiltin-headers-in-system-modules">,
+  Group<f_Group>,
+  Flags<[NoXarchOption]>,
+  Visibility<[CC1Option]>,
+  MarshallingInfoFlag<LangOpts<"BuiltinHeadersInSystemModules">>;
 def fmodule_maps : Flag <["-"], "fmodule-maps">,
   Visibility<[ClangOption, CLOption]>, Alias<fimplicit_module_maps>;
 def fmodule_name_EQ : Joined<["-"], "fmodule-name=">, Group<f_Group>,
Index: clang/include/clang/Basic/LangOptions.def
===================================================================
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -173,6 +173,7 @@
 BENIGN_LANGOPT(HeinousExtensions , 1, 0, "extensions that we really don't like and may be ripped out at any time")
 LANGOPT(Modules           , 1, 0, "modules semantics")
 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,
                     "compiling a module interface")
 BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
Index: clang/include/clang/Basic/Features.def
===================================================================
--- clang/include/clang/Basic/Features.def
+++ clang/include/clang/Basic/Features.def
@@ -230,6 +230,7 @@
 FEATURE(is_union, LangOpts.CPlusPlus)
 FEATURE(kcfi, LangOpts.Sanitize.has(SanitizerKind::KCFI))
 FEATURE(modules, LangOpts.Modules)
+FEATURE(builtin_headers_in_system_modules, LangOpts.BuiltinHeadersInSystemModules)
 FEATURE(safe_stack, LangOpts.Sanitize.has(SanitizerKind::SafeStack))
 FEATURE(shadow_call_stack,
         LangOpts.Sanitize.has(SanitizerKind::ShadowCallStack))
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to