[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 07:47:36 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/3] [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/3] 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/3] 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);



More information about the lldb-commits mailing list