[BreakpointList] Simplify/modernize BreakpointList (NFC)

I was looking at the code in BreakpointList.cpp and found it deserved a
quick cleanup.

  -  Use std::vector instead of a std::list.
  -  Extract duplicate code for notifying.
  -  Remove code duplication when returning a const value.
  -  Use range-based for loop.
  -  Use early return in loops.

Differential revision: https://reviews.llvm.org/D56425

llvm-svn: 350659

2 files changed, 60 insertions, 142 deletions

diff --git a/lldb/include/lldb/Breakpoint/BreakpointList.h b/lldb/include/lldb/Breakpoint/BreakpointList.h
index 65f996ac31e4..c4fc650a29f0 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointList.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointList.h
@@ -50,18 +50,6 @@ public:
   void Dump(Stream *s) const;
 
   //------------------------------------------------------------------
-  /// Returns a shared pointer to the breakpoint with id \a breakID.
-  ///
-  /// @param[in] breakID
-  ///   The breakpoint ID to seek for.
-  ///
-  /// @result
-  ///   A shared pointer to the breakpoint.  May contain a NULL pointer if the
-  ///   breakpoint doesn't exist.
-  //------------------------------------------------------------------
-  lldb::BreakpointSP FindBreakpointByID(lldb::break_id_t breakID);
-
-  //------------------------------------------------------------------
   /// Returns a shared pointer to the breakpoint with id \a breakID.  Const
   /// version.
   ///
@@ -72,7 +60,7 @@ public:
   ///   A shared pointer to the breakpoint.  May contain a NULL pointer if the
   ///   breakpoint doesn't exist.
   //------------------------------------------------------------------
-  const lldb::BreakpointSP FindBreakpointByID(lldb::break_id_t breakID) const;
+  lldb::BreakpointSP FindBreakpointByID(lldb::break_id_t breakID) const;
 
   //------------------------------------------------------------------
   /// Returns a shared pointer to the breakpoint with index \a i.
@@ -84,20 +72,7 @@ public:
   ///   A shared pointer to the breakpoint.  May contain a NULL pointer if the
   ///   breakpoint doesn't exist.
   //------------------------------------------------------------------
-  lldb::BreakpointSP GetBreakpointAtIndex(size_t i);
-
-  //------------------------------------------------------------------
-  /// Returns a shared pointer to the breakpoint with index \a i, const
-  /// version
-  ///
-  /// @param[in] i
-  ///   The breakpoint index to seek for.
-  ///
-  /// @result
-  ///   A shared pointer to the breakpoint.  May contain a NULL pointer if the
-  ///   breakpoint doesn't exist.
-  //------------------------------------------------------------------
-  const lldb::BreakpointSP GetBreakpointAtIndex(size_t i) const;
+  lldb::BreakpointSP GetBreakpointAtIndex(size_t i) const;
 
   //------------------------------------------------------------------
   /// Find all the breakpoints with a given name
@@ -197,7 +172,7 @@ public:
   void GetListMutex(std::unique_lock<std::recursive_mutex> &lock);
 
 protected:
-  typedef std::list<lldb::BreakpointSP> bp_collection;
+  typedef std::vector<lldb::BreakpointSP> bp_collection;
 
   bp_collection::iterator GetBreakpointIDIterator(lldb::break_id_t breakID);
 
@@ -207,7 +182,7 @@ protected:
   std::recursive_mutex &GetMutex() const { return m_mutex; }
 
   mutable std::recursive_mutex m_mutex;
-  bp_collection m_breakpoints; // The breakpoint list, currently a list.
+  bp_collection m_breakpoints;
   lldb::break_id_t m_next_break_id;
   bool m_is_internal;
 
diff --git a/lldb/source/Breakpoint/BreakpointList.cpp b/lldb/source/Breakpoint/BreakpointList.cpp
index 38adc7d21372..370da10d82b4 100644
--- a/lldb/source/Breakpoint/BreakpointList.cpp
+++ b/lldb/source/Breakpoint/BreakpointList.cpp
@@ -14,6 +14,13 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static void NotifyChange(const BreakpointSP &bp, BreakpointEventType event) {
+  Target &target = bp->GetTarget();
+  if (target.EventTypeHasListeners(Target::eBroadcastBitBreakpointChanged))
+    target.BroadcastEvent(Target::eBroadcastBitBreakpointChanged,
+                          new Breakpoint::BreakpointEventData(event, bp));
+}
+
 BreakpointList::BreakpointList(bool is_internal)
     : m_mutex(), m_breakpoints(), m_next_break_id(0),
       m_is_internal(is_internal) {}
