[lldb] Unify implementation of CommandReturnObject::SetError(NFC) #110707
+17
−19
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
This is a cleanup that moves the API towards value semantics.
|
@llvm/pr-subscribers-lldb Author: Adrian Prantl (adrian-prantl) ChangesThis is a cleanup that moves the API towards value semantics. Full diff: https://github.com/llvm/llvm-project/pull/110707.diff 9 Files Affected:
diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index 8c4dcb54d708f0..8f6c9f123b7690 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -131,7 +131,7 @@ class CommandReturnObject {
AppendError(llvm::formatv(format, std::forward<Args>(args)...).str());
}
- void SetError(const Status &error, const char *fallback_error_cstr = nullptr);
+ void SetError(Status error);
void SetError(llvm::Error error);
diff --git a/lldb/source/API/SBCommandReturnObject.cpp b/lldb/source/API/SBCommandReturnObject.cpp
index 7d2c102b3d8c14..d0cdebe8c64911 100644
--- a/lldb/source/API/SBCommandReturnObject.cpp
+++ b/lldb/source/API/SBCommandReturnObject.cpp
@@ -326,10 +326,10 @@ void SBCommandReturnObject::SetError(lldb::SBError &error,
const char *fallback_error_cstr) {
LLDB_INSTRUMENT_VA(this, error, fallback_error_cstr);
- if (error.IsValid())
- ref().SetError(error.ref(), fallback_error_cstr);
+ if (error.IsValid() && !error.Fail())
+ ref().SetError(error.ref().Clone());
else if (fallback_error_cstr)
- ref().SetError(Status(), fallback_error_cstr);
+ ref().SetError(Status::FromErrorString(fallback_error_cstr));
}
void SBCommandReturnObject::SetError(const char *error_cstr) {
diff --git a/lldb/source/Commands/CommandObjectBreakpointCommand.cpp b/lldb/source/Commands/CommandObjectBreakpointCommand.cpp
index b668cd0f7c22f0..ac2db5973effa9 100644
--- a/lldb/source/Commands/CommandObjectBreakpointCommand.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpointCommand.cpp
@@ -389,7 +389,7 @@ are no syntax errors may indicate that a function was declared but never called.
m_bp_options_vec, result);
}
if (!error.Success())
- result.SetError(error);
+ result.SetError(std::move(error));
} else {
// Special handling for one-liner specified inline.
if (m_options.m_use_one_liner)
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index b7cd955e00203d..2e1d6e6e5af996 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -202,7 +202,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// If the expression failed, return an error.
if (expr_result != eExpressionCompleted) {
if (valobj_sp)
- result.SetError(valobj_sp->GetError());
+ result.SetError(valobj_sp->GetError().Clone());
else
result.AppendErrorWithFormatv(
"unknown error evaluating expression `{0}`", expr);
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
index 9722c85a79b784..a72b409d21ed83 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -647,8 +647,8 @@ void CommandObjectExpression::DoExecute(llvm::StringRef command,
initialize = true;
repl_sp = target.GetREPL(repl_error, m_command_options.language,
nullptr, true);
- if (!repl_error.Success()) {
- result.SetError(repl_error);
+ if (repl_error.Fail()) {
+ result.SetError(std::move(repl_error));
return;
}
}
@@ -668,7 +668,7 @@ void CommandObjectExpression::DoExecute(llvm::StringRef command,
repl_error = Status::FromErrorStringWithFormat(
"Couldn't create a REPL for %s",
Language::GetNameForLanguageType(m_command_options.language));
- result.SetError(repl_error);
+ result.SetError(std::move(repl_error));
return;
}
}
diff --git a/lldb/source/Commands/CommandObjectMemoryTag.cpp b/lldb/source/Commands/CommandObjectMemoryTag.cpp
index bc76319018da96..23fad06518366f 100644
--- a/lldb/source/Commands/CommandObjectMemoryTag.cpp
+++ b/lldb/source/Commands/CommandObjectMemoryTag.cpp
@@ -290,7 +290,7 @@ class CommandObjectMemoryTagWrite : public CommandObjectParsed {
tagged_range->GetByteSize(), tags);
if (status.Fail()) {
- result.SetError(status);
+ result.SetError(std::move(status));
return;
}
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index 10e761fa8de1ae..e950fb346c253b 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -2775,7 +2775,7 @@ class CommandObjectTargetModulesAdd : public CommandObjectParsed {
result.AppendErrorWithFormat(
"Unable to locate the executable or symbol file with UUID %s",
strm.GetData());
- result.SetError(error);
+ result.SetError(std::move(error));
return;
}
} else {
@@ -4409,7 +4409,7 @@ class CommandObjectTargetSymbolsAdd : public CommandObjectParsed {
return AddModuleSymbols(m_exe_ctx.GetTargetPtr(), module_spec, flush,
result);
} else {
- result.SetError(error);
+ result.SetError(std::move(error));
}
return false;
}
diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp
index edbec0e305db74..bc1f5c39e702c6 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -622,7 +622,7 @@ class CommandObjectThreadStepWithTypeAndScope : public CommandObjectParsed {
result.SetStatus(eReturnStatusSuccessContinuingNoResult);
}
} else {
- result.SetError(new_plan_status);
+ result.SetError(std::move(new_plan_status));
}
}
@@ -1046,7 +1046,7 @@ class CommandObjectThreadUntil : public CommandObjectParsed {
new_plan_sp->SetIsControllingPlan(true);
new_plan_sp->SetOkayToDiscard(false);
} else {
- result.SetError(new_plan_status);
+ result.SetError(std::move(new_plan_status));
return;
}
} else {
@@ -1734,7 +1734,7 @@ class CommandObjectThreadJump : public CommandObjectParsed {
Status err = thread->JumpToLine(file, line, m_options.m_force, &warnings);
if (err.Fail()) {
- result.SetError(err);
+ result.SetError(std::move(err));
return;
}
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp
index 0bc58124f3941f..d5da73c00a5209 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -107,10 +107,8 @@ void CommandReturnObject::AppendError(llvm::StringRef in_string) {
error(GetErrorStream()) << msg << '\n';
}
-void CommandReturnObject::SetError(const Status &error,
- const char *fallback_error_cstr) {
- if (error.Fail())
- AppendError(error.AsCString(fallback_error_cstr));
+void CommandReturnObject::SetError(Status error) {
+ SetError(error.takeError());
}
void CommandReturnObject::SetError(llvm::Error error) {
|
|
|
|
I actually think it's fine to change this behavior, I don't think anyone would have expected SetError to AppendError... |
If you feel strongly about this (I actually think you have a point), you could make |
I think both are useful, and indeed, we have both "SetError" and "AppendError" internally in CommandReturnObject, and we use both. I don't think we need to deprecate SetError, but we should make it set the error to what you pass in, and then add AppendError which will add to what's there. If somebody out there was using SBCommandReturnObject::SetError to append to the error, then they just need to switch to the actually appropriate API now we've provided it. |
Sterling-Augustine
pushed a commit
to Sterling-Augustine/llvm-project
that referenced
this pull request
Oct 3, 2024
…vm#110707) This is a cleanup that moves the API towards value semantics.
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.
This is a cleanup that moves the API towards value semantics.