[Lldb-commits] [lldb] Add option to pass thread ID to thread select command (PR #73596)

Michael Christensen via lldb-commits lldb-commits at lists.llvm.org
Tue Dec 5 10:31:53 PST 2023


https://github.com/mdko updated https://github.com/llvm/llvm-project/pull/73596

>From 97a6e23c85457a14c91c5800fa03bb872e6f1fa6 Mon Sep 17 00:00:00 2001
From: Michael Christensen <mchristensen at meta.com>
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 a9f5a4f8a4fbd..9384df319cc22 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<OptionDefinition> 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 542c78be5a12d..23886046df8f6 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 91f8909471bf2..4d01b82d9d947 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 <mchristensen at meta.com>
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 9384df319cc22..9f535a1b6c944 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 23886046df8f6..558f2fbf96284 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 4d01b82d9d947..2345dc2b2b36a 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 <mchristensen at meta.com>
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 9f535a1b6c944..35618551c2f05 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 <mchristensen at meta.com>
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 a89a5be95b801..c7292b3b1471a 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 4bf71393bb07d..46d2a43a8e0c4 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 633a3ee696c20..3345c0659ae67 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 4d7e3d7f2497b..6b9bf1e3ca573 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 35618551c2f05..dd9fab4bbddab 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 <thread-index> (or -t <thread-id>)",
                             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 558f2fbf96284..ed3167727bcd3 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 2345dc2b2b36a..2a2cef4a905c9 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 2db54f68f259624616025ef56c8f5e3eadd86b61 Mon Sep 17 00:00:00 2001
From: Michael Christensen <mchristensen at meta.com>
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 46d2a43a8e0c4..d0cf54c31ca73 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 3345c0659ae67..cf72285434747 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 6b9bf1e3ca573..c8bf4f0f4792a 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 <mchristensen at meta.com>
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 cf72285434747..ed1dec85d4840 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



More information about the lldb-commits mailing list