From 6f98c14ef4ea08044247f753f3c000f6be3bf5a6 Mon Sep 17 00:00:00 2001 From: Ian Anderson Date: Wed, 6 Sep 2023 22:24:40 -0700 Subject: [PATCH 1/4] [Modules] Add a flag to control builtin headers being in system modules 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. Reviewed By: ChuanqiXu, Bigcheese, benlangmuir Differential Revision: https://reviews.llvm.org/D159483 (cherry picked from commit 0ea3d88bdb16aa6e9a0043cc3ed93dcb88a89dea) --- clang/include/clang/Basic/Features.def | 1 + clang/include/clang/Basic/LangOptions.def | 1 + clang/include/clang/Driver/Options.td | 6 ++ clang/include/clang/Lex/ModuleMap.h | 1 - clang/lib/Driver/ToolChains/Darwin.cpp | 33 +++++++++ clang/lib/Lex/ModuleMap.cpp | 69 ++++++++++++------- .../Inputs/MacOSX99.0.sdk/SDKSettings.json | 1 + clang/test/Driver/darwin-builtin-modules.c | 27 ++++++++ clang/test/Modules/Werror-Wsystem-headers.m | 16 +++-- clang/test/Modules/context-hash.c | 23 ++++--- clang/test/Modules/crash-vfs-include-pch.m | 8 +-- clang/test/Modules/cstd.m | 2 +- clang/test/Modules/pch-used.m | 4 +- clang/test/Modules/shadowed-submodule.m | 2 +- clang/test/Modules/stddef.c | 2 +- clang/test/Modules/stddef.m | 2 +- 16 files changed, 147 insertions(+), 51 deletions(-) create mode 100644 clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json create mode 100644 clang/test/Driver/darwin-builtin-modules.c diff --git a/clang/include/clang/Basic/Features.def b/clang/include/clang/Basic/Features.def index 3f665890e81d7..2edd026c1091e 100644 --- a/clang/include/clang/Basic/Features.def +++ b/clang/include/clang/Basic/Features.def @@ -279,6 +279,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 diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index 653efd362c530..87e29c402f837 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -184,6 +184,7 @@ BENIGN_LANGOPT(HeinousExtensions , 1, 0, "extensions that we really don't like a LANGOPT(Modules , 1, 0, "modules semantics") COMPATIBLE_LANGOPT(ModulesTS , 1, 0, "C++ Modules TS syntax") 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") diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index b05ffd493b4d8..321b3d8ea750f 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -2407,6 +2407,12 @@ defm modules : BoolFOption<"modules", LangOpts<"Modules">, Default, PosFlag, NegFlag, BothFlags<[NoXarchOption, CoreOption]>>; +def fbuiltin_headers_in_system_modules : Flag <["-"], "fbuiltin-headers-in-system-modules">, + Group, + Flags<[NoXarchOption,CC1Option]>, + ShouldParseIf, + HelpText<"builtin headers belong to system modules, and _Builtin_ modules are ignored for cstdlib headers">, + MarshallingInfoFlag>; def fmodule_maps : Flag <["-"], "fmodule-maps">, Flags<[CoreOption]>, Alias; def fmodule_name_EQ : Joined<["-"], "fmodule-name=">, Group, Flags<[NoXarchOption,CC1Option,CoreOption]>, MetaVarName<"">, diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h index 90ac84d3a4cd9..178906a753b5f 100644 --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/include/clang/Lex/ModuleMap.h @@ -427,7 +427,6 @@ class ModuleMap { } /// Is this a compiler builtin header? - static bool isBuiltinHeader(StringRef FileName); bool isBuiltinHeader(const FileEntry *File); /// Add a module map callback. diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index d0e61c213f9c1..ae44b5d740f1b 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2984,6 +2984,25 @@ bool Darwin::isAlignedAllocationUnavailable() const { return TargetVersion < alignedAllocMinVersion(OS); } +static bool sdkSupportsBuiltinModules(const Darwin::DarwinPlatformKind &TargetPlatform, const Optional &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 { @@ -3055,6 +3074,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. on darwin includes . + // The builtin include-nexts . 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"); } DerivedArgList * diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index 5e67c6901bdd9..665bb94a66f23 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -253,6 +253,45 @@ Optional 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(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(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) { @@ -296,7 +335,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 @@ -372,28 +411,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(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 @@ -2513,7 +2533,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 && ActiveModule) Diags.Report(CurrModuleDeclLoc, diag::note_mmap_add_framework_keyword) diff --git a/clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json b/clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json new file mode 100644 index 0000000000000..77b70e1a83c19 --- /dev/null +++ b/clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json @@ -0,0 +1 @@ +{"Version":"990.0", "MaximumDeploymentTarget": "99.0.99"} diff --git a/clang/test/Driver/darwin-builtin-modules.c b/clang/test/Driver/darwin-builtin-modules.c new file mode 100644 index 0000000000000..215f2b8d2c142 --- /dev/null +++ b/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 diff --git a/clang/test/Modules/Werror-Wsystem-headers.m b/clang/test/Modules/Werror-Wsystem-headers.m index 164c5b2ab672a..44633ef430b99 100644 --- a/clang/test/Modules/Werror-Wsystem-headers.m +++ b/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 diff --git a/clang/test/Modules/context-hash.c b/clang/test/Modules/context-hash.c index 8bb7422f6a54f..de3303a9b0b64 100644 --- a/clang/test/Modules/context-hash.c +++ b/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 diff --git a/clang/test/Modules/crash-vfs-include-pch.m b/clang/test/Modules/crash-vfs-include-pch.m index 5289f184eb04c..4c05a07d0f74d 100644 --- a/clang/test/Modules/crash-vfs-include-pch.m +++ b/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 \ diff --git a/clang/test/Modules/cstd.m b/clang/test/Modules/cstd.m index f07ad85ec5126..6b81b9013e9da 100644 --- a/clang/test/Modules/cstd.m +++ b/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; diff --git a/clang/test/Modules/pch-used.m b/clang/test/Modules/pch-used.m index 31d358391ce7f..5f6ffec337a51 100644 --- a/clang/test/Modules/pch-used.m +++ b/clang/test/Modules/pch-used.m @@ -1,8 +1,8 @@ // UNSUPPORTED: -zos, -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; } diff --git a/clang/test/Modules/shadowed-submodule.m b/clang/test/Modules/shadowed-submodule.m index c9c77bd13a1ca..6f34303d953e2 100644 --- a/clang/test/Modules/shadowed-submodule.m +++ b/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'}} diff --git a/clang/test/Modules/stddef.c b/clang/test/Modules/stddef.c index c33f3643112bb..c164964c6ffa3 100644 --- a/clang/test/Modules/stddef.c +++ b/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" diff --git a/clang/test/Modules/stddef.m b/clang/test/Modules/stddef.m index 4f2d6bb7407b3..1ff6ff73796ab 100644 --- a/clang/test/Modules/stddef.m +++ b/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 From f8e3601f52f99496da9a3761d682662f3f8bcf5b Mon Sep 17 00:00:00 2001 From: Ian Anderson Date: Wed, 13 Dec 2023 10:36:03 -0800 Subject: [PATCH 2/4] Remove the builtin_headers_in_system_modules feature (#75262) __has_feature(builtin_headers_in_system_modules) was added in https://reviews.llvm.org/D159483 to be used in the stdarg/stddef implementation headers. It ended up being unnecessary, but I forgot to remove the feature definition. (cherry picked from commit 6d18951b833b2a2dabe268b3be0163e03a9e1142) --- clang/include/clang/Basic/Features.def | 1 - clang/test/Driver/darwin-builtin-modules.c | 16 ---------------- 2 files changed, 17 deletions(-) diff --git a/clang/include/clang/Basic/Features.def b/clang/include/clang/Basic/Features.def index 2edd026c1091e..3f665890e81d7 100644 --- a/clang/include/clang/Basic/Features.def +++ b/clang/include/clang/Basic/Features.def @@ -279,7 +279,6 @@ 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 diff --git a/clang/test/Driver/darwin-builtin-modules.c b/clang/test/Driver/darwin-builtin-modules.c index 215f2b8d2c142..1c56e13bfb929 100644 --- a/clang/test/Driver/darwin-builtin-modules.c +++ b/clang/test/Driver/darwin-builtin-modules.c @@ -9,19 +9,3 @@ // 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 From 17d8e65f7e9d52e4978ef8ba1b0618f0d3f2fd8a Mon Sep 17 00:00:00 2001 From: Ian Anderson Date: Wed, 7 Aug 2024 10:14:58 -0700 Subject: [PATCH 3/4] [clang][modules] Enable built-in modules for the upcoming Apple releases (#102239) The upcoming Apple SDK releases will support the clang built-in headers being in the clang built-in modules: stop passing -fbuiltin-headers-in-system-modules for those SDK versions. (cherry picked from commit 961639962251de7428c3fe93fa17cfa6ab3c561a) --- clang/lib/Driver/ToolChains/Darwin.cpp | 37 ++++++++++++++++--- .../Inputs/DriverKit23.0.sdk/SDKSettings.json | 1 + .../SDKSettings.json | 0 clang/test/Driver/darwin-builtin-modules.c | 5 ++- 4 files changed, 35 insertions(+), 8 deletions(-) create mode 100644 clang/test/Driver/Inputs/DriverKit23.0.sdk/SDKSettings.json rename clang/test/Driver/Inputs/{MacOSX99.0.sdk => MacOSX15.0.sdk}/SDKSettings.json (100%) diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index ae44b5d740f1b..b696d25ac7c1a 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -2984,20 +2984,45 @@ bool Darwin::isAlignedAllocationUnavailable() const { return TargetVersion < alignedAllocMinVersion(OS); } -static bool sdkSupportsBuiltinModules(const Darwin::DarwinPlatformKind &TargetPlatform, const Optional &SDKInfo) { +static bool sdkSupportsBuiltinModules( + const Darwin::DarwinPlatformKind &TargetPlatform, + const Darwin::DarwinEnvironmentKind &TargetEnvironment, + const Optional &SDKInfo) { + switch (TargetEnvironment) { + case Darwin::NativeEnvironment: + case Darwin::Simulator: + case Darwin::MacCatalyst: + // Standard xnu/Mach/Darwin based environments + // depend on the SDK version. + break; + default: + // All other environments support builtin modules from the start. + return true; + } + if (!SDKInfo) + // If there is no SDK info, assume this is building against a + // pre-SDK version of macOS (i.e. before Mac OS X 10.4). Those + // don't support modules anyway, but the headers definitely + // don't support builtin modules either. It might also be some + // kind of degenerate build environment, err on the side of + // the old behavior which is to not use builtin modules. return false; VersionTuple SDKVersion = SDKInfo->getVersion(); switch (TargetPlatform) { + // Existing SDKs added support for builtin modules in the fall + // 2024 major releases. case Darwin::MacOS: - return SDKVersion >= VersionTuple(99U); + return SDKVersion >= VersionTuple(15U); case Darwin::IPhoneOS: - return SDKVersion >= VersionTuple(99U); + return SDKVersion >= VersionTuple(18U); case Darwin::TvOS: - return SDKVersion >= VersionTuple(99U); + return SDKVersion >= VersionTuple(18U); case Darwin::WatchOS: - return SDKVersion >= VersionTuple(99U); + return SDKVersion >= VersionTuple(11U); + + // New SDKs support builtin modules from the start. default: return true; } @@ -3086,7 +3111,7 @@ void Darwin::addClangTargetOptions(const llvm::opt::ArgList &DriverArgs, // 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)) + if (!sdkSupportsBuiltinModules(TargetPlatform, TargetEnvironment, SDKInfo)) CC1Args.push_back("-fbuiltin-headers-in-system-modules"); } diff --git a/clang/test/Driver/Inputs/DriverKit23.0.sdk/SDKSettings.json b/clang/test/Driver/Inputs/DriverKit23.0.sdk/SDKSettings.json new file mode 100644 index 0000000000000..7ba6c244df211 --- /dev/null +++ b/clang/test/Driver/Inputs/DriverKit23.0.sdk/SDKSettings.json @@ -0,0 +1 @@ +{"Version":"23.0", "MaximumDeploymentTarget": "23.0.99"} diff --git a/clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json b/clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json similarity index 100% rename from clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json rename to clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json diff --git a/clang/test/Driver/darwin-builtin-modules.c b/clang/test/Driver/darwin-builtin-modules.c index 1c56e13bfb929..ec515133be8ab 100644 --- a/clang/test/Driver/darwin-builtin-modules.c +++ b/clang/test/Driver/darwin-builtin-modules.c @@ -6,6 +6,7 @@ // 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 +// RUN: %clang -isysroot %S/Inputs/MacOSX15.0.sdk -target x86_64-apple-macos14.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s +// RUN: %clang -isysroot %S/Inputs/MacOSX15.0.sdk -target x86_64-apple-macos15.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s +// RUN: %clang -isysroot %S/Inputs/DriverKit23.0.sdk -target arm64-apple-driverkit23.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s // CHECK_FUTURE-NOT: -fbuiltin-headers-in-system-modules From 1c1f422fd25344cd5a0161e7d9fe71af8c1d1ddb Mon Sep 17 00:00:00 2001 From: Ian Anderson Date: Tue, 20 Aug 2024 03:29:11 -0700 Subject: [PATCH 4/4] [clang][modules] Built-in modules are not correctly enabled for Mac Catalyst (#104872) Mac Catalyst is the iOS platform, but it builds against the macOS SDK and so it needs to be checking the macOS SDK version instead of the iOS one. Add tests against a greater-than SDK version just to make sure this works beyond the initially supporting SDKs. (cherry picked from commit b9864387d9d00e1d4888181460d05dbc92364d75) --- clang/lib/Driver/ToolChains/Darwin.cpp | 10 +++++++++- .../test/Driver/Inputs/MacOSX15.1.sdk/SDKSettings.json | 1 + clang/test/Driver/darwin-builtin-modules.c | 3 +++ 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 clang/test/Driver/Inputs/MacOSX15.1.sdk/SDKSettings.json diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index b696d25ac7c1a..83d6e8b665767 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -3016,7 +3016,15 @@ static bool sdkSupportsBuiltinModules( case Darwin::MacOS: return SDKVersion >= VersionTuple(15U); case Darwin::IPhoneOS: - return SDKVersion >= VersionTuple(18U); + switch (TargetEnvironment) { + case Darwin::MacCatalyst: + // Mac Catalyst uses `-target arm64-apple-ios18.0-macabi` so the platform + // is iOS, but it builds with the macOS SDK, so it's the macOS SDK version + // that's relevant. + return SDKVersion >= VersionTuple(15U); + default: + return SDKVersion >= VersionTuple(18U); + } case Darwin::TvOS: return SDKVersion >= VersionTuple(18U); case Darwin::WatchOS: diff --git a/clang/test/Driver/Inputs/MacOSX15.1.sdk/SDKSettings.json b/clang/test/Driver/Inputs/MacOSX15.1.sdk/SDKSettings.json new file mode 100644 index 0000000000000..d46295b2ab5a1 --- /dev/null +++ b/clang/test/Driver/Inputs/MacOSX15.1.sdk/SDKSettings.json @@ -0,0 +1 @@ +{"Version":"15.1", "MaximumDeploymentTarget": "15.1.99"} diff --git a/clang/test/Driver/darwin-builtin-modules.c b/clang/test/Driver/darwin-builtin-modules.c index ec515133be8ab..4564d7317d7ab 100644 --- a/clang/test/Driver/darwin-builtin-modules.c +++ b/clang/test/Driver/darwin-builtin-modules.c @@ -8,5 +8,8 @@ // RUN: %clang -isysroot %S/Inputs/MacOSX15.0.sdk -target x86_64-apple-macos14.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s // RUN: %clang -isysroot %S/Inputs/MacOSX15.0.sdk -target x86_64-apple-macos15.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s +// RUN: %clang -isysroot %S/Inputs/MacOSX15.0.sdk -target x86_64-apple-ios18.0-macabi -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s +// RUN: %clang -isysroot %S/Inputs/MacOSX15.1.sdk -target x86_64-apple-macos15.1 -darwin-target-variant x86_64-apple-ios18.1-macabi -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s +// RUN: %clang -isysroot %S/Inputs/MacOSX15.1.sdk -target x86_64-apple-ios18.1-macabi -darwin-target-variant x86_64-apple-macos15.1 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s // RUN: %clang -isysroot %S/Inputs/DriverKit23.0.sdk -target arm64-apple-driverkit23.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s // CHECK_FUTURE-NOT: -fbuiltin-headers-in-system-modules