Merge pull request swiftlang#7618 from ian-twilightcoder/builtin-flag

[cherry-pick stable/20230725] [Modules] Add a flag to control builtin headers being in system modules

Author: ian-twilightcoder

clang/include/clang/Basic/Features.def

@@ -285,6 +285,7 @@ EXTENSION(matrix_types, LangOpts.MatrixTypes)
 EXTENSION(matrix_types_scalar_division, true)
 EXTENSION(cxx_attributes_on_using_declarations, LangOpts.CPlusPlus11)
 
+FEATURE(builtin_headers_in_system_modules, LangOpts.BuiltinHeadersInSystemModules)
 FEATURE(cxx_abi_relative_vtable, LangOpts.CPlusPlus && LangOpts.RelativeCXXABIVTables)
 
 // CUDA/HIP Features

clang/include/clang/Basic/LangOptions.def

@@ -184,6 +184,7 @@ LANGOPT(MathErrno         , 1, 1, "errno in math functions")
 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")

clang/include/clang/Driver/Options.td

@@ -2597,6 +2597,12 @@ defm modules : BoolFOption<"modules",
   LangOpts<"Modules">, Default<fcxx_modules.KeyPath>,
   PosFlag<SetTrue, [CC1Option], "Enable the 'modules' language feature">,
   NegFlag<SetFalse>, BothFlags<[NoXarchOption, CoreOption]>>;
+def fbuiltin_headers_in_system_modules : Flag <["-"], "fbuiltin-headers-in-system-modules">,
+  Group<f_Group>,
+  Flags<[NoXarchOption,CC1Option]>,
+  ShouldParseIf<fmodules.KeyPath>,
+  HelpText<"builtin headers belong to system modules, and _Builtin_ modules are ignored for cstdlib headers">,
+  MarshallingInfoFlag<LangOpts<"BuiltinHeadersInSystemModules">>;
 def fmodule_maps : Flag <["-"], "fmodule-maps">, Flags<[CoreOption]>, Alias<fimplicit_module_maps>;
 def fmodule_name_EQ : Joined<["-"], "fmodule-name=">, Group<f_Group>,
   Flags<[NoXarchOption,CC1Option,CoreOption]>, MetaVarName<"<name>">,

clang/include/clang/Lex/ModuleMap.h

@@ -426,7 +426,6 @@ class ModuleMap {
   }
 
   /// Is this a compiler builtin header?
-  static bool isBuiltinHeader(StringRef FileName);
   bool isBuiltinHeader(const FileEntry *File);
 
   /// Add a module map callback.

clang/lib/Driver/ToolChains/Darwin.cpp

@@ -3023,6 +3023,25 @@ bool Darwin::isAlignedAllocationUnavailable() const {
   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(99U);
+  case Darwin::IPhoneOS:
+    return SDKVersion >= VersionTuple(99U);
+  case Darwin::TvOS:
+    return SDKVersion >= VersionTuple(99U);
+  case Darwin::WatchOS:
+    return SDKVersion >= VersionTuple(99U);
+  default:
+    return true;
+  }
+}
+
 void Darwin::addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
                                    llvm::opt::ArgStringList &CC1Args,
                                    Action::OffloadKind DeviceOffloadKind) const {
@@ -3045,6 +3064,20 @@ void Darwin::addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
           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(

clang/lib/Lex/ModuleMap.cpp

@@ -253,6 +253,45 @@ OptionalFileEntryRef ModuleMap::findHeader(
   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  on some platforms.
+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 @@ bool ModuleMap::resolveAsBuiltinHeader(
       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 @@ static StringRef sanitizeFilenameAsIdentifier(StringRef Name,
   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
@@ -2484,7 +2504,10 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken,
   }
 
   bool NeedsFramework = false;
-  Map.addUnresolvedHeader(ActiveModule, std::move(Header), NeedsFramework);
+  // Don't add the top level headers to the builtin modules if the builtin headers
+  // belong to the system modules.
+  if (!Map.LangOpts.BuiltinHeadersInSystemModules || ActiveModule->isSubModule() || !isBuiltInModuleName(ActiveModule->Name))
+    Map.addUnresolvedHeader(ActiveModule, std::move(Header), NeedsFramework);
 
   if (NeedsFramework)
     Diags.Report(CurrModuleDeclLoc, diag::note_mmap_add_framework_keyword)

clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json

@@ -0,0 +1 @@
+{"Version":"990.0", "MaximumDeploymentTarget": "99.0.99"}

clang/test/Driver/darwin-builtin-modules.c

@@ -0,0 +1,27 @@
+// Check that darwin passes -fbuiltin-headers-in-system-modules
+// when expected.
+
+// RUN: %clang -target x86_64-apple-darwin22.4 -### %s 2>&1 | FileCheck %s
+// RUN: %clang -isysroot %S/Inputs/MacOSX10.15.versioned.sdk -target x86_64-apple-macos10.15 -### %s 2>&1 | FileCheck %s
+// RUN: %clang -isysroot %S/Inputs/iPhoneOS13.0.sdk -target arm64-apple-ios13.0 -### %s 2>&1 | FileCheck %s
+// CHECK: -fbuiltin-headers-in-system-modules
+
+// RUN: %clang -isysroot %S/Inputs/MacOSX99.0.sdk -target x86_64-apple-macos98.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
+// RUN: %clang -isysroot %S/Inputs/MacOSX99.0.sdk -target x86_64-apple-macos99.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
+// CHECK_FUTURE-NOT: -fbuiltin-headers-in-system-modules
+
+
+// Check that builtin_headers_in_system_modules is only set if -fbuiltin-headers-in-system-modules and -fmodules are both set.
+
+// RUN: %clang -isysroot %S/Inputs/iPhoneOS13.0.sdk -target arm64-apple-ios13.0 -fsyntax-only %s -Xclang -verify=no-feature
+// RUN: %clang -isysroot %S/Inputs/iPhoneOS13.0.sdk -target arm64-apple-ios13.0 -fsyntax-only %s -fmodules -Xclang -verify=yes-feature
+// RUN: %clang -isysroot %S/Inputs/MacOSX99.0.sdk -target x86_64-apple-macos99.0 -fsyntax-only %s -Xclang -verify=no-feature
+// RUN: %clang -isysroot %S/Inputs/MacOSX99.0.sdk -target x86_64-apple-macos99.0 -fsyntax-only %s -fmodules -Xclang -verify=no-feature
+
+#if __has_feature(builtin_headers_in_system_modules)
+#error "has builtin_headers_in_system_modules"
+// yes-feature-error@-1 {{}}
+#else
+#error "no builtin_headers_in_system_modules"
+// no-feature-error@-1 {{}}
+#endif

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

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