@@ -22,37 +29,34 @@ BreakpointList::~BreakpointList() {}
 
 break_id_t BreakpointList::Add(BreakpointSP &bp_sp, bool notify) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
+
   // Internal breakpoint IDs are negative, normal ones are positive
   bp_sp->SetID(m_is_internal ? --m_next_break_id : ++m_next_break_id);
 
   m_breakpoints.push_back(bp_sp);
-  if (notify) {
-    if (bp_sp->GetTarget().EventTypeHasListeners(
-            Target::eBroadcastBitBreakpointChanged))
-      bp_sp->GetTarget().BroadcastEvent(Target::eBroadcastBitBreakpointChanged,
-                                        new Breakpoint::BreakpointEventData(
-                                            eBreakpointEventTypeAdded, bp_sp));
-  }
+
+  if (notify)
+    NotifyChange(bp_sp, eBreakpointEventTypeAdded);
+
   return bp_sp->GetID();
 }
 
 bool BreakpointList::Remove(break_id_t break_id, bool notify) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  bp_collection::iterator pos = GetBreakpointIDIterator(break_id); // Predicate
-  if (pos != m_breakpoints.end()) {
-    BreakpointSP bp_sp(*pos);
-    m_breakpoints.erase(pos);
-    if (notify) {
-      if (bp_sp->GetTarget().EventTypeHasListeners(
-              Target::eBroadcastBitBreakpointChanged))
-        bp_sp->GetTarget().BroadcastEvent(
-            Target::eBroadcastBitBreakpointChanged,
-            new Breakpoint::BreakpointEventData(eBreakpointEventTypeRemoved,
-                                                bp_sp));
-    }
-    return true;
-  }
-  return false;
+
+  auto it = std::find_if(
+      m_breakpoints.begin(), m_breakpoints.end(),
+      [&](const BreakpointSP &bp) { return bp->GetID() == break_id; });
+
+  if (it == m_breakpoints.end())
+    return false;
+
+  if (notify)
+    NotifyChange(*it, eBreakpointEventTypeRemoved);
+
+  m_breakpoints.erase(it);
+
+  return true;
 }
 
 void BreakpointList::RemoveInvalidLocations(const ArchSpec &arch) {
@@ -79,93 +83,50 @@ void BreakpointList::RemoveAll(bool notify) {
   ClearAllBreakpointSites();
 
   if (notify) {
-    bp_collection::iterator pos, end = m_breakpoints.end();
-    for (pos = m_breakpoints.begin(); pos != end; ++pos) {
-      if ((*pos)->GetTarget().EventTypeHasListeners(
-              Target::eBroadcastBitBreakpointChanged)) {
-        (*pos)->GetTarget().BroadcastEvent(
-            Target::eBroadcastBitBreakpointChanged,
-            new Breakpoint::BreakpointEventData(eBreakpointEventTypeRemoved,
-                                                *pos));
-      }
-    }
+    for (const auto &bp_sp : m_breakpoints)
+      NotifyChange(bp_sp, eBreakpointEventTypeRemoved);
   }
-  m_breakpoints.erase(m_breakpoints.begin(), m_breakpoints.end());
+
+  m_breakpoints.clear();
 }
 
 void BreakpointList::RemoveAllowed(bool notify) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
 
-  bp_collection::iterator pos, end = m_breakpoints.end();
-  if (notify) {
-    for (pos = m_breakpoints.begin(); pos != end; ++pos) {
-      if(!(*pos)->AllowDelete())
-        continue;
-      if ((*pos)->GetTarget().EventTypeHasListeners(
-              Target::eBroadcastBitBreakpointChanged)) {
-        (*pos)->GetTarget().BroadcastEvent(
-            Target::eBroadcastBitBreakpointChanged,
-            new Breakpoint::BreakpointEventData(eBreakpointEventTypeRemoved,
-                                                *pos));
-      }
-    }
-  }
-  pos = m_breakpoints.begin();
-  while ( pos != end) {
-    auto bp = *pos;
-    if (bp->AllowDelete()) {
-      bp->ClearAllBreakpointSites();
-      pos = m_breakpoints.erase(pos);
-    } else
-      pos++;
+  for (const auto &bp_sp : m_breakpoints) {
+    if (bp_sp->AllowDelete())
+      bp_sp->ClearAllBreakpointSites();
+    if (notify)
+      NotifyChange(bp_sp, eBreakpointEventTypeRemoved);
   }
-}
-
-class BreakpointIDMatches {
-public:
-  BreakpointIDMatches(break_id_t break_id) : m_break_id(break_id) {}
 
-  bool operator()(const BreakpointSP &bp) const {
-    return m_break_id == bp->GetID();
-  }
-
-private:
-  const break_id_t m_break_id;
-};
+  m_breakpoints.erase(
+      std::remove_if(m_breakpoints.begin(), m_breakpoints.end(),
+                     [&](const BreakpointSP &bp) { return bp->AllowDelete(); }),
+      m_breakpoints.end());
+}
 
 BreakpointList::bp_collection::iterator
 BreakpointList::GetBreakpointIDIterator(break_id_t break_id) {
-  return std::find_if(m_breakpoints.begin(),
-                      m_breakpoints.end(),            // Search full range
-                      BreakpointIDMatches(break_id)); // Predicate
+  return std::find_if(
+      m_breakpoints.begin(), m_breakpoints.end(),
+      [&](const BreakpointSP &bp) { return bp->GetID() == break_id; });
 }
 
 BreakpointList::bp_collection::const_iterator
 BreakpointList::GetBreakpointIDConstIterator(break_id_t break_id) const {
-  return std::find_if(m_breakpoints.begin(),
-                      m_breakpoints.end(),            // Search full range
-                      BreakpointIDMatches(break_id)); // Predicate
-}
-
-BreakpointSP BreakpointList::FindBreakpointByID(break_id_t break_id) {
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  BreakpointSP stop_sp;
-  bp_collection::iterator pos = GetBreakpointIDIterator(break_id);
-  if (pos != m_breakpoints.end())
-    stop_sp = *pos;
-
-  return stop_sp;
+  return std::find_if(
+      m_breakpoints.begin(), m_breakpoints.end(),
+      [&](const BreakpointSP &bp) { return bp->GetID() == break_id; });
 }
 
