[lldb][NFCI] Change parameter type in UserExpression::GetObjectPointer #67055
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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).
|
@llvm/pr-subscribers-lldb ChangesGetObjectPointer (and other related methods) do not need Full diff: https://github.com/llvm/llvm-project/pull/67055.diff 4 Files Affected:
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;
|
JDevlieghere
approved these changes
Sep 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
GetObjectPointer (and other related methods) do not need
ConstStringparameters. The string parameter in these methods boil down to getting a StringRef and callingStackFrame::GetValueForVariableExpressionPathwhich takes aStringRefparameter. All the users ofGetObjectPointer(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).