[lldb] Remove CommandReturnObject::AppendRawError #171517
Merged
Conversation
This file contains hidden or 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
Member
|
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesRemove CommandReturnObject::AppendRawError and replace its two uses with AppendError, which correctly prefixes the message with "error:". The comment for the method is outdated and the prefixing is clearly desired in both situations. Full diff: https://github.com/llvm/llvm-project/pull/171517.diff 5 Files Affected:
diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index 0742f1b836f5e..f6e60840256a4 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -129,8 +129,6 @@ class CommandReturnObject {
void AppendError(llvm::StringRef in_string);
- void AppendRawError(llvm::StringRef in_string);
-
void AppendErrorWithFormat(const char *format, ...)
__attribute__((format(printf, 2, 3)));
diff --git a/lldb/source/Commands/CommandObjectMultiword.cpp b/lldb/source/Commands/CommandObjectMultiword.cpp
index a369557cca845..e08b33cdce940 100644
--- a/lldb/source/Commands/CommandObjectMultiword.cpp
+++ b/lldb/source/Commands/CommandObjectMultiword.cpp
@@ -205,7 +205,7 @@ void CommandObjectMultiword::Execute(const char *args_string,
.str());
}
error_msg.append("\n");
- result.AppendRawError(error_msg.c_str());
+ result.AppendError(error_msg);
}
std::string CommandObjectMultiword::GetSubcommandsHintText() {
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index ffcc9ceeb2a93..cb6acfc9c29a9 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -3708,7 +3708,7 @@ CommandInterpreter::ResolveCommandImpl(std::string &command_line,
for (uint32_t i = 0; i < num_matches; ++i) {
error_msg.Printf("\t%s\n", matches.GetStringAtIndex(i));
}
- result.AppendRawError(error_msg.GetString());
+ result.AppendError(error_msg.GetString());
}
} else {
// We didn't have only one match, otherwise we wouldn't get here.
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp
index ef5bfae1bd1bd..85b058e97a679 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -173,15 +173,6 @@ StructuredData::ObjectSP CommandReturnObject::GetErrorData() {
return Serialize(m_diagnostics);
}
-// Similar to AppendError, but do not prepend 'Status: ' to message, and don't
-// append "\n" to the end of it.
-
-void CommandReturnObject::AppendRawError(llvm::StringRef in_string) {
- SetStatus(eReturnStatusFailed);
- assert(!in_string.empty() && "Expected a non-empty error message");
- GetErrorStream() << in_string;
-}
-
void CommandReturnObject::SetStatus(ReturnStatus status) { m_status = status; }
ReturnStatus CommandReturnObject::GetStatus() const { return m_status; }
diff --git a/lldb/test/Shell/Commands/command-wrong-subcommand-error-msg.test b/lldb/test/Shell/Commands/command-wrong-subcommand-error-msg.test
index 5365f81c19587..968a4d6fb9682 100644
--- a/lldb/test/Shell/Commands/command-wrong-subcommand-error-msg.test
+++ b/lldb/test/Shell/Commands/command-wrong-subcommand-error-msg.test
@@ -4,5 +4,5 @@
# RUN: not %lldb -b -o 'breakpoint foo' %t.out -o exit 2>&1 | FileCheck %s --check-prefix BP-MSG
# RUN: not %lldb -b -o 'watchpoint set foo' %t.out -o exit 2>&1 | FileCheck %s --check-prefix WP-MSG
# CHECK: at main.c:2:21
-# BP-MSG: "foo" is not a valid subcommand of "breakpoint". Valid subcommands are: add, clear, command, delete, disable, and others. Use "help breakpoint" to find out more.
-# WP-MSG: "foo" is not a valid subcommand of "watchpoint set". Valid subcommands are: expression, variable. Use "help watchpoint set" to find out more.
\ No newline at end of file
+# BP-MSG: error: "foo" is not a valid subcommand of "breakpoint". Valid subcommands are: add, clear, command, delete, disable, and others. Use "help breakpoint" to find out more.
+# WP-MSG: error: "foo" is not a valid subcommand of "watchpoint set". Valid subcommands are: expression, variable. Use "help watchpoint set" to find out more.
|
Member
Author
|
I observed the unexpected behavior when writing up #171514 |
bulbazord
approved these changes
Dec 9, 2025
🐧 Linux x64 Test Results
Failed Tests(click on a test name to see its output) lldb-apilldb-api.functionalities/abbreviation/TestAbbreviations.pylldb-api.functionalities/ambigous_commands/TestAmbiguousCommands.pyIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
JDevlieghere
force-pushed
the
RemoveAppendRawError
branch
from
b64da22 to
7673f13
Compare
Remove CommandReturnObject::AppendRawError and replace its two uses with AppendError, which correctly prefixes the message with "error:". The comment for the method is outdated and the prefixing is clearly desired in both situations.
JDevlieghere
force-pushed
the
RemoveAppendRawError
branch
from
7673f13 to
59a37b3
Compare
jimingham
reviewed
Dec 9, 2025
| for (uint32_t i = 0; i < num_matches; ++i) { | ||
| error_msg.Printf("\t%s\n", matches.GetStringAtIndex(i)); | ||
| } | ||
| result.AppendRawError(error_msg.GetString()); |
Collaborator
There was a problem hiding this comment.
Do you also want to uncapitalize Ambiguous on line 3705 above as well.
Collaborator
There was a problem hiding this comment.
Also remove the pointless \n at the end.
jimingham
reviewed
Dec 9, 2025
| self.assertEqual( | ||
| result.GetError(), | ||
| "Ambiguous command 'co'. Possible matches:\n\tcommand\n\tcontinue\n\tcorefile\n", | ||
| "error: Ambiguous command 'co'. Possible matches:\n\tcommand\n\tcontinue\n\tcorefile\n", |
Collaborator
There was a problem hiding this comment.
This should be "error: ambiguous..." right?
Member
Author
There was a problem hiding this comment.
Yup, already put up #171519. I'll do the same for the newline.
jimingham
reviewed
Dec 9, 2025
| " Use \"help " + GetCommandName() + "\" to find out more.") | ||
| .str()); | ||
| } | ||
| error_msg.append("\n"); |
Collaborator
There was a problem hiding this comment.
AppendError ends up trimming this "\n" from the end of the string, then putting another on on. So there's no reason to keep the append("\n") here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Remove
CommandReturnObject::AppendRawErrorand replace its two uses withAppendError, which correctly prefixes the message witherror:. The comment for the method is outdated and the prefixing is clearly desired in both situations.