[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
This commit is contained in:
Ian Anderson
2023-09-06 22:24:40 -07:00
parent 6b03d8aa94
commit 0ea3d88bdb
16 changed files with 148 additions and 51 deletions

View File

@@ -278,6 +278,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

View File

@@ -173,6 +173,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")

View File

@@ -2952,6 +2952,13 @@ defm modules : BoolFOption<"modules",
"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]>,
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">,
Visibility<[ClangOption, CLOption]>, Alias<fimplicit_module_maps>;
def fmodule_name_EQ : Joined<["-"], "fmodule-name=">, Group<f_Group>,

View File

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

View File

@@ -2843,6 +2843,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 {
@@ -2865,6 +2884,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(

View File

@@ -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
@@ -2472,7 +2492,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)

View File

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

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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 \

View File

@@ -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;

View File

@@ -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; }

View File

@@ -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'}}

View File

@@ -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"

View File

@@ -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