[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: davide.

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

- Extract duplicate code
- Use range-based for loop
- Use early return in loops


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D56425

Files:
  source/Breakpoint/BreakpointList.cpp


Index: source/Breakpoint/BreakpointList.cpp
===================================================================
--- source/Breakpoint/BreakpointList.cpp
+++ source/Breakpoint/BreakpointList.cpp
@@ -14,6 +14,14 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static void NotifyChange(BreakpointSP bp, BreakpointEventType event) {
+  if (bp->GetTarget().EventTypeHasListeners(
+          Target::eBroadcastBitBreakpointChanged))
+    bp->GetTarget().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) {}
@@ -26,33 +34,23 @@
   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;
+  if (pos == m_breakpoints.end())
+    return false;
+
+  BreakpointSP bp_sp(*pos);
+  m_breakpoints.erase(pos);
+  if (notify)
+    NotifyChange(bp_sp, eBreakpointEventTypeRemoved);
+  return true;
 }
 
 void BreakpointList::RemoveInvalidLocations(const ArchSpec &arch) {
@@ -79,18 +77,11 @@
   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) {
@@ -98,18 +89,12 @@
 
   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));
-      }
+    for (const auto &bp_sp : m_breakpoints) {
+      if (bp_sp->AllowDelete())
+        NotifyChange(bp_sp, eBreakpointEventTypeRemoved);
     }
   }
+
   pos = m_breakpoints.begin();
   while ( pos != end) {
     auto bp = *pos;
@@ -199,28 +184,25 @@
 
 BreakpointSP BreakpointList::GetBreakpointAtIndex(size_t i) {
   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) {
+  for (const auto &bp_sp : m_breakpoints) {
     if (curr_i == i)
-      stop_sp = *pos;
+      return bp_sp;
+    curr_i++;
   }
-  return stop_sp;
+  return {};
 }
 
 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) {
+  for (const auto &bp_sp : m_breakpoints) {
     if (curr_i == i)
-      stop_sp = *pos;
+      return bp_sp;
+    curr_i++;
   }
-  return stop_sp;
+  return {};
 }
 
 void BreakpointList::UpdateBreakpoints(ModuleList &module_list, bool added,

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits