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/2] [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/2] 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 = _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits