Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a new SBExpressionOptions::SetLanguage() API (NFCI) #89981

Merged
merged 1 commit into from Apr 29, 2024

Conversation

adrian-prantl
Copy link
Collaborator

that separates out language and version. To avoid reinventing the wheel and introducing subtle incompatibilities, this API uses the table of languages and versiond defined by the upcoming DWARF 6 standard (https://dwarfstd.org/languages-v6.html). While the DWARF 6 spec is not finialized, the list of languages is broadly considered stable.

The primary motivation for this is to allow the Swift language plugin to switch between language dialects between, e.g., Swift 5.9 and 6.0 with out introducing a ton of new language codes. On the main branch this change is considered NFC.

Depends on #89980

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 24, 2024

@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)

Changes

that separates out language and version. To avoid reinventing the wheel and introducing subtle incompatibilities, this API uses the table of languages and versiond defined by the upcoming DWARF 6 standard (https://dwarfstd.org/languages-v6.html). While the DWARF 6 spec is not finialized, the list of languages is broadly considered stable.

The primary motivation for this is to allow the Swift language plugin to switch between language dialects between, e.g., Swift 5.9 and 6.0 with out introducing a ton of new language codes. On the main branch this change is considered NFC.

Depends on #89980


Patch is 34.69 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/89981.diff

24 Files Affected:

  • (modified) lldb/include/lldb/API/SBExpressionOptions.h (+4)
  • (modified) lldb/include/lldb/Expression/Expression.h (+2-5)
  • (modified) lldb/include/lldb/Expression/LLVMUserExpression.h (+1-1)
  • (modified) lldb/include/lldb/Expression/UserExpression.h (+18-15)
  • (modified) lldb/include/lldb/Symbol/TypeSystem.h (+4-6)
  • (modified) lldb/include/lldb/Target/StackFrame.h (+5-6)
  • (modified) lldb/include/lldb/Target/Target.h (+14-5)
  • (modified) lldb/include/lldb/lldb-private-types.h (+19)
  • (modified) lldb/source/API/SBExpressionOptions.cpp (+7)
  • (modified) lldb/source/API/SBFrame.cpp (+17-13)
  • (modified) lldb/source/Breakpoint/Watchpoint.cpp (+2-3)
  • (modified) lldb/source/Commands/CommandObjectDWIMPrint.cpp (+1-1)
  • (modified) lldb/source/Commands/CommandObjectType.cpp (+1-1)
  • (modified) lldb/source/Expression/LLVMUserExpression.cpp (+1-1)
  • (modified) lldb/source/Expression/UserExpression.cpp (+8-6)
  • (modified) lldb/source/Expression/UtilityFunction.cpp (+2-2)
  • (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+6-6)
  • (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp (+10-9)
  • (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h (+3-3)
  • (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+1-1)
  • (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (+6-6)
  • (modified) lldb/source/Target/Language.cpp (+34)
  • (modified) lldb/source/Target/StackFrame.cpp (+10-13)
  • (modified) lldb/source/Target/Target.cpp (+11-10)
diff --git a/lldb/include/lldb/API/SBExpressionOptions.h b/lldb/include/lldb/API/SBExpressionOptions.h
index e0ddfda5ba37a2..0f2d298d7513fe 100644
--- a/lldb/include/lldb/API/SBExpressionOptions.h
+++ b/lldb/include/lldb/API/SBExpressionOptions.h
@@ -67,6 +67,10 @@ class LLDB_API SBExpressionOptions {
   void SetTrapExceptions(bool trap_exceptions = true);
 
   void SetLanguage(lldb::LanguageType language);
+  /// Set the language using a pair of language code and version as
+  /// defined by the DWARF 6 specification.
+  /// WARNING: These codes may change until DWARF 6 is finalized.
+  void SetLanguage(uint16_t dwarf_lname_code, uint32_t dwarf_lversion);
 
 #ifndef SWIG
   void SetCancelCallback(lldb::ExpressionCancelCallback callback, void *baton);
diff --git a/lldb/include/lldb/Expression/Expression.h b/lldb/include/lldb/Expression/Expression.h
index 3e61d78828bbbf..356fe4b82ae43a 100644
--- a/lldb/include/lldb/Expression/Expression.h
+++ b/lldb/include/lldb/Expression/Expression.h
@@ -47,11 +47,8 @@ class Expression {
   /// expression.  Text() should contain the definition of this function.
   virtual const char *FunctionName() = 0;
 
-  /// Return the language that should be used when parsing.  To use the
-  /// default, return eLanguageTypeUnknown.
-  virtual lldb::LanguageType Language() const {
-    return lldb::eLanguageTypeUnknown;
-  }
+  /// Return the language that should be used when parsing.
+  virtual SourceLanguage Language() const { return {}; }
 
   /// Return the Materializer that the parser should use when registering
   /// external values.
diff --git a/lldb/include/lldb/Expression/LLVMUserExpression.h b/lldb/include/lldb/Expression/LLVMUserExpression.h
index 7d32d17dbf544c..40b463933c07e8 100644
--- a/lldb/include/lldb/Expression/LLVMUserExpression.h
+++ b/lldb/include/lldb/Expression/LLVMUserExpression.h
@@ -52,7 +52,7 @@ class LLVMUserExpression : public UserExpression {
   };
 
   LLVMUserExpression(ExecutionContextScope &exe_scope, llvm::StringRef expr,
-                     llvm::StringRef prefix, lldb::LanguageType language,
+                     llvm::StringRef prefix, SourceLanguage language,
                      ResultType desired_type,
                      const EvaluateExpressionOptions &options);
   ~LLVMUserExpression() override;
diff --git a/lldb/include/lldb/Expression/UserExpression.h b/lldb/include/lldb/Expression/UserExpression.h
index b6cfeec7e89933..b04d00b72e8faa 100644
--- a/lldb/include/lldb/Expression/UserExpression.h
+++ b/lldb/include/lldb/Expression/UserExpression.h
@@ -56,7 +56,7 @@ class UserExpression : public Expression {
   ///     If not eResultTypeAny, the type to use for the expression
   ///     result.
   UserExpression(ExecutionContextScope &exe_scope, llvm::StringRef expr,
-                 llvm::StringRef prefix, lldb::LanguageType language,
+                 llvm::StringRef prefix, SourceLanguage language,
                  ResultType desired_type,
                  const EvaluateExpressionOptions &options);
 
@@ -202,7 +202,7 @@ class UserExpression : public Expression {
   virtual bool IsParseCacheable() { return true; }
   /// Return the language that should be used when parsing.  To use the
   /// default, return eLanguageTypeUnknown.
-  lldb::LanguageType Language() const override { return m_language; }
+  SourceLanguage Language() const override { return m_language; }
 
   /// Return the desired result type of the function, or eResultTypeAny if
   /// indifferent.
@@ -315,19 +315,22 @@ class UserExpression : public Expression {
                            lldb::ProcessSP &process_sp,
                            lldb::StackFrameSP &frame_sp);
 
-  Address m_address;       ///< The address the process is stopped in.
-  std::string m_expr_text; ///< The text of the expression, as typed by the user
-  std::string m_expr_prefix; ///< The text of the translation-level definitions,
-                             ///as provided by the user
-  std::string m_fixed_text; ///< The text of the expression with fix-its applied
-                            ///- this won't be set if the fixed text doesn't
-                            ///parse.
-  lldb::LanguageType m_language; ///< The language to use when parsing
-                                 ///(eLanguageTypeUnknown means use defaults)
-  ResultType m_desired_type; ///< The type to coerce the expression's result to.
-                             ///If eResultTypeAny, inferred from the expression.
-  EvaluateExpressionOptions
-      m_options; ///< Additional options provided by the user.
+  /// The address the process is stopped in.
+  Address m_address;
+  /// The text of the expression, as typed by the user.
+  std::string m_expr_text;
+  /// The text of the translation-level definitions, as provided by the user.
+  std::string m_expr_prefix;
+  /// The text of the expression with fix-its applied this won't be set if the
+  /// fixed text doesn't parse.
+  std::string m_fixed_text;
+  /// The language to use when parsing (unknown means use defaults).
+  SourceLanguage m_language;
+  /// The type to coerce the expression's result to. If eResultTypeAny, inferred
+  /// from the expression.
+  ResultType m_desired_type;
+  /// Additional options provided by the user.
+  EvaluateExpressionOptions m_options;
 };
 
 } // namespace lldb_private
diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h
index 3a927d313b823d..6221e6d2e5d834 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -474,12 +474,10 @@ class TypeSystem : public PluginInterface,
     return IsPointerOrReferenceType(type, nullptr);
   }
 
-  virtual UserExpression *
-  GetUserExpression(llvm::StringRef expr, llvm::StringRef prefix,
-                    lldb::LanguageType language,
-                    Expression::ResultType desired_type,
-                    const EvaluateExpressionOptions &options,
-                    ValueObject *ctx_obj) {
+  virtual UserExpression *GetUserExpression(
+      llvm::StringRef expr, llvm::StringRef prefix, SourceLanguage language,
+      Expression::ResultType desired_type,
+      const EvaluateExpressionOptions &options, ValueObject *ctx_obj) {
     return nullptr;
   }
 
diff --git a/lldb/include/lldb/Target/StackFrame.h b/lldb/include/lldb/Target/StackFrame.h
index 6c18511c6e1ac3..259f8f9da0d1c7 100644
--- a/lldb/include/lldb/Target/StackFrame.h
+++ b/lldb/include/lldb/Target/StackFrame.h
@@ -446,13 +446,12 @@ class StackFrame : public ExecutionContextScope,
   /// Query this frame to determine what the default language should be when
   /// parsing expressions given the execution context.
   ///
-  /// \return
-  ///   The language of the frame if known, else lldb::eLanguageTypeUnknown.
-  lldb::LanguageType GetLanguage();
+  /// \return   The language of the frame if known.
+  SourceLanguage GetLanguage();
 
-  // similar to GetLanguage(), but is allowed to take a potentially incorrect
-  // guess if exact information is not available
-  lldb::LanguageType GuessLanguage();
+  /// Similar to GetLanguage(), but is allowed to take a potentially incorrect
+  /// guess if exact information is not available
+  SourceLanguage GuessLanguage();
 
   /// Attempt to econstruct the ValueObject for a given raw address touched by
   /// the current instruction.  The ExpressionPath should indicate how to get
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 2c2e6b2831ccee..e7b1f5ca78592e 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -200,7 +200,7 @@ class TargetProperties : public Properties {
 
   bool GetBreakpointsConsultPlatformAvoidList();
 
-  lldb::LanguageType GetLanguage() const;
+  SourceLanguage GetLanguage() const;
 
   llvm::StringRef GetExpressionPrefixContents();
 
@@ -310,9 +310,18 @@ class EvaluateExpressionOptions {
     m_execution_policy = policy;
   }
 
-  lldb::LanguageType GetLanguage() const { return m_language; }
+  SourceLanguage GetLanguage() const { return m_language; }
 
-  void SetLanguage(lldb::LanguageType language) { m_language = language; }
+  void SetLanguage(lldb::LanguageType language_type) {
+    m_language = SourceLanguage(language_type);
+  }
+
+  /// Set the language using a pair of language code and version as
+  /// defined by the DWARF 6 specification.
+  /// WARNING: These codes may change until DWARF 6 is finalized.
+  void SetLanguage(uint16_t dwarf_lname_code, uint32_t dwarf_lversion) {
+    m_language = SourceLanguage(dwarf_lname_code, dwarf_lversion);
+  }
 
   bool DoesCoerceToId() const { return m_coerce_to_id; }
 
@@ -445,7 +454,7 @@ class EvaluateExpressionOptions {
 
 private:
   ExecutionPolicy m_execution_policy = default_execution_policy;
-  lldb::LanguageType m_language = lldb::eLanguageTypeUnknown;
+  SourceLanguage m_language;
   std::string m_prefix;
   bool m_coerce_to_id = false;
   bool m_unwind_on_error = true;
@@ -1160,7 +1169,7 @@ class Target : public std::enable_shared_from_this<Target>,
 
   UserExpression *
   GetUserExpressionForLanguage(llvm::StringRef expr, llvm::StringRef prefix,
-                               lldb::LanguageType language,
+                               SourceLanguage language,
                                Expression::ResultType desired_type,
                                const EvaluateExpressionOptions &options,
                                ValueObject *ctx_obj, Status &error);
diff --git a/lldb/include/lldb/lldb-private-types.h b/lldb/include/lldb/lldb-private-types.h
index 7d301666df1a17..b8779fce28f5a4 100644
--- a/lldb/include/lldb/lldb-private-types.h
+++ b/lldb/include/lldb/lldb-private-types.h
@@ -96,6 +96,25 @@ struct RegisterSet {
   const uint32_t *registers;
 };
 
+/// A type-erased pair of llvm::dwarf::SourceLanguageName and version.
+struct SourceLanguage {
+  SourceLanguage() = default;
+  SourceLanguage(lldb::LanguageType language_type);
+  SourceLanguage(uint32_t name, uint16_t version)
+      : name(name), version(version) {}
+  SourceLanguage(std::optional<std::pair<uint32_t, uint16_t>> name_vers)
+      : name(name_vers ? name_vers->first : 0),
+        version(name_vers ? name_vers->second : 0) {}
+  operator bool() const { return name > 0; }
+  lldb::LanguageType AsLanguageType() const;
+  llvm::StringRef GetDescription() const;
+  bool IsC() const;
+  bool IsObjC() const;
+  bool IsCPlusPlus() const;
+  uint16_t name = 0;
+  uint32_t version = 0;
+};
+
 struct OptionEnumValueElement {
   int64_t value;
   const char *string_value;
diff --git a/lldb/source/API/SBExpressionOptions.cpp b/lldb/source/API/SBExpressionOptions.cpp
index bd81a04596b10d..3aea121f7323d4 100644
--- a/lldb/source/API/SBExpressionOptions.cpp
+++ b/lldb/source/API/SBExpressionOptions.cpp
@@ -156,6 +156,13 @@ void SBExpressionOptions::SetLanguage(lldb::LanguageType language) {
   m_opaque_up->SetLanguage(language);
 }
 
+void SBExpressionOptions::SetLanguage(uint16_t dwarf_lname_code,
+                                      uint32_t dwarf_lversion) {
+  LLDB_INSTRUMENT_VA(this, dwarf_lname_code, dwarf_lversion);
+
+  m_opaque_up->SetLanguage(dwarf_lname_code, dwarf_lversion);
+}
+
 void SBExpressionOptions::SetCancelCallback(
     lldb::ExpressionCancelCallback callback, void *baton) {
   LLDB_INSTRUMENT_VA(this, callback, baton);
diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp
index a16bbc2ae7562d..2de1f803aefe4a 100644
--- a/lldb/source/API/SBFrame.cpp
+++ b/lldb/source/API/SBFrame.cpp
@@ -1024,10 +1024,10 @@ SBValue SBFrame::EvaluateExpression(const char *expr) {
     options.SetFetchDynamicValue(fetch_dynamic_value);
     options.SetUnwindOnError(true);
     options.SetIgnoreBreakpoints(true);
-    if (target->GetLanguage() != eLanguageTypeUnknown)
-      options.SetLanguage(target->GetLanguage());
-    else
-      options.SetLanguage(frame->GetLanguage());
+    SourceLanguage lang = target->GetLanguage();
+    if (!lang)
+      lang = frame->GetLanguage();
+    options.SetLanguage(lang.name, lang.version);
     return EvaluateExpression(expr, options);
   } else {
     Status error;
@@ -1053,10 +1053,12 @@ SBFrame::EvaluateExpression(const char *expr,
 
   StackFrame *frame = exe_ctx.GetFramePtr();
   Target *target = exe_ctx.GetTargetPtr();
-  if (target && target->GetLanguage() != eLanguageTypeUnknown)
-    options.SetLanguage(target->GetLanguage());
-  else if (frame)
-    options.SetLanguage(frame->GetLanguage());
+  SourceLanguage language;
+  if (target)
+    language = target->GetLanguage();
+  if (!language && frame)
+    language = frame->GetLanguage();
+  options.SetLanguage(language.name, language.version);
   return EvaluateExpression(expr, options);
 }
 
@@ -1074,10 +1076,12 @@ SBValue SBFrame::EvaluateExpression(const char *expr,
   options.SetIgnoreBreakpoints(true);
   StackFrame *frame = exe_ctx.GetFramePtr();
   Target *target = exe_ctx.GetTargetPtr();
-  if (target && target->GetLanguage() != eLanguageTypeUnknown)
-    options.SetLanguage(target->GetLanguage());
-  else if (frame)
-    options.SetLanguage(frame->GetLanguage());
+  SourceLanguage language;
+  if (target)
+    language = target->GetLanguage();
+  if (!language && frame)
+    language = frame->GetLanguage();
+  options.SetLanguage(language.name, language.version);
   return EvaluateExpression(expr, options);
 }
 
@@ -1218,7 +1222,7 @@ lldb::LanguageType SBFrame::GuessLanguage() const {
     if (stop_locker.TryLock(&process->GetRunLock())) {
       frame = exe_ctx.GetFramePtr();
       if (frame) {
-        return frame->GuessLanguage();
+        return frame->GuessLanguage().AsLanguageType();
       }
     }
   }
diff --git a/lldb/source/Breakpoint/Watchpoint.cpp b/lldb/source/Breakpoint/Watchpoint.cpp
index a128ced5750449..edb1a0e93460c4 100644
--- a/lldb/source/Breakpoint/Watchpoint.cpp
+++ b/lldb/source/Breakpoint/Watchpoint.cpp
@@ -460,9 +460,8 @@ void Watchpoint::SetCondition(const char *condition) {
     // Pass nullptr for expr_prefix (no translation-unit level definitions).
     Status error;
     m_condition_up.reset(m_target.GetUserExpressionForLanguage(
-        condition, llvm::StringRef(), lldb::eLanguageTypeUnknown,
-        UserExpression::eResultTypeAny, EvaluateExpressionOptions(), nullptr,
-        error));
+        condition, {}, {}, UserExpression::eResultTypeAny,
+        EvaluateExpressionOptions(), nullptr, error));
     if (error.Fail()) {
       // FIXME: Log something...
       m_condition_up.reset();
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index e1255f37d9bc58..f0b37e6832eab0 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -96,7 +96,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
   // Either Swift was explicitly specified, or the frame is Swift.
   lldb::LanguageType language = m_expr_options.language;
   if (language == lldb::eLanguageTypeUnknown && frame)
-    language = frame->GuessLanguage();
+    language = frame->GuessLanguage().AsLanguageType();
 
   // Add a hint if object description was requested, but no description
   // function was implemented.
diff --git a/lldb/source/Commands/CommandObjectType.cpp b/lldb/source/Commands/CommandObjectType.cpp
index 97489bdc2d9c28..46537dd1b98aaf 100644
--- a/lldb/source/Commands/CommandObjectType.cpp
+++ b/lldb/source/Commands/CommandObjectType.cpp
@@ -2509,7 +2509,7 @@ class CommandObjectTypeLookup : public CommandObjectRaw {
     if (!frame)
       return lang_type;
 
-    lang_type = frame->GuessLanguage();
+    lang_type = frame->GuessLanguage().AsLanguageType();
     if (lang_type != lldb::eLanguageTypeUnknown)
       return lang_type;
 
diff --git a/lldb/source/Expression/LLVMUserExpression.cpp b/lldb/source/Expression/LLVMUserExpression.cpp
index 9c31cc84bf8f8b..1434011c80ad81 100644
--- a/lldb/source/Expression/LLVMUserExpression.cpp
+++ b/lldb/source/Expression/LLVMUserExpression.cpp
@@ -42,7 +42,7 @@ char LLVMUserExpression::ID;
 LLVMUserExpression::LLVMUserExpression(ExecutionContextScope &exe_scope,
                                        llvm::StringRef expr,
                                        llvm::StringRef prefix,
-                                       lldb::LanguageType language,
+                                       SourceLanguage language,
                                        ResultType desired_type,
                                        const EvaluateExpressionOptions &options)
     : UserExpression(exe_scope, expr, prefix, language, desired_type, options),
diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp
index c181712a2f0b24..84578c7f9ad272 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -39,6 +39,7 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/StreamString.h"
+#include "llvm/BinaryFormat/Dwarf.h"
 
 using namespace lldb_private;
 
@@ -46,8 +47,7 @@ char UserExpression::ID;
 
 UserExpression::UserExpression(ExecutionContextScope &exe_scope,
                                llvm::StringRef expr, llvm::StringRef prefix,
-                               lldb::LanguageType language,
-                               ResultType desired_type,
+                               SourceLanguage language, ResultType desired_type,
                                const EvaluateExpressionOptions &options)
     : Expression(exe_scope), m_expr_text(std::string(expr)),
       m_expr_prefix(std::string(prefix)), m_language(language),
@@ -176,7 +176,7 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
   }
 
   lldb_private::ExecutionPolicy execution_policy = options.GetExecutionPolicy();
-  lldb::LanguageType language = options.GetLanguage();
+  SourceLanguage language = options.GetLanguage();
   const ResultType desired_type = options.DoesCoerceToId()
                                       ? UserExpression::eResultTypeId
                                       : UserExpression::eResultTypeAny;
@@ -242,7 +242,7 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
   // If the language was not specified in the expression command, set it to the
   // language in the target's properties if specified, else default to the
   // langage for the frame.
-  if (language == lldb::eLanguageTypeUnknown) {
+  if (!language.name) {
     if (target->GetLanguage() != lldb::eLanguageTypeUnknown)
       language = target->GetLanguage();
     else if (StackFrame *frame = exe_ctx.GetFramePtr())
@@ -384,7 +384,8 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
       } else {
         if (expr_result) {
           result_valobj_sp = expr_result->GetValueObject();
-          result_valobj_sp->SetPreferredDisplayLanguage(language);
+          result_valobj_sp->SetPreferredDisplayLanguage(
+              language.AsLanguageType());
 
           LLDB_LOG(log,
                    "== [UserExpression::Evaluate] Execution completed "
@@ -426,7 +427,8 @@ UserExpression::Execute(DiagnosticManager &diagnostic_manager,
   Target *target = exe_ctx.GetTargetPtr();
   if (options.GetSuppressPersistentResult() && result_var && target) {
     if (auto *persistent_state =
-            target->GetPersistentExpressionStateForLanguage(m_language))
+            target->GetPersistentExpressionStateForLanguage(
+                m_language.AsLanguageType()))
       persistent_state->RemovePersistentVariable(result_var);
   }
   return expr_result;
diff --git a/lldb/source/Expression/UtilityFunction.cpp b/lldb/source/Expression/UtilityFunction.cpp
index d7a3c9d41d0484..7b34c2c2ff76d5 100644
--- a/lldb/source/Expression/UtilityFunction.cpp
+++ b/lldb/source/Expression/UtilityFunction.cpp
@@ -80,8 +80,8 @@ FunctionCaller *UtilityFunction::MakeFunctionCaller(
   name.append("-caller");
 
   m_caller_up.reset(process_sp->GetTarget().GetFunctionCallerForLanguage(
-      Language(), return_type, impl_code_address, arg_value_list, name.c_str(),
-      error));
+      Language().AsLanguageType(), return_type, impl_code_address,
+      arg_value_list, name.c_str(), error));
   if (error.Fail()) {
 
     return nullptr;
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index f48bd...
[truncated]

Copy link

github-actions bot commented Apr 24, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

SymbolContext sc =
GetSymbolContext(eSymbolContextFunction | eSymbolContextSymbol);
if (sc.function)
lang_type = LanguageType(sc.function->GetMangled().GuessLanguage());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works because the SourceLanguage(LanguageType) constructor isn't marked explicit. Would it be clearer to mark it explicit and instead rewrite this as SourceLanguage(sc.function->GetMangled().GuessLanguage())

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I also wanted to make clear that my intention would be to eventually deprecate eLanguageType and replace all uses with the superior split SourceLanguage. I just had to stop somewhere for this first iteration of the patch. But this is not meant to be the final state.

Copy link
Member

@Michael137 Michael137 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (with minor comment)

Comment on lines 70 to 82
/// Set the language using a pair of language code and version as
/// defined by the DWARF 6 specification.
/// WARNING: These codes may change until DWARF 6 is finalized.
void SetLanguage(uint16_t dwarf_lname_code, uint32_t dwarf_lversion);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels wrong. How would you feel about exposing SourceLanguage as SBSourceLanguage and having a SetLanguage overload that takes that as an argument? It seems like a small difference, but I think it conceptually makes sense to initialize an SBSourceLanguage with a dwarf code/version.

As an added benefit it's also more future proof, if we need to extend the class, or want to use it elsewhere in the SB API where we currently use lldb::LanguageType. A quick grep shows 21 instances of lldb::LanguageType and if we thread this through more I could imagine wanting to have the ability of passing an SBSourceLanguage like we do here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JDevlieghere Can you take another look? This version generates a header file in the API directory from LLVM's Dwarf.def and (at least locally) passes the testsuite and installs into the include/API directory.

adrian-prantl added a commit that referenced this pull request Apr 26, 2024
This patch adds DWARF constants for DW_AT_language_name and
DW_AT_language_version to Dwarf.def and Dwarf.h.

While the DWARF 6 spec is not finalized, the constants are published on
the DWARF website and considered stable, with idea being that the list
published on dwarfstd.org is the authoritative source that is being
continuously updated between DWARF revisions, as new languages are being
developed.

https://dwarfstd.org/languages-v6.html

My main motivation for adding this is to use in
#89981
Copy link

github-actions bot commented Apr 26, 2024

✅ With the latest revision this PR passed the Python code formatter.

@adrian-prantl adrian-prantl force-pushed the set-language-version branch 2 times, most recently from 7620297 to 280caa3 Compare April 26, 2024 20:15
lldb/utils/TableGen/LLDBSBAPIDWARFEnum.cpp Show resolved Hide resolved
lldb/utils/TableGen/LLDBSBAPIDWARFEnum.cpp Outdated Show resolved Hide resolved
@@ -10,6 +10,7 @@ config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
config.lldb_obj_root = "@LLDB_BINARY_DIR@"
config.lldb_src_root = "@LLDB_SOURCE_DIR@"
config.lldb_built_include_dir = lit_config.substitute("@LLDB_BINARY_DIR@/include")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is always @LLDB_BINARY_DIR@/include, why not use

config.lldb_built_include_dir = os.path.join(config.lldb_obj_root. 'include')

or even better: don't create a new variable at all and have dotest derive it from config.lldb_obj_root?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JDevlieghere I made the change — I'm not sure it does what you were hoping for though, since we still need to thread through lldb_obj_root.

@adrian-prantl adrian-prantl force-pushed the set-language-version branch 2 times, most recently from e561968 to e9b2e98 Compare April 26, 2024 23:00
@@ -239,6 +239,9 @@ def delete_module_cache(path):
if is_configured("server"):
dotest_cmd += ["--server", config.server]

if is_configured("lldb_built_include_dir"):
dotest_cmd += ["--lldb-built-include-dir", config.lldb_built_include_dir]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do:

 is_configured("config.lldb_obj_root"):
  dotest_cmd += ["--lldb-built-include-dir", os.path.join(config.lldb_obj_root, 'include')] 

you can get rid of config.lldb_built_include_dir.

@adrian-prantl adrian-prantl force-pushed the set-language-version branch 2 times, most recently from 19cfcf3 to 6d91ad8 Compare April 29, 2024 17:14
Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bearing with me! LGTM.

@adrian-prantl adrian-prantl force-pushed the set-language-version branch 3 times, most recently from 00fdda9 to c4b1ff6 Compare April 29, 2024 20:10
that separates out language and version. To avoid reinventing the
wheel and introducing subtle incompatibilities, this API uses the
table of languages and versiond defined by the upcoming DWARF 6
standard (https://dwarfstd.org/languages-v6.html). While the DWARF 6
spec is not finialized, the list of languages is broadly considered
stable.

The primary motivation for this is to allow the Swift language plugin
to switch between language dialects between, e.g., Swift 5.9 and 6.0
with out introducing a ton of new language codes. On the main branch
this change is considered NFC.
@adrian-prantl adrian-prantl merged commit 975eca0 into llvm:main Apr 29, 2024
4 checks passed
adrian-prantl added a commit to adrian-prantl/llvm-project that referenced this pull request Apr 30, 2024
that separates out language and version. To avoid reinventing the wheel
and introducing subtle incompatibilities, this API uses the table of
languages and versiond defined by the upcoming DWARF 6 standard
(https://dwarfstd.org/languages-v6.html). While the DWARF 6 spec is not
finialized, the list of languages is broadly considered stable.

The primary motivation for this is to allow the Swift language plugin to
switch between language dialects between, e.g., Swift 5.9 and 6.0 with
out introducing a ton of new language codes. On the main branch this
change is considered NFC.

Depends on llvm#89980

(cherry picked from commit 975eca0)

 Conflicts:
	lldb/packages/Python/lldbsuite/test/dotest_args.py
	lldb/source/Commands/CommandObjectDWIMPrint.cpp
	lldb/source/Expression/UserExpression.cpp
adrian-prantl added a commit to adrian-prantl/llvm-project that referenced this pull request Apr 30, 2024
This patch adds DWARF constants for DW_AT_language_name and
DW_AT_language_version to Dwarf.def and Dwarf.h.

While the DWARF 6 spec is not finalized, the constants are published on
the DWARF website and considered stable, with idea being that the list
published on dwarfstd.org is the authoritative source that is being
continuously updated between DWARF revisions, as new languages are being
developed.

https://dwarfstd.org/languages-v6.html

My main motivation for adding this is to use in
llvm#89981

(cherry picked from commit 300340f)
adrian-prantl added a commit to adrian-prantl/llvm-project that referenced this pull request Apr 30, 2024
that separates out language and version. To avoid reinventing the wheel
and introducing subtle incompatibilities, this API uses the table of
languages and versiond defined by the upcoming DWARF 6 standard
(https://dwarfstd.org/languages-v6.html). While the DWARF 6 spec is not
finialized, the list of languages is broadly considered stable.

The primary motivation for this is to allow the Swift language plugin to
switch between language dialects between, e.g., Swift 5.9 and 6.0 with
out introducing a ton of new language codes. On the main branch this
change is considered NFC.

Depends on llvm#89980

(cherry picked from commit 975eca0)

 Conflicts:
	lldb/packages/Python/lldbsuite/test/dotest_args.py
	lldb/source/Commands/CommandObjectDWIMPrint.cpp
	lldb/source/Expression/UserExpression.cpp
adrian-prantl added a commit to adrian-prantl/llvm-project that referenced this pull request Apr 30, 2024
This patch adds DWARF constants for DW_AT_language_name and
DW_AT_language_version to Dwarf.def and Dwarf.h.

While the DWARF 6 spec is not finalized, the constants are published on
the DWARF website and considered stable, with idea being that the list
published on dwarfstd.org is the authoritative source that is being
continuously updated between DWARF revisions, as new languages are being
developed.

https://dwarfstd.org/languages-v6.html

My main motivation for adding this is to use in
llvm#89981

(cherry picked from commit 300340f)
@nico
Copy link
Contributor

nico commented Apr 30, 2024

Why does this use tablegen to parse a .def file?

Can't you get the same behavior without tablegen, using normal xmacro techniques, something like

enum SBSourceLanguageName : uint16_t {

#define HANDLE_DW_LNAME(ID, NAME, DESC, LOWER_BOUND) \
  eLanguageName ## NAME = ID,

#include "llvm/include/llvm/BinaryFormat/Dwarf.def"

#undef HANDLE_DW_LNAME

};

? Why bring tablegen into this?

(Sorry if I'm missing something obvious.)

@nico
Copy link
Contributor

nico commented Apr 30, 2024

I left another comment on the commit that doesn't show up here (975eca0).

If you do want to keep the generated file: All other tablegen invocations use ".inc" for tablegen output. Maybe this could match that convention?

(But not needing a generated file seems even nicer.)

@adrian-prantl
Copy link
Collaborator Author

Why does this use tablegen to parse a .def file?

Can't you get the same behavior without tablegen, using normal xmacro techniques, something like

enum SBSourceLanguageName : uint16_t {

#define HANDLE_DW_LNAME(ID, NAME, DESC, LOWER_BOUND) \
  eLanguageName ## NAME = ID,

#include "llvm/include/llvm/BinaryFormat/Dwarf.def"

#undef HANDLE_DW_LNAME

};

? Why bring tablegen into this?

(Sorry if I'm missing something obvious.)

That's a very valid question to ask!
The reason is that the header files produced here are part of the LLDB scripting API and are going to be processed by tools that are not full compilers and fully expanding this at compile-time avoids any compatibility issues with not fully compliant interface generators. The reason why this is implemented in C++ is because I don't want to assume that there is sed or something equivalent available on the build system especially on platforms like Windows.

@adrian-prantl
Copy link
Collaborator Author

That's also why it's a .h file and not a .inc.

@adrian-prantl
Copy link
Collaborator Author

Btw. github doesn't let you see older revisions, but the code snippet you posted that includes a .def file was exactly my first version of this patch. You can still find @JDevlieghere's comment asking me to change it though :-)

Another thing worth noting is that the public LLDB API can't expect an LLVM header to be present either.

@nico
Copy link
Contributor

nico commented Apr 30, 2024

Thanks for the explanation! What kind of tool reads this file?

It's still weird to use tblgen to process non-td files imho. We have a bunch of places that run python scripts as part of the build (clang/utils/bundle_resources.py, clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py, compiler-rt/lib/sanitizer_common/scripts/gen_dynamic_list.py, clang-tools-extra/clangd/quality/CompletionModelCodegen.py) – maybe that could work here?

@bulbazord
Copy link
Member

This doesn't actually copy SBLanguages.h into the framework, so this breaks the framework.
These tests now fail:

********************
********************
Unresolved Tests (5):
  lldb-api :: api/check_public_api_headers/TestPublicAPIHeaders.py
  lldb-api :: api/multiple-debuggers/TestMultipleDebuggers.py
  lldb-api :: api/multiple-targets/TestMultipleTargets.py
  lldb-api :: api/multithreaded/TestMultithreaded.py
  lldb-api :: functionalities/plugins/command_plugin/TestPluginCommands.py

@JDevlieghere
Copy link
Member

JDevlieghere commented Apr 30, 2024

Thanks for the explanation! What kind of tool reads this file?

One such tool is SWIG, which happens to support macro expansion, but we have other tools downstream that don't.

It's still weird to use tblgen to process non-td files imho. We have a bunch of places that run python scripts as part of the build (clang/utils/bundle_resources.py, clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py, compiler-rt/lib/sanitizer_common/scripts/gen_dynamic_list.py, clang-tools-extra/clangd/quality/CompletionModelCodegen.py) – maybe that could work here?

Yeah I agree and that's my fault for suggesting it in the first place to Adrian. The idea was to avoid creating a new tool. We considered Python but weren't sure it was a hard build dependency (even though LLDB relies on it extensively, you can actually turn it off with a CMake flag). If that's the case (and it sounds like it is!) then that's certainly a viable option.

@JDevlieghere
Copy link
Member

#90753

adrian-prantl added a commit to adrian-prantl/llvm-project that referenced this pull request May 1, 2024
that separates out language and version. To avoid reinventing the wheel
and introducing subtle incompatibilities, this API uses the table of
languages and versiond defined by the upcoming DWARF 6 standard
(https://dwarfstd.org/languages-v6.html). While the DWARF 6 spec is not
finialized, the list of languages is broadly considered stable.

The primary motivation for this is to allow the Swift language plugin to
switch between language dialects between, e.g., Swift 5.9 and 6.0 with
out introducing a ton of new language codes. On the main branch this
change is considered NFC.

Depends on llvm#89980

(cherry picked from commit 975eca0)

 Conflicts:
	lldb/packages/Python/lldbsuite/test/dotest_args.py
	lldb/source/Commands/CommandObjectDWIMPrint.cpp
	lldb/source/Expression/UserExpression.cpp
adrian-prantl added a commit to adrian-prantl/llvm-project that referenced this pull request May 1, 2024
This patch adds DWARF constants for DW_AT_language_name and
DW_AT_language_version to Dwarf.def and Dwarf.h.

While the DWARF 6 spec is not finalized, the constants are published on
the DWARF website and considered stable, with idea being that the list
published on dwarfstd.org is the authoritative source that is being
continuously updated between DWARF revisions, as new languages are being
developed.

https://dwarfstd.org/languages-v6.html

My main motivation for adding this is to use in
llvm#89981

(cherry picked from commit 300340f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants