[Lldb-commits] [lldb] [lldb][ClangExpressionParser][NFC] Factor LangOptions logic out of ClangExpressionParser constructor (PR #101669)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Fri Aug 2 08:54:43 PDT 2024
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/101669
>From a4518719a09ef2910209663c62544dc5797f2df7 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 2 Aug 2024 13:48:18 +0100
Subject: [PATCH 1/4] [lldb][ClangExpressionParser][NFC] Factor LangOptions
logic out of ClangExpressionParser constructor
We plan to eventually use the Clang driver to initialize the
`CompilerInstance`.
This should make refactorings of this code more straightforward.
---
lldb/include/lldb/Expression/Expression.h | 2 +-
lldb/include/lldb/Expression/UserExpression.h | 2 +-
.../Clang/ClangExpressionParser.cpp | 301 +++++++++---------
3 files changed, 157 insertions(+), 148 deletions(-)
diff --git a/lldb/include/lldb/Expression/Expression.h b/lldb/include/lldb/Expression/Expression.h
index 356fe4b82ae43..8de9364436ccf 100644
--- a/lldb/include/lldb/Expression/Expression.h
+++ b/lldb/include/lldb/Expression/Expression.h
@@ -56,7 +56,7 @@ class Expression {
/// Return the desired result type of the function, or eResultTypeAny if
/// indifferent.
- virtual ResultType DesiredResultType() { return eResultTypeAny; }
+ virtual ResultType DesiredResultType() const { return eResultTypeAny; }
/// Flags
diff --git a/lldb/include/lldb/Expression/UserExpression.h b/lldb/include/lldb/Expression/UserExpression.h
index b04d00b72e8fa..7ce463d2cb4e7 100644
--- a/lldb/include/lldb/Expression/UserExpression.h
+++ b/lldb/include/lldb/Expression/UserExpression.h
@@ -206,7 +206,7 @@ class UserExpression : public Expression {
/// Return the desired result type of the function, or eResultTypeAny if
/// indifferent.
- ResultType DesiredResultType() override { return m_desired_type; }
+ ResultType DesiredResultType() const override { return m_desired_type; }
/// Return true if validation code should be inserted into the expression.
bool NeedsValidation() override { return true; }
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 303e88feea20b..11d519e738ee5 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -77,6 +77,7 @@
#include "lldb/Host/HostInfo.h"
#include "lldb/Symbol/SymbolVendor.h"
#include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/ExecutionContextScope.h"
#include "lldb/Target/Language.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
@@ -351,62 +352,20 @@ static void SetupDefaultClangDiagnostics(CompilerInstance &compiler) {
}
}
-//===----------------------------------------------------------------------===//
-// Implementation of ClangExpressionParser
-//===----------------------------------------------------------------------===//
-
-ClangExpressionParser::ClangExpressionParser(
- ExecutionContextScope *exe_scope, Expression &expr,
- bool generate_debug_info, std::vector<std::string> include_directories,
- std::string filename)
- : ExpressionParser(exe_scope, expr, generate_debug_info), m_compiler(),
- m_pp_callbacks(nullptr),
- m_include_directories(std::move(include_directories)),
- m_filename(std::move(filename)) {
+static void SetupLangOpts(CompilerInstance &compiler,
+ ExecutionContextScope &exe_scope,
+ Expression const &expr) {
Log *log = GetLog(LLDBLog::Expressions);
- // We can't compile expressions without a target. So if the exe_scope is
- // null or doesn't have a target, then we just need to get out of here. I'll
- // lldbassert and not make any of the compiler objects since
- // I can't return errors directly from the constructor. Further calls will
- // check if the compiler was made and
- // bag out if it wasn't.
-
- if (!exe_scope) {
- lldbassert(exe_scope &&
- "Can't make an expression parser with a null scope.");
- return;
- }
-
- lldb::TargetSP target_sp;
- target_sp = exe_scope->CalculateTarget();
- if (!target_sp) {
- lldbassert(target_sp.get() &&
- "Can't make an expression parser with a null target.");
- return;
- }
-
- // 1. Create a new compiler instance.
- m_compiler = std::make_unique<CompilerInstance>();
+ // If the expression is being evaluated in the context of an existing stack
+ // frame, we introspect to see if the language runtime is available.
- // Make sure clang uses the same VFS as LLDB.
- m_compiler->createFileManager(FileSystem::Instance().GetVirtualFileSystem());
+ lldb::StackFrameSP frame_sp = exe_scope.CalculateStackFrame();
+ lldb::ProcessSP process_sp = exe_scope.CalculateProcess();
// Defaults to lldb::eLanguageTypeUnknown.
lldb::LanguageType frame_lang = expr.Language().AsLanguageType();
- std::string abi;
- ArchSpec target_arch;
- target_arch = target_sp->GetArchitecture();
-
- const auto target_machine = target_arch.GetMachine();
-
- // If the expression is being evaluated in the context of an existing stack
- // frame, we introspect to see if the language runtime is available.
-
- lldb::StackFrameSP frame_sp = exe_scope->CalculateStackFrame();
- lldb::ProcessSP process_sp = exe_scope->CalculateProcess();
-
// Make sure the user hasn't provided a preferred execution language with
// `expression --language X -- ...`
if (frame_sp && frame_lang == lldb::eLanguageTypeUnknown)
@@ -414,73 +373,11 @@ ClangExpressionParser::ClangExpressionParser(
if (process_sp && frame_lang != lldb::eLanguageTypeUnknown) {
LLDB_LOGF(log, "Frame has language of type %s",
- Language::GetNameForLanguageType(frame_lang));
+ lldb_private::Language::GetNameForLanguageType(frame_lang));
}
- // 2. Configure the compiler with a set of default options that are
- // appropriate for most situations.
- if (target_arch.IsValid()) {
- std::string triple = target_arch.GetTriple().str();
- m_compiler->getTargetOpts().Triple = triple;
- LLDB_LOGF(log, "Using %s as the target triple",
- m_compiler->getTargetOpts().Triple.c_str());
- } else {
- // If we get here we don't have a valid target and just have to guess.
- // Sometimes this will be ok to just use the host target triple (when we
- // evaluate say "2+3", but other expressions like breakpoint conditions and
- // other things that _are_ target specific really shouldn't just be using
- // the host triple. In such a case the language runtime should expose an
- // overridden options set (3), below.
- m_compiler->getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
- LLDB_LOGF(log, "Using default target triple of %s",
- m_compiler->getTargetOpts().Triple.c_str());
- }
- // Now add some special fixes for known architectures: Any arm32 iOS
- // environment, but not on arm64
- if (m_compiler->getTargetOpts().Triple.find("arm64") == std::string::npos &&
- m_compiler->getTargetOpts().Triple.find("arm") != std::string::npos &&
- m_compiler->getTargetOpts().Triple.find("ios") != std::string::npos) {
- m_compiler->getTargetOpts().ABI = "apcs-gnu";
- }
- // Supported subsets of x86
- if (target_machine == llvm::Triple::x86 ||
- target_machine == llvm::Triple::x86_64) {
- m_compiler->getTargetOpts().FeaturesAsWritten.push_back("+sse");
- m_compiler->getTargetOpts().FeaturesAsWritten.push_back("+sse2");
- }
-
- // Set the target CPU to generate code for. This will be empty for any CPU
- // that doesn't really need to make a special
- // CPU string.
- m_compiler->getTargetOpts().CPU = target_arch.GetClangTargetCPU();
-
- // Set the target ABI
- abi = GetClangTargetABI(target_arch);
- if (!abi.empty())
- m_compiler->getTargetOpts().ABI = abi;
-
- // 3. Create and install the target on the compiler.
- m_compiler->createDiagnostics();
- // Limit the number of error diagnostics we emit.
- // A value of 0 means no limit for both LLDB and Clang.
- m_compiler->getDiagnostics().setErrorLimit(target_sp->GetExprErrorLimit());
-
- auto target_info = TargetInfo::CreateTargetInfo(
- m_compiler->getDiagnostics(), m_compiler->getInvocation().TargetOpts);
- if (log) {
- LLDB_LOGF(log, "Target datalayout string: '%s'",
- target_info->getDataLayoutString());
- LLDB_LOGF(log, "Target ABI: '%s'", target_info->getABI().str().c_str());
- LLDB_LOGF(log, "Target vector alignment: %d",
- target_info->getMaxVectorAlign());
- }
- m_compiler->setTarget(target_info);
-
- assert(m_compiler->hasTarget());
-
- // 4. Set language options.
lldb::LanguageType language = expr.Language().AsLanguageType();
- LangOptions &lang_opts = m_compiler->getLangOpts();
+ LangOptions &lang_opts = compiler.getLangOpts();
switch (language) {
case lldb::eLanguageTypeC:
@@ -522,7 +419,7 @@ ClangExpressionParser::ClangExpressionParser(
case lldb::eLanguageTypeC_plus_plus_11:
case lldb::eLanguageTypeC_plus_plus_14:
lang_opts.CPlusPlus11 = true;
- m_compiler->getHeaderSearchOpts().UseLibcxx = true;
+ compiler.getHeaderSearchOpts().UseLibcxx = true;
[[fallthrough]];
case lldb::eLanguageTypeC_plus_plus_03:
lang_opts.CPlusPlus = true;
@@ -539,7 +436,7 @@ ClangExpressionParser::ClangExpressionParser(
lang_opts.ObjC = true;
lang_opts.CPlusPlus = true;
lang_opts.CPlusPlus11 = true;
- m_compiler->getHeaderSearchOpts().UseLibcxx = true;
+ compiler.getHeaderSearchOpts().UseLibcxx = true;
break;
}
@@ -551,42 +448,14 @@ ClangExpressionParser::ClangExpressionParser(
if (expr.DesiredResultType() == Expression::eResultTypeId)
lang_opts.DebuggerCastResultToId = true;
- lang_opts.CharIsSigned = ArchSpec(m_compiler->getTargetOpts().Triple.c_str())
- .CharIsSignedByDefault();
+ lang_opts.CharIsSigned =
+ ArchSpec(compiler.getTargetOpts().Triple.c_str()).CharIsSignedByDefault();
// Spell checking is a nice feature, but it ends up completing a lot of types
// that we didn't strictly speaking need to complete. As a result, we spend a
// long time parsing and importing debug information.
lang_opts.SpellChecking = false;
- auto *clang_expr = dyn_cast<ClangUserExpression>(&m_expr);
- if (clang_expr && clang_expr->DidImportCxxModules()) {
- LLDB_LOG(log, "Adding lang options for importing C++ modules");
-
- lang_opts.Modules = true;
- // We want to implicitly build modules.
- lang_opts.ImplicitModules = true;
- // To automatically import all submodules when we import 'std'.
- lang_opts.ModulesLocalVisibility = false;
-
- // We use the @import statements, so we need this:
- // FIXME: We could use the modules-ts, but that currently doesn't work.
- lang_opts.ObjC = true;
-
- // Options we need to parse libc++ code successfully.
- // FIXME: We should ask the driver for the appropriate default flags.
- lang_opts.GNUMode = true;
- lang_opts.GNUKeywords = true;
- lang_opts.CPlusPlus11 = true;
- lang_opts.BuiltinHeadersInSystemModules = true;
-
- // The Darwin libc expects this macro to be set.
- lang_opts.GNUCVersion = 40201;
-
- SetupModuleHeaderPaths(m_compiler.get(), m_include_directories,
- target_sp);
- }
-
if (process_sp && lang_opts.ObjC) {
if (auto *runtime = ObjCLanguageRuntime::Get(*process_sp)) {
switch (runtime->GetRuntimeVersion()) {
@@ -615,6 +484,146 @@ ClangExpressionParser::ClangExpressionParser(
// 'fopen'). Those libc functions are already correctly handled by LLDB, and
// additionally enabling them as expandable builtins is breaking Clang.
lang_opts.NoBuiltin = true;
+}
+
+void SetupImportStdModuleLangOpts(CompilerInstance &compiler) {
+ LangOptions &lang_opts = compiler.getLangOpts();
+ lang_opts.Modules = true;
+ // We want to implicitly build modules.
+ lang_opts.ImplicitModules = true;
+ // To automatically import all submodules when we import 'std'.
+ lang_opts.ModulesLocalVisibility = false;
+
+ // We use the @import statements, so we need this:
+ // FIXME: We could use the modules-ts, but that currently doesn't work.
+ lang_opts.ObjC = true;
+
+ // Options we need to parse libc++ code successfully.
+ // FIXME: We should ask the driver for the appropriate default flags.
+ lang_opts.GNUMode = true;
+ lang_opts.GNUKeywords = true;
+ lang_opts.CPlusPlus11 = true;
+ lang_opts.BuiltinHeadersInSystemModules = true;
+
+ // The Darwin libc expects this macro to be set.
+ lang_opts.GNUCVersion = 40201;
+}
+
+//===----------------------------------------------------------------------===//
+// Implementation of ClangExpressionParser
+//===----------------------------------------------------------------------===//
+
+ClangExpressionParser::ClangExpressionParser(
+ ExecutionContextScope *exe_scope, Expression &expr,
+ bool generate_debug_info, std::vector<std::string> include_directories,
+ std::string filename)
+ : ExpressionParser(exe_scope, expr, generate_debug_info), m_compiler(),
+ m_pp_callbacks(nullptr),
+ m_include_directories(std::move(include_directories)),
+ m_filename(std::move(filename)) {
+ Log *log = GetLog(LLDBLog::Expressions);
+
+ // We can't compile expressions without a target. So if the exe_scope is
+ // null or doesn't have a target, then we just need to get out of here. I'll
+ // lldbassert and not make any of the compiler objects since
+ // I can't return errors directly from the constructor. Further calls will
+ // check if the compiler was made and
+ // bag out if it wasn't.
+
+ if (!exe_scope) {
+ lldbassert(exe_scope &&
+ "Can't make an expression parser with a null scope.");
+ return;
+ }
+
+ lldb::TargetSP target_sp;
+ target_sp = exe_scope->CalculateTarget();
+ if (!target_sp) {
+ lldbassert(target_sp.get() &&
+ "Can't make an expression parser with a null target.");
+ return;
+ }
+
+ // 1. Create a new compiler instance.
+ m_compiler = std::make_unique<CompilerInstance>();
+
+ // Make sure clang uses the same VFS as LLDB.
+ m_compiler->createFileManager(FileSystem::Instance().GetVirtualFileSystem());
+
+ std::string abi;
+ ArchSpec target_arch;
+ target_arch = target_sp->GetArchitecture();
+
+ const auto target_machine = target_arch.GetMachine();
+
+ // 2. Configure the compiler with a set of default options that are
+ // appropriate for most situations.
+ if (target_arch.IsValid()) {
+ std::string triple = target_arch.GetTriple().str();
+ m_compiler->getTargetOpts().Triple = triple;
+ LLDB_LOGF(log, "Using %s as the target triple",
+ m_compiler->getTargetOpts().Triple.c_str());
+ } else {
+ // If we get here we don't have a valid target and just have to guess.
+ // Sometimes this will be ok to just use the host target triple (when we
+ // evaluate say "2+3", but other expressions like breakpoint conditions and
+ // other things that _are_ target specific really shouldn't just be using
+ // the host triple. In such a case the language runtime should expose an
+ // overridden options set (3), below.
+ m_compiler->getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
+ LLDB_LOGF(log, "Using default target triple of %s",
+ m_compiler->getTargetOpts().Triple.c_str());
+ }
+ // Now add some special fixes for known architectures: Any arm32 iOS
+ // environment, but not on arm64
+ if (m_compiler->getTargetOpts().Triple.find("arm64") == std::string::npos &&
+ m_compiler->getTargetOpts().Triple.find("arm") != std::string::npos &&
+ m_compiler->getTargetOpts().Triple.find("ios") != std::string::npos) {
+ m_compiler->getTargetOpts().ABI = "apcs-gnu";
+ }
+ // Supported subsets of x86
+ if (target_machine == llvm::Triple::x86 ||
+ target_machine == llvm::Triple::x86_64) {
+ m_compiler->getTargetOpts().FeaturesAsWritten.push_back("+sse");
+ m_compiler->getTargetOpts().FeaturesAsWritten.push_back("+sse2");
+ }
+
+ // Set the target CPU to generate code for. This will be empty for any CPU
+ // that doesn't really need to make a special
+ // CPU string.
+ m_compiler->getTargetOpts().CPU = target_arch.GetClangTargetCPU();
+
+ // Set the target ABI
+ abi = GetClangTargetABI(target_arch);
+ if (!abi.empty())
+ m_compiler->getTargetOpts().ABI = abi;
+
+ // 3. Create and install the target on the compiler.
+ m_compiler->createDiagnostics();
+ // Limit the number of error diagnostics we emit.
+ // A value of 0 means no limit for both LLDB and Clang.
+ m_compiler->getDiagnostics().setErrorLimit(target_sp->GetExprErrorLimit());
+
+ auto target_info = TargetInfo::CreateTargetInfo(
+ m_compiler->getDiagnostics(), m_compiler->getInvocation().TargetOpts);
+ if (log) {
+ LLDB_LOGF(log, "Target datalayout string: '%s'",
+ target_info->getDataLayoutString());
+ LLDB_LOGF(log, "Target ABI: '%s'", target_info->getABI().str().c_str());
+ LLDB_LOGF(log, "Target vector alignment: %d",
+ target_info->getMaxVectorAlign());
+ }
+ m_compiler->setTarget(target_info);
+
+ assert(m_compiler->hasTarget());
+
+ // 4. Set language options.
+ SetupLangOpts(*m_compiler, *exe_scope, expr);
+ if (isa<ClangUserExpression>(&m_expr)) {
+ LLDB_LOG(log, "Adding lang options for importing C++ modules");
+ SetupImportStdModuleLangOpts(*m_compiler);
+ SetupModuleHeaderPaths(m_compiler.get(), m_include_directories, target_sp);
+ }
// Set CodeGen options
m_compiler->getCodeGenOpts().EmitDeclMetadata = true;
@@ -648,7 +657,7 @@ ClangExpressionParser::ClangExpressionParser(
m_compiler->createSourceManager(m_compiler->getFileManager());
m_compiler->createPreprocessor(TU_Complete);
- switch (language) {
+ switch (expr.Language().AsLanguageType()) {
case lldb::eLanguageTypeC:
case lldb::eLanguageTypeC89:
case lldb::eLanguageTypeC99:
>From bea9397c224a09a072da8275f6771b4d946f4b7e Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 2 Aug 2024 15:27:57 +0100
Subject: [PATCH 2/4] fixup! make SetupImportStdModuleLangOpts static
---
.../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 11d519e738ee5..f962309875c65 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -486,7 +486,7 @@ static void SetupLangOpts(CompilerInstance &compiler,
lang_opts.NoBuiltin = true;
}
-void SetupImportStdModuleLangOpts(CompilerInstance &compiler) {
+static void SetupImportStdModuleLangOpts(CompilerInstance &compiler) {
LangOptions &lang_opts = compiler.getLangOpts();
lang_opts.Modules = true;
// We want to implicitly build modules.
>From 0807e6fa4e925a74375109e824844324c270deb5 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 2 Aug 2024 15:47:09 +0100
Subject: [PATCH 3/4] fixup! add back DidImportCxxModules check
---
.../Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index f962309875c65..ee6bc6ca43217 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -619,7 +619,8 @@ ClangExpressionParser::ClangExpressionParser(
// 4. Set language options.
SetupLangOpts(*m_compiler, *exe_scope, expr);
- if (isa<ClangUserExpression>(&m_expr)) {
+ 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);
SetupModuleHeaderPaths(m_compiler.get(), m_include_directories, target_sp);
>From 594f80c3f8e6c97c9e656e5ca17a98833381d415 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 2 Aug 2024 16:54:30 +0100
Subject: [PATCH 4/4] fixup! reluctantly, west-const
---
.../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 ee6bc6ca43217..aed065edab90f 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -354,7 +354,7 @@ static void SetupDefaultClangDiagnostics(CompilerInstance &compiler) {
static void SetupLangOpts(CompilerInstance &compiler,
ExecutionContextScope &exe_scope,
- Expression const &expr) {
+ const Expression &expr) {
Log *log = GetLog(LLDBLog::Expressions);
// If the expression is being evaluated in the context of an existing stack
More information about the lldb-commits
mailing list