https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/101778
>From 3d7c2b87ec73a48de30e1c5387a7f0d8d817b0f4 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Fri, 2 Aug 2024 23:38:18 +0100
Subject: [PATCH 1/6] [lldb][ClangExpressionParser] Set
BuiltinHeadersInSystemModules depending on SDK version
This patch addresses an issue that will arise with future
SDKs. The ClangExpressionParser currently unconditionally
sets `-fbuiltin-headers-in-system-modules` when evaluating
expressions with the `target.import-std-module` setting.
This flag should, however, be set depending on the SDK
version (which is what the Clang Darwin toolchain does).
Unfortunately, the compiler instance that we create with
`ClangExpressionParser` never consults the Clang driver,
and thus doesn't correctly infer `BuiltinHeadersInSystemModules`.
Note, this isn't an issue with the `CompilerInstance` that the
`ClangModulesDeclVendor` creates because it uses the `createInovcation`
API, which calls into `Darwin::addClangTargetOptions`.
This patch mimicks how `sdkSupportsBuiltinModules` is used in
`Darwin::addClangTargetOptions`.
This ensures that the `import-std-module` API tests run cleanly
regardless of SDK version.
The plan is to make the `CompilerInstance` construction in
`ClangExpressionParser` go through the driver, so we can avoid
duplicating the logic in LLDB.
---
.../Clang/ClangExpressionParser.cpp | 69 ++++++++++++++++++-
1 file changed, 66 insertions(+), 3 deletions(-)
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 2a8bdf29314e4..c6217e2f13394 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/ExternalASTSource.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/Basic/Builtins.h"
+#include "clang/Basic/DarwinSDKInfo.h"
#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/TargetInfo.h"
@@ -561,7 +562,62 @@ static void SetupLangOpts(CompilerInstance &compiler,
lang_opts.NoBuiltin = true;
}
-static void SetupImportStdModuleLangOpts(CompilerInstance &compiler) {
+// NOTE: should be kept in sync with sdkSupportsBuiltinModules in
+// Toolchains/Darwin.cpp
+static bool
+sdkSupportsBuiltinModulesImpl(const llvm::Triple &triple,
+ const std::optional<DarwinSDKInfo> &SDKInfo) {
+ if (!SDKInfo)
+ return false;
+
+ VersionTuple SDKVersion = SDKInfo->getVersion();
+ switch (triple.getOS()) {
+ case Triple::OSType::MacOSX:
+ return SDKVersion >= VersionTuple(15U);
+ case Triple::OSType::IOS:
+ return SDKVersion >= VersionTuple(18U);
+ case Triple::OSType::TvOS:
+ return SDKVersion >= VersionTuple(18U);
+ case Triple::OSType::WatchOS:
+ return SDKVersion >= VersionTuple(11U);
+ case Triple::OSType::XROS:
+ return SDKVersion >= VersionTuple(2U);
+ default:
+ // New SDKs support builtin modules from the start.
+ return true;
+ }
+}
+
+static bool
+sdkSupportsBuiltinModules(llvm::Triple const &triple,
+ std::vector<std::string> const &include_dirs) {
+ static constexpr std::string_view s_sdk_suffix = ".sdk";
+ auto it = llvm::find_if(include_dirs, [](std::string const &path) {
+ return path.find(s_sdk_suffix) != std::string::npos;
+ });
+ if (it == include_dirs.end())
+ return false;
+
+ size_t suffix = it->find(s_sdk_suffix);
+ if (suffix == std::string::npos)
+ return false;
+
+ auto VFS = FileSystem::Instance().GetVirtualFileSystem();
+ if (!VFS)
+ return false;
+
+ std::string sdk_path = it->substr(0, suffix + s_sdk_suffix.size());
+ auto parsed = clang::parseDarwinSDKInfo(*VFS, sdk_path);
+ if (!parsed)
+ return false;
+
+ return sdkSupportsBuiltinModulesImpl(triple, *parsed);
+}
+
+static void
+SetupImportStdModuleLangOpts(CompilerInstance &compiler,
+ llvm::Triple const &triple,
+ std::vector<std::string> const &include_dirs) {
LangOptions &lang_opts = compiler.getLangOpts();
lang_opts.Modules = true;
// We want to implicitly build modules.
@@ -578,7 +634,12 @@ static void SetupImportStdModuleLangOpts(CompilerInstance
&compiler) {
lang_opts.GNUMode = true;
lang_opts.GNUKeywords = true;
lang_opts.CPlusPlus11 = true;
- lang_opts.BuiltinHeadersInSystemModules = true;
+
+ // FIXME: We should use the driver to derive this for use.
+ // ClangModulesDeclVendor already parses the SDKSettings for the purposes of
+ // this check.
+ lang_opts.BuiltinHeadersInSystemModules =
+ !sdkSupportsBuiltinModules(triple, include_dirs);
// The Darwin libc expects this macro to be set.
lang_opts.GNUCVersion = 40201;
@@ -663,7 +724,9 @@ ClangExpressionParser::ClangExpressionParser(
if (auto *clang_expr = dyn_cast<ClangUserExpression>(&m_expr);
clang_expr && clang_expr->DidImportCxxModules()) {
LLDB_LOG(log, "Adding lang options for importing C++ modules");
- SetupImportStdModuleLangOpts(*m_compiler);
+ SetupImportStdModuleLangOpts(*m_compiler,
+ target_sp->GetArchitecture().GetTriple(),
+ m_include_directories);
SetupModuleHeaderPaths(m_compiler.get(), m_include_directories, target_sp);
}
>From 664059ce3118296c8b575d84df2fff40104a8eda Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Sat, 3 Aug 2024 01:12:27 +0100
Subject: [PATCH 2/6] fixup! fix comment
---
.../Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index c6217e2f13394..5deb6a782481c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -635,7 +635,7 @@ SetupImportStdModuleLangOpts(CompilerInstance &compiler,
lang_opts.GNUKeywords = true;
lang_opts.CPlusPlus11 = true;
- // FIXME: We should use the driver to derive this for use.
+ // FIXME: We should use the driver to derive this for us.
// ClangModulesDeclVendor already parses the SDKSettings for the purposes of
// this check.
lang_opts.BuiltinHeadersInSystemModules =
>From eb2f21333ff6d7bd6881e14fdb275ebb9572eef2 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Sat, 3 Aug 2024 17:44:41 +0100
Subject: [PATCH 3/6] fixup!
---
.../Clang/ClangExpressionParser.cpp | 27 +++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 5deb6a782481c..69c23e43c7fad 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -588,9 +588,23 @@ sdkSupportsBuiltinModulesImpl(const llvm::Triple &triple,
}
}
+/// Returns true if the SDK for the specified triple supports
+/// builtin modules in system headers. This is used to decide
+/// whether to pass -fbuiltin-headers-in-system-modules to
+/// the compiler instance when compiling the `std` module.
+///
+/// This function assumes one of the directories in \ref include_dirs
+/// is an SDK path that exists on the host. The SDK version is
+/// read from the SDKSettings.json in that directory.
+///
+/// \param[in] triple The target triple.
+/// \param[in] include_dirs The include directories the compiler will use
+/// during header search.
+///
static bool
sdkSupportsBuiltinModules(llvm::Triple const &triple,
std::vector<std::string> const &include_dirs) {
+ // Find an SDK directory.
static constexpr std::string_view s_sdk_suffix = ".sdk";
auto it = llvm::find_if(include_dirs, [](std::string const &path) {
return path.find(s_sdk_suffix) != std::string::npos;
@@ -599,14 +613,16 @@ sdkSupportsBuiltinModules(llvm::Triple const &triple,
return false;
size_t suffix = it->find(s_sdk_suffix);
- if (suffix == std::string::npos)
- return false;
+ assert (suffix == std::string::npos);
auto VFS = FileSystem::Instance().GetVirtualFileSystem();
if (!VFS)
return false;
+ // Extract: /path/to/some.sdk
std::string sdk_path = it->substr(0, suffix + s_sdk_suffix.size());
+
+ // Extract SDK version from the /path/to/some.sdk/SDKSettings.json
auto parsed = clang::parseDarwinSDKInfo(*VFS, sdk_path);
if (!parsed)
return false;
@@ -614,6 +630,13 @@ sdkSupportsBuiltinModules(llvm::Triple const &triple,
return sdkSupportsBuiltinModulesImpl(triple, *parsed);
}
+/// Sets the LangOptions to prepare for running with `import-std-module`
enabled.
+///
+/// \param[out] compiler CompilerInstance on which to set the LangOptions.
+/// \param[in] triple The target triple.
+/// \param[in] include_dirs The include directories the compiler will use
+/// during header search.
+///
static void
SetupImportStdModuleLangOpts(CompilerInstance &compiler,
llvm::Triple const &triple,
>From 3070e78a175db5a1cc994cd544910170b72f6d2c Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Sat, 3 Aug 2024 17:44:53 +0100
Subject: [PATCH 4/6] fixup! add comments
---
.../Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 69c23e43c7fad..fbe1953dba5b8 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -613,7 +613,7 @@ sdkSupportsBuiltinModules(llvm::Triple const &triple,
return false;
size_t suffix = it->find(s_sdk_suffix);
- assert (suffix == std::string::npos);
+ assert(suffix == std::string::npos);
auto VFS = FileSystem::Instance().GetVirtualFileSystem();
if (!VFS)
@@ -630,7 +630,8 @@ sdkSupportsBuiltinModules(llvm::Triple const &triple,
return sdkSupportsBuiltinModulesImpl(triple, *parsed);
}
-/// Sets the LangOptions to prepare for running with `import-std-module`
enabled.
+/// Sets the LangOptions to prepare for running with `import-std-module`
+/// enabled.
///
/// \param[out] compiler CompilerInstance on which to set the LangOptions.
/// \param[in] triple The target triple.
>From b4b79b9a96d0b0461d94e4810a37a4aa767f8db1 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Sat, 3 Aug 2024 17:46:12 +0100
Subject: [PATCH 5/6] fixup! move assert
---
.../Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index fbe1953dba5b8..e05cd649bb044 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -612,14 +612,13 @@ sdkSupportsBuiltinModules(llvm::Triple const &triple,
if (it == include_dirs.end())
return false;
- size_t suffix = it->find(s_sdk_suffix);
- assert(suffix == std::string::npos);
-
auto VFS = FileSystem::Instance().GetVirtualFileSystem();
if (!VFS)
return false;
// Extract: /path/to/some.sdk
+ size_t suffix = it->find(s_sdk_suffix);
+ assert(suffix != std::string::npos);
std::string sdk_path = it->substr(0, suffix + s_sdk_suffix.size());
// Extract SDK version from the /path/to/some.sdk/SDKSettings.json
>From ff7333fd9a072b2c31d1aa1b0f33d70b70b1504f Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Tue, 6 Aug 2024 11:45:36 +0100
Subject: [PATCH 6/6] fixup! add another FIXME
---
.../Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index e05cd649bb044..9531104d81b1c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -661,6 +661,8 @@ SetupImportStdModuleLangOpts(CompilerInstance &compiler,
// FIXME: We should use the driver to derive this for us.
// ClangModulesDeclVendor already parses the SDKSettings for the purposes of
// this check.
+ // FIXME: Instead of using include_dirs here, we should use DW_AT_LLVM_sdk
+ // and look up a matching local SDK using HostInfo.
lang_opts.BuiltinHeadersInSystemModules =
!sdkSupportsBuiltinModules(triple, include_dirs);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
