Refactor to a thread collection SP

Author: Jlalond

lldb/include/lldb/API/SBSaveCoreOptions.h

@@ -14,6 +14,7 @@
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBThread.h"
+#include "lldb/API/SBThreadCollection.h"
 
 namespace lldb {
 
@@ -111,26 +112,18 @@ class LLDB_API SBSaveCoreOptions {
   ///   style specific regions.
   SBError AddMemoryRegionToSave(const SBMemoryRegionInfo &region);
 
-  /// Get the number of Threads to be saved
+  /// Get an unsorted collection of the threads to save
   ///
   /// \returns
-  ///  The count of Threads to be saved.
-  uint32_t GetNumThreads() const;
-
-  /// Get the Thread at the specified index.
-  ///
-  /// \param [in] idx
-  ///   The zero based index of the thread to return.
-  /// \returns
-  ///   The thread at the specified index, or an invalid SBThread if the index
-  ///   is greater than or equal to the number of threads.
-  lldb::SBThread GetThreadAtIndex(uint32_t idx) const;
+  ///   An unsorted collection with zero or more threads.
+  SBThreadCollection GetThreadsToSave() const;
 
   /// Reset all options.
   void Clear();
 
 protected:
   friend class SBProcess;
+  friend class SBThreadCollection;
   lldb_private::SaveCoreOptions &ref() const;
 
 private:

lldb/include/lldb/API/SBThreadCollection.h

@@ -48,7 +48,7 @@ class LLDB_API SBThreadCollection {
 private:
   friend class SBProcess;
   friend class SBThread;
-
+  friend class SBSaveCoreOptions;
   lldb::ThreadCollectionSP m_opaque_sp;
 };
 

lldb/include/lldb/Symbol/SaveCoreOptions.h

@@ -46,8 +46,7 @@ class SaveCoreOptions {
 
   void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo &region);
 
-  std::optional<lldb::ThreadSP> GetThreadAtIndex(uint32_t idx) const;
-  uint32_t GetNumThreads() const;
+  lldb::ThreadCollectionSP GetThreadsToSave() const;
 
   void Clear();
 
@@ -60,8 +59,6 @@ class SaveCoreOptions {
   lldb::ProcessSP m_process_sp;
   std::unordered_map<lldb::tid_t, lldb::ThreadSP> m_threads_to_save;
   MemoryRanges m_regions_to_save;
-
-  std::vector<lldb::tid_t> m_thread_indexes;
 };
 } // namespace lldb_private
 

lldb/source/API/SBSaveCoreOptions.cpp

@@ -100,17 +100,9 @@ SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo &region) {
   return SBError();
 }
 
-uint32_t lldb::SBSaveCoreOptions::GetNumThreads() const {
+lldb::SBThreadCollection SBSaveCoreOptions::GetThreadsToSave() const {
   LLDB_INSTRUMENT_VA(this);
-  return m_opaque_up->GetNumThreads();
-}
-
-SBThread SBSaveCoreOptions::GetThreadAtIndex(uint32_t idx) const {
-  LLDB_INSTRUMENT_VA(this, idx);
-  std::optional<lldb::ThreadSP> thread_sp = m_opaque_up->GetThreadAtIndex(idx);
-  if (thread_sp)
-    return SBThread(thread_sp.value());
-  return SBThread();
+  return SBThreadCollection(m_opaque_up->GetThreadsToSave());
 }
 
 void SBSaveCoreOptions::Clear() {

lldb/source/Symbol/SaveCoreOptions.cpp

@@ -88,32 +88,11 @@ Status SaveCoreOptions::AddThread(lldb::ThreadSP thread_sp) {
   }
 
   m_threads_to_save.insert({thread_sp->GetID(), thread_sp});
-  m_thread_indexes.push_back(thread_sp->GetID());
   return error;
 }
 
 bool SaveCoreOptions::RemoveThread(lldb::ThreadSP thread_sp) {
-  if (!thread_sp)
-    return false;
-  if (m_threads_to_save.erase(thread_sp->GetID()) == 0)
-    return false;
-
-  auto it = std::find(m_thread_indexes.begin(), m_thread_indexes.end(),
-                      thread_sp->GetID());
-  m_thread_indexes.erase(it);
-  return true;
-}
-
-uint32_t SaveCoreOptions::GetNumThreads() const {
-  return m_threads_to_save.size();
-}
-
-std::optional<lldb::ThreadSP>
-SaveCoreOptions::GetThreadAtIndex(uint32_t idx) const {
-  if (idx >= m_thread_indexes.size())
-    return std::nullopt;
-  lldb::tid_t tid = m_thread_indexes[idx];
-  return m_threads_to_save.find(tid)->second;
+  return thread_sp && m_threads_to_save.erase(thread_sp->GetID()) > 0;
 }
 
 bool SaveCoreOptions::ShouldThreadBeSaved(lldb::tid_t tid) const {
@@ -136,6 +115,15 @@ const MemoryRanges &SaveCoreOptions::GetCoreFileMemoryRanges() const {
   return m_regions_to_save;
 }
 
+lldb::ThreadCollectionSP SaveCoreOptions::GetThreadsToSave() const {
+  lldb::ThreadCollectionSP threadcollection_sp =
+      std::make_shared<ThreadCollection>();
+  for (const auto &thread : m_threads_to_save) {
+    threadcollection_sp->AddThread(thread.second);
+  }
+  return threadcollection_sp;
+}
+
 Status
 SaveCoreOptions::EnsureValidConfiguration(lldb::ProcessSP process_sp) const {
   Status error;
@@ -166,5 +154,4 @@ void SaveCoreOptions::Clear() {
   m_threads_to_save.clear();
   m_process_sp.reset();
   m_regions_to_save.Clear();
-  m_thread_indexes.clear();
 }

lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py

@@ -93,28 +93,14 @@ def test_removing_and_adding_insertion_order(self):
             error = options.AddThread(thread)
             self.assertTrue(error.Success())
 
-        # Get the middle thread, remove it, and insert it at the end.
+        # Get the middle thread, remove it, and insert it back.
         middle_thread = threads[1]
         self.assertTrue(options.RemoveThread(middle_thread))
-        num_threads = options.GetNumThreads()
-        self.assertEqual(num_threads, 2)
-        error = options.AddThread(middle_thread)
-        self.assertTrue(error.Success())
-        num_threads = options.GetNumThreads()
-        self.assertEqual(num_threads, 3)
-        thread_at_last_index = options.GetThreadAtIndex(2)
-        self.assertEqual(thread_at_last_index.id, middle_thread.id)
-        thread_at_middle_index = options.GetThreadAtIndex(1)
-        self.assertEqual(thread_at_middle_index.id, threads[2].id)
-
-        # Pop the front thread, remove it, and insert it at the end.
-        front_thread = threads[0]
-        self.assertTrue(options.RemoveThread(front_thread))
-        num_threads = options.GetNumThreads()
-        self.assertEqual(num_threads, 2)
-        error = options.AddThread(front_thread)
-        self.assertTrue(error.Success())
-        num_threads = options.GetNumThreads()
-        self.assertEqual(num_threads, 3)
-        thread_at_last_index = options.GetThreadAtIndex(2)
-        self.assertEqual(thread_at_last_index.id, front_thread.id)
+        thread_collection = options.GetThreadsToSave()
+        self.assertTrue(thread_collection is not None)
+        self.assertEqual(thread_collection.GetSize(), 2)
+        # error = options.AddThread(middle_thread)
+        # self.assertTrue(error.Success())
+        # thread_collection = options.GetThreadsToSave()
+        # self.assertEqual(thread_collection.GetSize(), 3)
+        # self.assertIn(middle_thread, thread_collection)