-const BreakpointSP
-BreakpointList::FindBreakpointByID(break_id_t break_id) const {
+BreakpointSP BreakpointList::FindBreakpointByID(break_id_t break_id) const {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  BreakpointSP stop_sp;
-  bp_collection::const_iterator pos = GetBreakpointIDConstIterator(break_id);
-  if (pos != m_breakpoints.end())
-    stop_sp = *pos;
 
-  return stop_sp;
+  auto it = GetBreakpointIDConstIterator(break_id);
+  if (it != m_breakpoints.end())
+    return *it;
+  return {};
 }
 
 bool BreakpointList::FindBreakpointsByName(const char *name,
@@ -182,6 +143,7 @@ bool BreakpointList::FindBreakpointsByName(const char *name,
       matching_bps.Add(bkpt_sp, false);
     }
   }
+
   return true;
 }
 
@@ -197,30 +159,11 @@ void BreakpointList::Dump(Stream *s) const {
   s->IndentLess();
 }
 
-BreakpointSP BreakpointList::GetBreakpointAtIndex(size_t i) {
+BreakpointSP BreakpointList::GetBreakpointAtIndex(size_t i) const {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  BreakpointSP stop_sp;
-  bp_collection::iterator end = m_breakpoints.end();
-  bp_collection::iterator pos;
-  size_t curr_i = 0;
-  for (pos = m_breakpoints.begin(), curr_i = 0; pos != end; ++pos, ++curr_i) {
-    if (curr_i == i)
-      stop_sp = *pos;
-  }
-  return stop_sp;
-}
-
-const BreakpointSP BreakpointList::GetBreakpointAtIndex(size_t i) const {
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  BreakpointSP stop_sp;
-  bp_collection::const_iterator end = m_breakpoints.end();
-  bp_collection::const_iterator pos;
-  size_t curr_i = 0;
-  for (pos = m_breakpoints.begin(), curr_i = 0; pos != end; ++pos, ++curr_i) {
-    if (curr_i == i)
-      stop_sp = *pos;
-  }
-  return stop_sp;
+  if (i < m_breakpoints.size())
+    return m_breakpoints[i];
+  return {};
 }
 
 void BreakpointList::UpdateBreakpoints(ModuleList &module_list, bool added,