[Lldb-commits] [lldb] [lldb][NFCI] Change parameter type in UserExpression::GetObjectPointer (PR #67055)

Alex Langford via lldb-commits lldb-commits at lists.llvm.org
Thu Sep 21 12:53:44 PDT 2023


https://github.com/bulbazord created https://github.com/llvm/llvm-project/pull/67055

GetObjectPointer (and other related methods) do not need `ConstString` parameters. The string parameter in these methods boil down to getting a StringRef and calling `StackFrame::GetValueForVariableExpressionPath` which takes a `StringRef` parameter. All the users of `GetObjectPointer` (and related methods) end up creating ConstString objects to pass to these methods, but they could just as easily be StringRefs (potentially saving us some allocations in the StringPool).

>From 78a841d60b6867ae54b05f57e18179984df1026e Mon Sep 17 00:00:00 2001
From: Alex Langford <alangford at apple.com>
Date: Thu, 21 Sep 2023 10:09:19 -0700
Subject: [PATCH] [lldb][NFCI] Change parameter type in
 UserExpression::GetObjectPointer

GetObjectPointer (and other related methods) do not need `ConstString`
parameters. The string parameter in these methods boil down to getting a
StringRef and calling `StackFrame::GetValueForVariableExpressionPath`
which takes a `StringRef` parameter. All the users of `GetObjectPointer`
end up creating ConstString objects to pass to these methods, but they
could just as easily be StringRefs (potentially saving us some allocations
in the StringPool).
---
 lldb/include/lldb/Expression/UserExpression.h |  5 ++--
 lldb/source/Expression/UserExpression.cpp     | 18 +++++-------
 .../Clang/ClangUserExpression.cpp             | 29 +++++++++----------
 .../Clang/ClangUserExpression.h               |  2 +-
 4 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/lldb/include/lldb/Expression/UserExpression.h b/lldb/include/lldb/Expression/UserExpression.h
index 2d62fa37a24c2c4..df7a76664f6d5b6 100644
--- a/lldb/include/lldb/Expression/UserExpression.h
+++ b/lldb/include/lldb/Expression/UserExpression.h
@@ -278,7 +278,8 @@ class UserExpression : public Expression {
             lldb::ExpressionVariableSP &result) = 0;
 
   static lldb::addr_t GetObjectPointer(lldb::StackFrameSP frame_sp,
-                                       ConstString &object_name, Status &err);
+                                       llvm::StringRef object_name,
+                                       Status &err);
 
   /// Return ValueObject for a given variable name in the current stack frame
   ///
@@ -295,7 +296,7 @@ class UserExpression : public Expression {
   ///          'nullptr' (and sets the error status parameter 'err').
   static lldb::ValueObjectSP
   GetObjectPointerValueObject(lldb::StackFrameSP frame,
-                              ConstString const &object_name, Status &err);
+                              llvm::StringRef object_name, Status &err);
 
   /// Populate m_in_cplusplus_method and m_in_objectivec_method based on the
   /// environment.
diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp
index e16a335d581a71b..c181712a2f0b243 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -35,7 +35,6 @@
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadPlan.h"
 #include "lldb/Target/ThreadPlanCallUserExpression.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
