[Lldb-commits] [PATCH] D84527: Rename StoppointLocation to StoppointSite and drop its relationship with BreakpointLocation

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Some nits but overall this looks good.



================
Comment at: lldb/include/lldb/Breakpoint/Breakpoint.h:657
   bool m_resolve_indirect_symbols;
-  uint32_t m_hit_count; // Number of times this breakpoint/watchpoint has been
-                        // hit.  This is kept
+  StoppointHitCounter m_hit_counter; // Number of times this breakpoint has been
+                                     // hit. This is kept
----------------
Please make this a Doxygen comment `///` above the variable. 


================
Comment at: lldb/include/lldb/Breakpoint/StoppointHitCounter.h:37
+private:
+  uint32_t m_hit_count = 0; ///< Number of times this breakpoint/watchpoint has
+                            ///< been hit.
----------------
With the 80 col limit the trailing comments are really hard to read and maintain. Please put them above the variable.


================
Comment at: lldb/include/lldb/Breakpoint/StoppointSite.h:20
+public:
+  // Constructors and Destructors
+  StoppointSite(lldb::break_id_t bid, lldb::addr_t m_addr, bool hardware);
----------------
I'm slowly getting rid of these (rather useless) comments, so let's not add any new ones :-) 


================
Comment at: lldb/include/lldb/Breakpoint/StoppointSite.h:52
+protected:
+  lldb::break_id_t m_id; ///< Stoppoint site ID.
+  lldb::addr_t m_addr; ///< The load address of this stop point.
----------------
Same comment as earlier. 


================
Comment at: lldb/include/lldb/Breakpoint/StoppointSite.h:66
+private:
+  // For StoppointSite only
+  StoppointSite(const StoppointSite &) = delete;
----------------
Private already conveys that. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84527/new/

https://reviews.llvm.org/D84527