llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb <details> <summary>Changes</summary> 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). --- Full diff: https://github.com/llvm/llvm-project/pull/67055.diff 4 Files Affected: - (modified) lldb/include/lldb/Expression/UserExpression.h (+3-2) - (modified) lldb/source/Expression/UserExpression.cpp (+8-10) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp (+14-15) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h (+1-1) ``````````diff 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; `````````` </details> https://github.com/llvm/llvm-project/pull/67055 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