@@ -99,13 +98,12 @@ bool UserExpression::MatchesContext(ExecutionContext &exe_ctx) {
 }
 
 lldb::ValueObjectSP UserExpression::GetObjectPointerValueObject(
-    lldb::StackFrameSP frame_sp, ConstString const &object_name, Status &err) {
+    lldb::StackFrameSP frame_sp, llvm::StringRef object_name, Status &err) {
   err.Clear();
 
   if (!frame_sp) {
-    err.SetErrorStringWithFormat(
-        "Couldn't load '%s' because the context is incomplete",
-        object_name.AsCString());
+    err.SetErrorStringWithFormatv(
+        "Couldn't load '{0}' because the context is incomplete", object_name);
     return {};
   }
 
@@ -113,7 +111,7 @@ lldb::ValueObjectSP UserExpression::GetObjectPointerValueObject(
   lldb::ValueObjectSP valobj_sp;
 
   return frame_sp->GetValueForVariableExpressionPath(
-      object_name.GetStringRef(), lldb::eNoDynamicValues,
+      object_name, lldb::eNoDynamicValues,
       StackFrame::eExpressionPathOptionCheckPtrVsMember |
           StackFrame::eExpressionPathOptionsNoFragileObjcIvar |
           StackFrame::eExpressionPathOptionsNoSyntheticChildren |
@@ -122,7 +120,7 @@ lldb::ValueObjectSP UserExpression::GetObjectPointerValueObject(
 }
 
 lldb::addr_t UserExpression::GetObjectPointer(lldb::StackFrameSP frame_sp,
-                                              ConstString &object_name,
+                                              llvm::StringRef object_name,
                                               Status &err) {
   auto valobj_sp =
       GetObjectPointerValueObject(std::move(frame_sp), object_name, err);
@@ -133,9 +131,9 @@ lldb::addr_t UserExpression::GetObjectPointer(lldb::StackFrameSP frame_sp,
   lldb::addr_t ret = valobj_sp->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
 
   if (ret == LLDB_INVALID_ADDRESS) {
-    err.SetErrorStringWithFormat(
-        "Couldn't load '%s' because its value couldn't be evaluated",
-        object_name.AsCString());
+    err.SetErrorStringWithFormatv(
+        "Couldn't load '{0}' because its value couldn't be evaluated",
+        object_name);
     return LLDB_INVALID_ADDRESS;
   }
 
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index f9d323d360458ce..68bdd96e8adb03f 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -872,7 +872,7 @@ bool ClangUserExpression::Complete(ExecutionContext &exe_ctx,
 }
 
 lldb::addr_t ClangUserExpression::GetCppObjectPointer(
-    lldb::StackFrameSP frame_sp, ConstString &object_name, Status &err) {
+    lldb::StackFrameSP frame_sp, llvm::StringRef object_name, Status &err) {
   auto valobj_sp =
       GetObjectPointerValueObject(std::move(frame_sp), object_name, err);
 
@@ -889,9 +889,9 @@ lldb::addr_t ClangUserExpression::GetCppObjectPointer(
   lldb::addr_t ret = valobj_sp->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
 
   if (ret == LLDB_INVALID_ADDRESS) {
-    err.SetErrorStringWithFormat(
-        "Couldn't load '%s' because its value couldn't be evaluated",
-        object_name.AsCString());
+    err.SetErrorStringWithFormatv(
+        "Couldn't load '{0}' because its value couldn't be evaluated",
+        object_name);
     return LLDB_INVALID_ADDRESS;
   }
 
@@ -910,19 +910,18 @@ bool ClangUserExpression::AddArguments(ExecutionContext &exe_ctx,
     if (!frame_sp)
       return true;
 
-    ConstString object_name;
-
-    if (m_in_cplusplus_method) {
-      object_name.SetCString("this");
-    } else if (m_in_objectivec_method) {
-      object_name.SetCString("self");
-    } else {
+    if (!m_in_cplusplus_method && !m_in_objectivec_method) {
       diagnostic_manager.PutString(
           eDiagnosticSeverityError,
           "need object pointer but don't know the language");
       return false;
     }
 
+    static constexpr llvm::StringLiteral g_cplusplus_object_name("this");
+    static constexpr llvm::StringLiteral g_objc_object_name("self");
+    llvm::StringRef object_name =
+        m_in_cplusplus_method ? g_cplusplus_object_name : g_objc_object_name;
+
     Status object_ptr_error;
 
     if (m_ctx_obj) {
@@ -942,14 +941,14 @@ bool ClangUserExpression::AddArguments(ExecutionContext &exe_ctx,
     }
 
     if (!object_ptr_error.Success()) {
-      exe_ctx.GetTargetRef().GetDebugger().GetAsyncOutputStream()->Printf(
-          "warning: `%s' is not accessible (substituting 0). %s\n",
-          object_name.AsCString(), object_ptr_error.AsCString());
+      exe_ctx.GetTargetRef().GetDebugger().GetAsyncOutputStream()->Format(
+          "warning: `{0}' is not accessible (substituting 0). {1}\n",
+          object_name, object_ptr_error.AsCString());
       object_ptr = 0;
     }
 
     if (m_in_objectivec_method) {
-      ConstString cmd_name("_cmd");
+      static constexpr llvm::StringLiteral cmd_name("_cmd");
 
       cmd_ptr = GetObjectPointer(frame_sp, cmd_name, object_ptr_error);
 
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
index 0fbeff7f6143133..7a8c095f61183b8 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
@@ -204,7 +204,7 @@ class ClangUserExpression : public LLVMUserExpression {
                         bool for_completion);
 
   lldb::addr_t GetCppObjectPointer(lldb::StackFrameSP frame,
-                                   ConstString &object_name, Status &err);
+                                   llvm::StringRef object_name, Status &err);
 
   /// Defines how the current expression should be wrapped.
   ClangExpressionSourceCode::WrapKind GetWrapKind() const;



More information about the lldb-commits mailing list