From 97a6e23c85457a14c91c5800fa03bb872e6f1fa6 Mon Sep 17 00:00:00 2001 From: Michael Christensen Date: Mon, 27 Nov 2023 12:49:24 -0800 Subject: [PATCH 1/7] Add option to pass thread ID to thread select command --- lldb/source/Commands/CommandObjectThread.cpp | 59 +++++++++++++++++-- lldb/source/Commands/Options.td | 5 ++ .../thread/select/TestThreadSelect.py | 23 +++++++- 3 files changed, 78 insertions(+), 9 deletions(-) diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp index a9f5a4f8a4fbd7..9384df319cc221 100644 --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -1129,8 +1129,44 @@ class CommandObjectThreadUntil : public CommandObjectParsed { // CommandObjectThreadSelect +#define LLDB_OPTIONS_thread_select +#include "CommandOptions.inc" + class CommandObjectThreadSelect : public CommandObjectParsed { public: + class CommandOptions : public Options { + public: + CommandOptions() { OptionParsingStarting(nullptr); } + + ~CommandOptions() override = default; + + void OptionParsingStarting(ExecutionContext *execution_context) override { + m_thread_id = false; + } + + Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg, + ExecutionContext *execution_context) override { + const int short_option = m_getopt_table[option_idx].val; + switch (short_option) { + case 't': { + m_thread_id = true; + break; + } + + default: + llvm_unreachable("Unimplemented option"); + } + + return {}; + } + + llvm::ArrayRef GetDefinitions() override { + return llvm::ArrayRef(g_thread_select_options); + } + + bool m_thread_id; + }; + CommandObjectThreadSelect(CommandInterpreter &interpreter) : CommandObjectParsed(interpreter, "thread select", "Change the currently selected thread.", nullptr, @@ -1165,6 +1201,8 @@ class CommandObjectThreadSelect : public CommandObjectParsed { nullptr); } + Options *GetOptions() override { return &m_options; } + protected: void DoExecute(Args &command, CommandReturnObject &result) override { Process *process = m_exe_ctx.GetProcessPtr(); @@ -1173,22 +1211,29 @@ class CommandObjectThreadSelect : public CommandObjectParsed { return; } else if (command.GetArgumentCount() != 1) { result.AppendErrorWithFormat( - "'%s' takes exactly one thread index argument:\nUsage: %s\n", - m_cmd_name.c_str(), m_cmd_syntax.c_str()); + "'%s' takes exactly one thread %s argument:\nUsage: %s\n", + m_cmd_name.c_str(), m_options.m_thread_id ? "ID" : "index", + m_cmd_syntax.c_str()); return; } uint32_t index_id; if (!llvm::to_integer(command.GetArgumentAtIndex(0), index_id)) { - result.AppendErrorWithFormat("Invalid thread index '%s'", + result.AppendErrorWithFormat("Invalid thread %s '%s'", + m_options.m_thread_id ? "ID" : "index", command.GetArgumentAtIndex(0)); return; } - Thread *new_thread = - process->GetThreadList().FindThreadByIndexID(index_id).get(); + Thread *new_thread = nullptr; + if (m_options.m_thread_id) { + new_thread = process->GetThreadList().FindThreadByID(index_id).get(); + } else { + new_thread = process->GetThreadList().FindThreadByIndexID(index_id).get(); + } if (new_thread == nullptr) { - result.AppendErrorWithFormat("invalid thread #%s.\n", + result.AppendErrorWithFormat("invalid thread %s%s.\n", + m_options.m_thread_id ? "ID " : "#", command.GetArgumentAtIndex(0)); return; } @@ -1196,6 +1241,8 @@ class CommandObjectThreadSelect : public CommandObjectParsed { process->GetThreadList().SetSelectedThreadByID(new_thread->GetID(), true); result.SetStatus(eReturnStatusSuccessFinishNoResult); } + + CommandOptions m_options; }; // CommandObjectThreadList diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 542c78be5a12da..23886046df8f67 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -1117,6 +1117,11 @@ let Command = "thread plan list" in { Desc<"Display thread plans for unreported threads">; } +let Command = "thread select" in { + def thread_thread_id : Option<"thread_id", "t">, + Desc<"Provide a thread ID instead of a thread index.">; +} + let Command = "thread trace dump function calls" in { def thread_trace_dump_function_calls_file : Option<"file", "F">, Group<1>, Arg<"Filename">, diff --git a/lldb/test/API/commands/thread/select/TestThreadSelect.py b/lldb/test/API/commands/thread/select/TestThreadSelect.py index 91f8909471bf2b..4d01b82d9d947e 100644 --- a/lldb/test/API/commands/thread/select/TestThreadSelect.py +++ b/lldb/test/API/commands/thread/select/TestThreadSelect.py @@ -12,17 +12,34 @@ def test_invalid_arg(self): self, "// break here", lldb.SBFileSpec("main.cpp") ) - self.expect( - "thread select -1", error=True, startstr="error: Invalid thread index '-1'" - ) self.expect( "thread select 0x1ffffffff", error=True, startstr="error: Invalid thread index '0x1ffffffff'", ) + self.expect( + "thread select -t 0x1ffffffff", + error=True, + startstr="error: Invalid thread ID '0x1ffffffff'", + ) # Parses but not a valid thread id. self.expect( "thread select 0xffffffff", error=True, startstr="error: invalid thread #0xffffffff.", ) + self.expect( + "thread select -t 0xffffffff", + error=True, + startstr="error: invalid thread ID 0xffffffff.", + ) + + def test_thread_select_tid(self): + self.build() + + lldbutil.run_to_source_breakpoint( + self, "// break here", lldb.SBFileSpec("main.cpp") + ) + self.runCmd( + "thread select -t %d" % self.thread().GetThreadID(), + ) From 34045b9b2e04e01fed142ad2d7f4503e69646c9f Mon Sep 17 00:00:00 2001 From: Michael Christensen Date: Tue, 28 Nov 2023 17:44:03 -0800 Subject: [PATCH 2/7] Code review changes -- update thread-id argument name, separate into separate group, and format --- lldb/source/Commands/CommandObjectThread.cpp | 72 +++++++++++-------- lldb/source/Commands/Options.td | 4 +- .../thread/select/TestThreadSelect.py | 16 ++++- 3 files changed, 58 insertions(+), 34 deletions(-) diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp index 9384df319cc221..9f535a1b6c944f 100644 --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -1134,22 +1134,25 @@ class CommandObjectThreadUntil : public CommandObjectParsed { class CommandObjectThreadSelect : public CommandObjectParsed { public: - class CommandOptions : public Options { + class OptionGroupThreadSelect : public OptionGroup { public: - CommandOptions() { OptionParsingStarting(nullptr); } + OptionGroupThreadSelect() { OptionParsingStarting(nullptr); } - ~CommandOptions() override = default; + ~OptionGroupThreadSelect() override = default; void OptionParsingStarting(ExecutionContext *execution_context) override { - m_thread_id = false; + m_thread_id = LLDB_INVALID_THREAD_ID; } Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg, ExecutionContext *execution_context) override { - const int short_option = m_getopt_table[option_idx].val; + const int short_option = g_thread_select_options[option_idx].short_option; switch (short_option) { case 't': { - m_thread_id = true; + if (option_arg.getAsInteger(0, m_thread_id)) { + m_thread_id = LLDB_INVALID_THREAD_ID; + return Status("Invalid thread ID: '%s'.", option_arg.str().c_str()); + } break; } @@ -1164,7 +1167,7 @@ class CommandObjectThreadSelect : public CommandObjectParsed { return llvm::ArrayRef(g_thread_select_options); } - bool m_thread_id; + lldb::tid_t m_thread_id; }; CommandObjectThreadSelect(CommandInterpreter &interpreter) @@ -1179,6 +1182,7 @@ class CommandObjectThreadSelect : public CommandObjectParsed { // Define the first (and only) variant of this arg. thread_idx_arg.arg_type = eArgTypeThreadIndex; thread_idx_arg.arg_repetition = eArgRepeatPlain; + thread_idx_arg.arg_opt_set_association = LLDB_OPT_SET_1; // There is only one variant this argument could be; put it into the // argument entry. @@ -1186,6 +1190,9 @@ class CommandObjectThreadSelect : public CommandObjectParsed { // Push the data for the first argument into the m_arguments vector. m_arguments.push_back(arg); + + m_option_group.Append(&m_options, LLDB_OPT_SET_ALL, LLDB_OPT_SET_2); + m_option_group.Finalize(); } ~CommandObjectThreadSelect() override = default; @@ -1201,7 +1208,7 @@ class CommandObjectThreadSelect : public CommandObjectParsed { nullptr); } - Options *GetOptions() override { return &m_options; } + Options *GetOptions() override { return &m_option_group; } protected: void DoExecute(Args &command, CommandReturnObject &result) override { @@ -1209,40 +1216,47 @@ class CommandObjectThreadSelect : public CommandObjectParsed { if (process == nullptr) { result.AppendError("no process"); return; - } else if (command.GetArgumentCount() != 1) { + } else if (m_options.m_thread_id == LLDB_INVALID_THREAD_ID && command.GetArgumentCount() != 1) { result.AppendErrorWithFormat( - "'%s' takes exactly one thread %s argument:\nUsage: %s\n", - m_cmd_name.c_str(), m_options.m_thread_id ? "ID" : "index", - m_cmd_syntax.c_str()); + "'%s' takes exactly one thread index argument, or a thread ID option:\nUsage: %s\n", + m_cmd_name.c_str(), m_cmd_syntax.c_str()); return; - } - - uint32_t index_id; - if (!llvm::to_integer(command.GetArgumentAtIndex(0), index_id)) { - result.AppendErrorWithFormat("Invalid thread %s '%s'", - m_options.m_thread_id ? "ID" : "index", - command.GetArgumentAtIndex(0)); + } else if (m_options.m_thread_id != LLDB_INVALID_THREAD_ID && command.GetArgumentCount() != 0) { + result.AppendErrorWithFormat( + "'%s' cannot take both a thread ID option and a thread index argument:\nUsage: %s\n", + m_cmd_name.c_str(), m_cmd_syntax.c_str()); return; } Thread *new_thread = nullptr; - if (m_options.m_thread_id) { - new_thread = process->GetThreadList().FindThreadByID(index_id).get(); + if (command.GetArgumentCount() == 1) { + uint32_t index_id; + if (!llvm::to_integer(command.GetArgumentAtIndex(0), index_id)) { + result.AppendErrorWithFormat("Invalid thread index '%s'", + command.GetArgumentAtIndex(0)); + return; + } + new_thread = process->GetThreadList().FindThreadByIndexID(index_id).get(); + if (new_thread == nullptr) { + result.AppendErrorWithFormat("Invalid thread #%s.\n", + command.GetArgumentAtIndex(0)); + return; + } } else { - new_thread = process->GetThreadList().FindThreadByIndexID(index_id).get(); - } - if (new_thread == nullptr) { - result.AppendErrorWithFormat("invalid thread %s%s.\n", - m_options.m_thread_id ? "ID " : "#", - command.GetArgumentAtIndex(0)); - return; + new_thread = process->GetThreadList().FindThreadByID(m_options.m_thread_id).get(); + if (new_thread == nullptr) { + result.AppendErrorWithFormat("Invalid thread ID %lu.\n", + m_options.m_thread_id); + return; + } } process->GetThreadList().SetSelectedThreadByID(new_thread->GetID(), true); result.SetStatus(eReturnStatusSuccessFinishNoResult); } - CommandOptions m_options; + OptionGroupThreadSelect m_options; + OptionGroupOptions m_option_group; }; // CommandObjectThreadList diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 23886046df8f67..558f2fbf962842 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -1118,8 +1118,8 @@ let Command = "thread plan list" in { } let Command = "thread select" in { - def thread_thread_id : Option<"thread_id", "t">, - Desc<"Provide a thread ID instead of a thread index.">; + def thread_select_thread_id : Option<"thread-id", "t">, Group<2>, + Arg<"ThreadID">, Desc<"Provide a thread ID instead of a thread index.">; } let Command = "thread trace dump function calls" in { diff --git a/lldb/test/API/commands/thread/select/TestThreadSelect.py b/lldb/test/API/commands/thread/select/TestThreadSelect.py index 4d01b82d9d947e..2345dc2b2b36a9 100644 --- a/lldb/test/API/commands/thread/select/TestThreadSelect.py +++ b/lldb/test/API/commands/thread/select/TestThreadSelect.py @@ -20,18 +20,28 @@ def test_invalid_arg(self): self.expect( "thread select -t 0x1ffffffff", error=True, - startstr="error: Invalid thread ID '0x1ffffffff'", + startstr="error: Invalid thread ID", + ) + self.expect( + "thread select 1 2 3", + error=True, + startstr="error: 'thread select' takes exactly one thread index argument, or a thread ID option:", + ) + self.expect( + "thread select -t 1234 1", + error=True, + startstr="error: 'thread select' cannot take both a thread ID option and a thread index argument:", ) # Parses but not a valid thread id. self.expect( "thread select 0xffffffff", error=True, - startstr="error: invalid thread #0xffffffff.", + startstr="error: Invalid thread #0xffffffff.", ) self.expect( "thread select -t 0xffffffff", error=True, - startstr="error: invalid thread ID 0xffffffff.", + startstr="error: Invalid thread ID", ) def test_thread_select_tid(self): From 3ef88cd6c82dd08fcbf504283b70049628feb462 Mon Sep 17 00:00:00 2001 From: Michael Christensen Date: Tue, 28 Nov 2023 17:59:24 -0800 Subject: [PATCH 3/7] Fix formatting --- lldb/source/Commands/CommandObjectThread.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp index 9f535a1b6c944f..35618551c2f05e 100644 --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -1216,15 +1216,18 @@ class CommandObjectThreadSelect : public CommandObjectParsed { if (process == nullptr) { result.AppendError("no process"); return; - } else if (m_options.m_thread_id == LLDB_INVALID_THREAD_ID && command.GetArgumentCount() != 1) { + } else if (m_options.m_thread_id == LLDB_INVALID_THREAD_ID && + command.GetArgumentCount() != 1) { result.AppendErrorWithFormat( - "'%s' takes exactly one thread index argument, or a thread ID option:\nUsage: %s\n", + "'%s' takes exactly one thread index argument, or a thread ID " + "option:\nUsage: %s\n", m_cmd_name.c_str(), m_cmd_syntax.c_str()); return; - } else if (m_options.m_thread_id != LLDB_INVALID_THREAD_ID && command.GetArgumentCount() != 0) { - result.AppendErrorWithFormat( - "'%s' cannot take both a thread ID option and a thread index argument:\nUsage: %s\n", - m_cmd_name.c_str(), m_cmd_syntax.c_str()); + } else if (m_options.m_thread_id != LLDB_INVALID_THREAD_ID && + command.GetArgumentCount() != 0) { + result.AppendErrorWithFormat("'%s' cannot take both a thread ID option " + "and a thread index argument:\nUsage: %s\n", + m_cmd_name.c_str(), m_cmd_syntax.c_str()); return; } @@ -1239,11 +1242,12 @@ class CommandObjectThreadSelect : public CommandObjectParsed { new_thread = process->GetThreadList().FindThreadByIndexID(index_id).get(); if (new_thread == nullptr) { result.AppendErrorWithFormat("Invalid thread #%s.\n", - command.GetArgumentAtIndex(0)); + command.GetArgumentAtIndex(0)); return; } } else { - new_thread = process->GetThreadList().FindThreadByID(m_options.m_thread_id).get(); + new_thread = + process->GetThreadList().FindThreadByID(m_options.m_thread_id).get(); if (new_thread == nullptr) { result.AppendErrorWithFormat("Invalid thread ID %lu.\n", m_options.m_thread_id); From e30a155562439f42feea990f6c109f53c218047e Mon Sep 17 00:00:00 2001 From: Michael Christensen Date: Tue, 28 Nov 2023 22:56:09 -0800 Subject: [PATCH 4/7] Code review changes -- update help text and usage text, fix format specifier, add thread ID completion --- .../lldb/Interpreter/CommandCompletions.h | 3 +++ .../Interpreter/CommandOptionArgumentTable.h | 1 + lldb/include/lldb/lldb-enumerations.h | 3 ++- lldb/source/Commands/CommandCompletions.cpp | 18 ++++++++++++++++++ lldb/source/Commands/CommandObjectThread.cpp | 7 ++++--- lldb/source/Commands/Options.td | 3 ++- .../commands/thread/select/TestThreadSelect.py | 2 +- 7 files changed, 31 insertions(+), 6 deletions(-) diff --git a/lldb/include/lldb/Interpreter/CommandCompletions.h b/lldb/include/lldb/Interpreter/CommandCompletions.h index a89a5be95b801c..c7292b3b1471a8 100644 --- a/lldb/include/lldb/Interpreter/CommandCompletions.h +++ b/lldb/include/lldb/Interpreter/CommandCompletions.h @@ -120,6 +120,9 @@ class CommandCompletions { CompletionRequest &request, SearchFilter *searcher); + static void ThreadIDs(CommandInterpreter &interpreter, + CompletionRequest &request, SearchFilter *searcher); + /// This completer works for commands whose only arguments are a command path. /// It isn't tied to an argument type because it completes not on a single /// argument but on the sequence of arguments, so you have to invoke it by diff --git a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h index 4bf71393bb07dc..46d2a43a8e0c41 100644 --- a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h +++ b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h @@ -191,6 +191,7 @@ static constexpr OptionEnumValueElement g_completion_type[] = { "Completes to a remote disk directory."}, {lldb::eTypeCategoryNameCompletion, "type-category-name", "Completes to a type category name."}, + {lldb::eThreadIDCompletion, "thread-id", "Completes to a thread ID."}, {lldb::eCustomCompletion, "custom", "Custom completion."}, }; diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index 633a3ee696c208..3345c0659ae672 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -1296,10 +1296,11 @@ enum CompletionType { eRemoteDiskFileCompletion = (1u << 22), eRemoteDiskDirectoryCompletion = (1u << 23), eTypeCategoryNameCompletion = (1u << 24), + eThreadIDCompletion = (1u << 25), // This item serves two purposes. It is the last element in the enum, so // you can add custom enums starting from here in your Option class. Also // if you & in this bit the base code will not process the option. - eCustomCompletion = (1u << 25) + eCustomCompletion = (1u << 26) }; } // namespace lldb diff --git a/lldb/source/Commands/CommandCompletions.cpp b/lldb/source/Commands/CommandCompletions.cpp index 4d7e3d7f2497bb..6b9bf1e3ca573f 100644 --- a/lldb/source/Commands/CommandCompletions.cpp +++ b/lldb/source/Commands/CommandCompletions.cpp @@ -83,6 +83,7 @@ bool CommandCompletions::InvokeCommonCompletionCallbacks( CommandCompletions::RemoteDiskDirectories}, {lldb::eTypeCategoryNameCompletion, CommandCompletions::TypeCategoryNames}, + {lldb::eThreadIDCompletion, CommandCompletions::ThreadIDs}, {lldb::CompletionType::eNoCompletion, nullptr} // This one has to be last in the list. }; @@ -807,6 +808,23 @@ void CommandCompletions::TypeCategoryNames(CommandInterpreter &interpreter, }); } +void CommandCompletions::ThreadIDs(CommandInterpreter &interpreter, + CompletionRequest &request, + SearchFilter *searcher) { + const ExecutionContext &exe_ctx = interpreter.GetExecutionContext(); + if (!exe_ctx.HasProcessScope()) + return; + + ThreadList &threads = exe_ctx.GetProcessPtr()->GetThreadList(); + lldb::ThreadSP thread_sp; + for (uint32_t idx = 0; (thread_sp = threads.GetThreadAtIndex(idx)); ++idx) { + StreamString strm; + thread_sp->GetStatus(strm, 0, 1, 1, true); + request.TryCompleteCurrentArg(std::to_string(thread_sp->GetID()), + strm.GetString()); + } +} + void CommandCompletions::CompleteModifiableCmdPathArgs( CommandInterpreter &interpreter, CompletionRequest &request, OptionElementVector &opt_element_vector) { diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp index 35618551c2f05e..dd9fab4bbddabc 100644 --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -1172,7 +1172,8 @@ class CommandObjectThreadSelect : public CommandObjectParsed { CommandObjectThreadSelect(CommandInterpreter &interpreter) : CommandObjectParsed(interpreter, "thread select", - "Change the currently selected thread.", nullptr, + "Change the currently selected thread.", + "thread select (or -t )", eCommandRequiresProcess | eCommandTryTargetAPILock | eCommandProcessMustBeLaunched | eCommandProcessMustBePaused) { @@ -1241,7 +1242,7 @@ class CommandObjectThreadSelect : public CommandObjectParsed { } new_thread = process->GetThreadList().FindThreadByIndexID(index_id).get(); if (new_thread == nullptr) { - result.AppendErrorWithFormat("Invalid thread #%s.\n", + result.AppendErrorWithFormat("Invalid thread index #%s.\n", command.GetArgumentAtIndex(0)); return; } @@ -1249,7 +1250,7 @@ class CommandObjectThreadSelect : public CommandObjectParsed { new_thread = process->GetThreadList().FindThreadByID(m_options.m_thread_id).get(); if (new_thread == nullptr) { - result.AppendErrorWithFormat("Invalid thread ID %lu.\n", + result.AppendErrorWithFormat("Invalid thread ID %" PRIu64 ".\n", m_options.m_thread_id); return; } diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 558f2fbf962842..ed3167727bcd32 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -1119,7 +1119,8 @@ let Command = "thread plan list" in { let Command = "thread select" in { def thread_select_thread_id : Option<"thread-id", "t">, Group<2>, - Arg<"ThreadID">, Desc<"Provide a thread ID instead of a thread index.">; + Arg<"ThreadID">, Completion<"ThreadID">, + Desc<"Provide a thread ID instead of a thread index.">; } let Command = "thread trace dump function calls" in { diff --git a/lldb/test/API/commands/thread/select/TestThreadSelect.py b/lldb/test/API/commands/thread/select/TestThreadSelect.py index 2345dc2b2b36a9..2a2cef4a905c92 100644 --- a/lldb/test/API/commands/thread/select/TestThreadSelect.py +++ b/lldb/test/API/commands/thread/select/TestThreadSelect.py @@ -36,7 +36,7 @@ def test_invalid_arg(self): self.expect( "thread select 0xffffffff", error=True, - startstr="error: Invalid thread #0xffffffff.", + startstr="error: Invalid thread index #0xffffffff.", ) self.expect( "thread select -t 0xffffffff", From e109a2ea37b20141aad8c4db4d39ff56e8a6dc4e Mon Sep 17 00:00:00 2001 From: Michael Christensen Date: Wed, 29 Nov 2023 00:59:13 -0800 Subject: [PATCH 5/7] Empty commit to retrigger CI From 2db54f68f259624616025ef56c8f5e3eadd86b61 Mon Sep 17 00:00:00 2001 From: Michael Christensen Date: Tue, 5 Dec 2023 05:40:04 -0800 Subject: [PATCH 6/7] Code review changes -- put eThreadIDCompletion after eCustomCompletion enum; add completion terminator --- .../Interpreter/CommandOptionArgumentTable.h | 2 +- lldb/include/lldb/lldb-enumerations.h | 62 +++++++++---------- lldb/source/Commands/CommandCompletions.cpp | 7 ++- 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h index 46d2a43a8e0c41..d0cf54c31ca73f 100644 --- a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h +++ b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h @@ -191,8 +191,8 @@ static constexpr OptionEnumValueElement g_completion_type[] = { "Completes to a remote disk directory."}, {lldb::eTypeCategoryNameCompletion, "type-category-name", "Completes to a type category name."}, - {lldb::eThreadIDCompletion, "thread-id", "Completes to a thread ID."}, {lldb::eCustomCompletion, "custom", "Custom completion."}, + {lldb::eThreadIDCompletion, "thread-id", "Completes to a thread ID."}, }; llvm::StringRef RegisterNameHelpTextCallback(); diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index 3345c0659ae672..cf72285434747b 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -1270,37 +1270,37 @@ enum WatchpointValueKind { }; enum CompletionType { - eNoCompletion = 0u, - eSourceFileCompletion = (1u << 0), - eDiskFileCompletion = (1u << 1), - eDiskDirectoryCompletion = (1u << 2), - eSymbolCompletion = (1u << 3), - eModuleCompletion = (1u << 4), - eSettingsNameCompletion = (1u << 5), - ePlatformPluginCompletion = (1u << 6), - eArchitectureCompletion = (1u << 7), - eVariablePathCompletion = (1u << 8), - eRegisterCompletion = (1u << 9), - eBreakpointCompletion = (1u << 10), - eProcessPluginCompletion = (1u << 11), - eDisassemblyFlavorCompletion = (1u << 12), - eTypeLanguageCompletion = (1u << 13), - eFrameIndexCompletion = (1u << 14), - eModuleUUIDCompletion = (1u << 15), - eStopHookIDCompletion = (1u << 16), - eThreadIndexCompletion = (1u << 17), - eWatchpointIDCompletion = (1u << 18), - eBreakpointNameCompletion = (1u << 19), - eProcessIDCompletion = (1u << 20), - eProcessNameCompletion = (1u << 21), - eRemoteDiskFileCompletion = (1u << 22), - eRemoteDiskDirectoryCompletion = (1u << 23), - eTypeCategoryNameCompletion = (1u << 24), - eThreadIDCompletion = (1u << 25), - // This item serves two purposes. It is the last element in the enum, so - // you can add custom enums starting from here in your Option class. Also - // if you & in this bit the base code will not process the option. - eCustomCompletion = (1u << 26) + eNoCompletion = 0ul, + eSourceFileCompletion = (1ul << 0), + eDiskFileCompletion = (1ul << 1), + eDiskDirectoryCompletion = (1ul << 2), + eSymbolCompletion = (1ul << 3), + eModuleCompletion = (1ul << 4), + eSettingsNameCompletion = (1ul << 5), + ePlatformPluginCompletion = (1ul << 6), + eArchitectureCompletion = (1ul << 7), + eVariablePathCompletion = (1ul << 8), + eRegisterCompletion = (1ul << 9), + eBreakpointCompletion = (1ul << 10), + eProcessPluginCompletion = (1ul << 11), + eDisassemblyFlavorCompletion = (1ul << 12), + eTypeLanguageCompletion = (1ul << 13), + eFrameIndexCompletion = (1ul << 14), + eModuleUUIDCompletion = (1ul << 15), + eStopHookIDCompletion = (1ul << 16), + eThreadIndexCompletion = (1ul << 17), + eWatchpointIDCompletion = (1ul << 18), + eBreakpointNameCompletion = (1ul << 19), + eProcessIDCompletion = (1ul << 20), + eProcessNameCompletion = (1ul << 21), + eRemoteDiskFileCompletion = (1ul << 22), + eRemoteDiskDirectoryCompletion = (1ul << 23), + eTypeCategoryNameCompletion = (1ul << 24), + eCustomCompletion = (1ul << 25), + eThreadIDCompletion = (1ul << 26), + // This last enum element is just for input validation. + // Add new completions before this element. + eTerminatorCompletion = (1ul << 63) }; } // namespace lldb diff --git a/lldb/source/Commands/CommandCompletions.cpp b/lldb/source/Commands/CommandCompletions.cpp index 6b9bf1e3ca573f..c8bf4f0f4792ad 100644 --- a/lldb/source/Commands/CommandCompletions.cpp +++ b/lldb/source/Commands/CommandCompletions.cpp @@ -44,7 +44,7 @@ typedef void (*CompletionCallback)(CommandInterpreter &interpreter, lldb_private::SearchFilter *searcher); struct CommonCompletionElement { - uint32_t type; + uint64_t type; CompletionCallback callback; }; @@ -54,6 +54,7 @@ bool CommandCompletions::InvokeCommonCompletionCallbacks( bool handled = false; const CommonCompletionElement common_completions[] = { + {lldb::eNoCompletion, nullptr}, {lldb::eSourceFileCompletion, CommandCompletions::SourceFiles}, {lldb::eDiskFileCompletion, CommandCompletions::DiskFiles}, {lldb::eDiskDirectoryCompletion, CommandCompletions::DiskDirectories}, @@ -84,12 +85,12 @@ bool CommandCompletions::InvokeCommonCompletionCallbacks( {lldb::eTypeCategoryNameCompletion, CommandCompletions::TypeCategoryNames}, {lldb::eThreadIDCompletion, CommandCompletions::ThreadIDs}, - {lldb::CompletionType::eNoCompletion, + {lldb::eTerminatorCompletion, nullptr} // This one has to be last in the list. }; for (int i = 0;; i++) { - if (common_completions[i].type == lldb::eNoCompletion) + if (common_completions[i].type == lldb::eTerminatorCompletion) break; else if ((common_completions[i].type & completion_mask) == common_completions[i].type && From e1f1f0e8d80e5710593a6c9e8f0e3212e46a95bb Mon Sep 17 00:00:00 2001 From: Michael Christensen Date: Tue, 5 Dec 2023 10:27:42 -0800 Subject: [PATCH 7/7] Code review changes -- update eTerminatorCompletion value --- lldb/include/lldb/lldb-enumerations.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index cf72285434747b..ed1dec85d48406 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -1299,8 +1299,9 @@ enum CompletionType { eCustomCompletion = (1ul << 25), eThreadIDCompletion = (1ul << 26), // This last enum element is just for input validation. - // Add new completions before this element. - eTerminatorCompletion = (1ul << 63) + // Add new completions before this element, + // and then increment eTerminatorCompletion's shift value + eTerminatorCompletion = (1ul << 27) }; } // namespace lldb