Add AllowRepeats to SBCommandInterpreterRunOptions. #94786
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
This is useful if you have a transcript of a user session and want to rerun those commands with RunCommandInterpreter. The same functionality is also useful in testing.
|
@llvm/pr-subscribers-lldb Author: None (jimingham) ChangesThis is useful if you have a transcript of a user session and want to rerun those commands with RunCommandInterpreter. The same functionality is also useful in testing. I'm adding it primarily for the second reason. In a subsequent patch, I'm adding the ability to Python based commands to provide their "auto-repeat" command. Among other things, that will allow potentially state destroying user commands to prevent auto-repeat. Testing this with Shell or pexpect tests is not nearly as accurate or convenient as using RunCommandInterpreter, but to use that I need to allow auto-repeat. I think for consistency's sake, having interactive sessions always do auto-repeats is the right choice, though that's a lightly held opinion... Full diff: https://github.com/llvm/llvm-project/pull/94786.diff 6 Files Affected:
diff --git a/lldb/bindings/interface/SBCommandInterpreterRunOptionsDocstrings.i b/lldb/bindings/interface/SBCommandInterpreterRunOptionsDocstrings.i
index b37da0535d18a..a4398d95ed0d1 100644
--- a/lldb/bindings/interface/SBCommandInterpreterRunOptionsDocstrings.i
+++ b/lldb/bindings/interface/SBCommandInterpreterRunOptionsDocstrings.i
@@ -10,5 +10,8 @@ A default SBCommandInterpreterRunOptions object has:
* PrintResults: true
* PrintErrors: true
* AddToHistory: true
+* AllowRepeats false
+Interactive debug sessions always allow repeats, the AllowRepeats
+run option only affects non-interactive sessions.
") lldb::SBCommandInterpreterRunOptions;
diff --git a/lldb/include/lldb/API/SBCommandInterpreterRunOptions.h b/lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
index 69b969267e755..b32a8ca51cd08 100644
--- a/lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
+++ b/lldb/include/lldb/API/SBCommandInterpreterRunOptions.h
@@ -71,6 +71,14 @@ class LLDB_API SBCommandInterpreterRunOptions {
bool GetSpawnThread() const;
void SetSpawnThread(bool);
+
+ bool GetAllowRepeats() const;
+
+ // By default, RunCommandInterpreter will discard repeats if the
+ // IOHandler being used is not interactive. Setting AllowRepeats to true
+ // will override this behavior and always process empty lines in the input
+ // as a repeat command.
+ void SetAllowRepeats(bool);
private:
lldb_private::CommandInterpreterRunOptions *get() const;
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 8863523b2e31f..5c2bcd99681a4 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -93,15 +93,19 @@ class CommandInterpreterRunOptions {
/// \param[in] add_to_history
/// If \b true add the commands to the command history. If \b false, don't
/// add them.
+ /// \param[in] process_repeats
+ /// If \b true then process empty lines as repeat commands even if the
+ /// interpreter is non-interactive.
CommandInterpreterRunOptions(LazyBool stop_on_continue,
LazyBool stop_on_error, LazyBool stop_on_crash,
LazyBool echo_commands, LazyBool echo_comments,
LazyBool print_results, LazyBool print_errors,
- LazyBool add_to_history)
+ LazyBool add_to_history, LazyBool process_repeats)
: m_stop_on_continue(stop_on_continue), m_stop_on_error(stop_on_error),
m_stop_on_crash(stop_on_crash), m_echo_commands(echo_commands),
m_echo_comment_commands(echo_comments), m_print_results(print_results),
- m_print_errors(print_errors), m_add_to_history(add_to_history) {}
+ m_print_errors(print_errors), m_add_to_history(add_to_history),
+ m_allow_repeats(process_repeats) {}
CommandInterpreterRunOptions() = default;
@@ -182,6 +186,11 @@ class CommandInterpreterRunOptions {
void SetSpawnThread(bool spawn_thread) {
m_spawn_thread = spawn_thread ? eLazyBoolYes : eLazyBoolNo;
}
+ bool GetAllowRepeats() const { return DefaultToNo(m_allow_repeats); }
+
+ void SetAllowRepeats(bool allow_repeats) {
+ m_allow_repeats = allow_repeats ? eLazyBoolYes : eLazyBoolNo;
+ }
LazyBool m_stop_on_continue = eLazyBoolCalculate;
LazyBool m_stop_on_error = eLazyBoolCalculate;
@@ -193,6 +202,7 @@ class CommandInterpreterRunOptions {
LazyBool m_add_to_history = eLazyBoolCalculate;
LazyBool m_auto_handle_events;
LazyBool m_spawn_thread;
+ LazyBool m_allow_repeats = eLazyBoolCalculate;
private:
static bool DefaultToYes(LazyBool flag) {
diff --git a/lldb/source/API/SBCommandInterpreterRunOptions.cpp b/lldb/source/API/SBCommandInterpreterRunOptions.cpp
index 6c6b2aa15a792..0c7581d6f1f5b 100644
--- a/lldb/source/API/SBCommandInterpreterRunOptions.cpp
+++ b/lldb/source/API/SBCommandInterpreterRunOptions.cpp
@@ -164,6 +164,18 @@ void SBCommandInterpreterRunOptions::SetSpawnThread(bool spawn_thread) {
m_opaque_up->SetSpawnThread(spawn_thread);
}
+bool SBCommandInterpreterRunOptions::GetAllowRepeats() const {
+ LLDB_INSTRUMENT_VA(this);
+
+ return m_opaque_up->GetAllowRepeats();
+}
+
+void SBCommandInterpreterRunOptions::SetAllowRepeats(bool allow_repeats) {
+ LLDB_INSTRUMENT_VA(this, allow_repeats);
+
+ m_opaque_up->SetAllowRepeats(allow_repeats);
+}
+
lldb_private::CommandInterpreterRunOptions *
SBCommandInterpreterRunOptions::get() const {
return m_opaque_up.get();
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index acd6294cb3f42..95b9bb491abf6 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2707,7 +2707,8 @@ enum {
eHandleCommandFlagEchoCommentCommand = (1u << 3),
eHandleCommandFlagPrintResult = (1u << 4),
eHandleCommandFlagPrintErrors = (1u << 5),
- eHandleCommandFlagStopOnCrash = (1u << 6)
+ eHandleCommandFlagStopOnCrash = (1u << 6),
+ eHandleCommandFlagAllowRepeats = (1u << 7)
};
void CommandInterpreter::HandleCommandsFromFile(
@@ -3129,14 +3130,18 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
return;
const bool is_interactive = io_handler.GetIsInteractive();
- if (!is_interactive) {
+ bool allow_repeats = io_handler.GetFlags().Test(eHandleCommandFlagAllowRepeats);
+
+ if (!is_interactive && !allow_repeats) {
// When we are not interactive, don't execute blank lines. This will happen
// sourcing a commands file. We don't want blank lines to repeat the
// previous command and cause any errors to occur (like redefining an
// alias, get an error and stop parsing the commands file).
+ // But obey the AllowRepeats flag if the user has set it.
if (line.empty())
return;
-
+ }
+ if (!is_interactive) {
// When using a non-interactive file handle (like when sourcing commands
// from a file) we need to echo the command out so we don't just see the
// command output and no command...
@@ -3388,6 +3393,8 @@ CommandInterpreter::GetIOHandler(bool force_create,
flags |= eHandleCommandFlagPrintResult;
if (options->m_print_errors != eLazyBoolNo)
flags |= eHandleCommandFlagPrintErrors;
+ if (options->m_allow_repeats == eLazyBoolYes)
+ flags |= eHandleCommandFlagAllowRepeats;
} else {
flags = eHandleCommandFlagEchoCommand | eHandleCommandFlagPrintResult |
eHandleCommandFlagPrintErrors;
diff --git a/lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py b/lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
index af97493133766..c9fa7e2da7358 100644
--- a/lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
+++ b/lldb/test/API/python_api/interpreter/TestRunCommandInterpreterAPI.py
@@ -47,28 +47,60 @@ def setUp(self):
TestBase.setUp(self)
self.stdin_path = self.getBuildArtifact("stdin.txt")
-
+ self.stdout_path = self.getBuildArtifact("stdout.txt")
+ def run_commands_string(self, command_string, options = lldb.SBCommandInterpreterRunOptions()):
+ """Run the commands in command_string through RunCommandInterpreter.
+ Returns (n_errors, quit_requested, has_crashed, result_string)."""
+
with open(self.stdin_path, "w") as input_handle:
- input_handle.write("nonexistingcommand\nquit")
+ input_handle.write(command_string)
- self.dbg.SetInputFile(open(self.stdin_path, "r"))
+ n_errors = 0
+ quit_requested = False
+ has_crashed = False
+
+ with open(self.stdin_path, "r") as in_fileH, open(self.stdout_path, "w") as out_fileH:
+ self.dbg.SetInputFile(in_fileH)
- # No need to track the output
- devnull = open(os.devnull, "w")
- self.dbg.SetOutputFile(devnull)
- self.dbg.SetErrorFile(devnull)
+ self.dbg.SetOutputFile(out_fileH)
+ self.dbg.SetErrorFile(out_fileH)
+
+ n_errors, quit_requested, has_crashed = self.dbg.RunCommandInterpreter(
+ True, False, options, 0, False, False
+ )
+
+ result_string = None
+ with open(self.stdout_path, "r") as out_fileH:
+ result_string = out_fileH.read()
+
+ print(f"Command: '{command_string}'\nResult:\n{result_string}")
+ return (n_errors, quit_requested, has_crashed, result_string)
+
def test_run_session_with_error_and_quit(self):
"""Run non-existing and quit command returns appropriate values"""
- n_errors, quit_requested, has_crashed = self.dbg.RunCommandInterpreter(
- True, False, lldb.SBCommandInterpreterRunOptions(), 0, False, False
- )
-
+ n_errors, quit_requested, has_crashed, _ = self.run_commands_string(
+ "nonexistingcommand\nquit\n")
self.assertGreater(n_errors, 0)
self.assertTrue(quit_requested)
self.assertFalse(has_crashed)
+ def test_allow_repeat(self):
+ """Try auto-repeat of process launch - the command will fail and
+ the auto-repeat will fail because of no auto-repeat."""
+ options = lldb.SBCommandInterpreterRunOptions()
+ options.SetEchoCommands(False)
+ options.SetAllowRepeats(True)
+
+ n_errors, quit_requested, has_crashed, result_str = self.run_commands_string(
+ "process launch\n\n", options)
+ self.assertEqual(n_errors, 2)
+ self.assertFalse(quit_requested)
+ self.assertFalse(has_crashed)
+
+ self.assertIn("invalid target", result_str)
+ self.assertIn("No auto repeat", result_str)
class SBCommandInterpreterRunOptionsCase(TestBase):
NO_DEBUG_INFO_TESTCASE = True
@@ -86,6 +118,7 @@ def test_command_interpreter_run_options(self):
self.assertTrue(opts.GetPrintResults())
self.assertTrue(opts.GetPrintErrors())
self.assertTrue(opts.GetAddToHistory())
+ self.assertFalse(opts.GetAllowRepeats())
# Invert values
opts.SetStopOnContinue(not opts.GetStopOnContinue())
@@ -95,6 +128,7 @@ def test_command_interpreter_run_options(self):
opts.SetPrintResults(not opts.GetPrintResults())
opts.SetPrintErrors(not opts.GetPrintErrors())
opts.SetAddToHistory(not opts.GetAddToHistory())
+ opts.SetAllowRepeats(not opts.GetAllowRepeats())
# Check the value changed
self.assertTrue(opts.GetStopOnContinue())
@@ -104,3 +138,4 @@ def test_command_interpreter_run_options(self):
self.assertFalse(opts.GetPrintResults())
self.assertFalse(opts.GetPrintErrors())
self.assertFalse(opts.GetAddToHistory())
+ self.assertTrue(opts.GetAllowRepeats())
|
You can test this locally with the following command:git-clang-format --diff b653357141030620ce3e70ea939efbcb71d40657 9d0ec3d57a46d4c9b42124a9ea07cbc18c216b09 -- lldb/include/lldb/API/SBCommandInterpreterRunOptions.h lldb/include/lldb/Interpreter/CommandInterpreter.h lldb/source/API/SBCommandInterpreterRunOptions.cpp lldb/source/Interpreter/CommandInterpreter.cppView the diff from clang-format here.diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 48f6618ab0..8683e2a88a 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -100,8 +100,7 @@ public:
LazyBool stop_on_error, LazyBool stop_on_crash,
LazyBool echo_commands, LazyBool echo_comments,
LazyBool print_results, LazyBool print_errors,
- LazyBool add_to_history,
- LazyBool handle_repeats)
+ LazyBool add_to_history, LazyBool handle_repeats)
: m_stop_on_continue(stop_on_continue), m_stop_on_error(stop_on_error),
m_stop_on_crash(stop_on_crash), m_echo_commands(echo_commands),
m_echo_comment_commands(echo_comments), m_print_results(print_results),
|
|
✅ With the latest revision this PR passed the Python code formatter. |
medismailben
approved these changes
Jun 7, 2024
| CommandInterpreterRunOptions(LazyBool stop_on_continue, | ||
| LazyBool stop_on_error, LazyBool stop_on_crash, | ||
| LazyBool echo_commands, LazyBool echo_comments, | ||
| LazyBool print_results, LazyBool print_errors, | ||
| LazyBool add_to_history) | ||
| LazyBool add_to_history, | ||
| LazyBool process_repeats) |
There was a problem hiding this comment.
nit: not sure what you mean by process_repeats, may be command_repeats would be make more sense here.
There was a problem hiding this comment.
I was trying to stay in line with the other parameter names in this function. So for instance, add_to_history says commands should be added to the history. The option I'm adding is to tell RunCommandInterpreter to process any repeat commands in the input stream.
There was a problem hiding this comment.
Internally in lldb we call the handling of newlines in the interactive interpreter as "repeat commands", and at the the CommandInterpreterRunOptions setting option was "AllowRepeats".
There was a problem hiding this comment.
tell RunCommandInterpreter to process any repeat commands
Oh, process is a verb here --' when I read it I thought you were talking about the Process class which confused me a little ^^
May be we could rename it handle_repeats to avoid misunderstandings. Otherwise LGTM.
JDevlieghere
approved these changes
Jun 7, 2024
Comment on lines
77
to
80
| // By default, RunCommandInterpreter will discard repeats if the | ||
| // IOHandler being used is not interactive. Setting AllowRepeats to true | ||
| // will override this behavior and always process empty lines in the input | ||
| // as a repeat command. |
There was a problem hiding this comment.
These should be /// so they get picked up by Doxygen.
| @@ -182,6 +187,11 @@ class CommandInterpreterRunOptions { | |||
| void SetSpawnThread(bool spawn_thread) { | |||
| m_spawn_thread = spawn_thread ? eLazyBoolYes : eLazyBoolNo; | |||
| } | |||
| bool GetAllowRepeats() const { return DefaultToNo(m_allow_repeats); } | |||
There was a problem hiding this comment.
Nit: newline between SetSpawnThread and GetAllowRepeats?
| @@ -3129,14 +3130,19 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler, | |||
| return; | |||
|
|
|||
| const bool is_interactive = io_handler.GetIsInteractive(); | |||
| if (!is_interactive) { | |||
| bool allow_repeats = | |||
There was a problem hiding this comment.
nit: const bool for consistency with the line above.
nekoshirro
pushed a commit
to nekoshirro/Alchemist-LLVM
that referenced
this pull request
Jun 9, 2024
This is useful if you have a transcript of a user session and want to rerun those commands with RunCommandInterpreter. The same functionality is also useful in testing. I'm adding it primarily for the second reason. In a subsequent patch, I'm adding the ability to Python based commands to provide their "auto-repeat" command. Among other things, that will allow potentially state destroying user commands to prevent auto-repeat. Testing this with Shell or pexpect tests is not nearly as accurate or convenient as using RunCommandInterpreter, but to use that I need to allow auto-repeat. I think for consistency's sake, having interactive sessions always do auto-repeats is the right choice, though that's a lightly held opinion... Signed-off-by: Hafidz Muzakky <[email protected]>
Closed
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 useful if you have a transcript of a user session and want to rerun those commands with RunCommandInterpreter. The same functionality is also useful in testing.
I'm adding it primarily for the second reason. In a subsequent patch, I'm adding the ability to Python based commands to provide their "auto-repeat" command. Among other things, that will allow potentially state destroying user commands to prevent auto-repeat. Testing this with Shell or pexpect tests is not nearly as accurate or convenient as using RunCommandInterpreter, but to use that I need to allow auto-repeat.
I think for consistency's sake, having interactive sessions always do auto-repeats is the right choice, though that's a lightly held opinion...