[lldb][NFCI] Remove m_being_created from Breakpoint classes (#79716)

The purpose of m_being_created in these classes was to prevent
broadcasting an event related to these Breakpoints during the creation
of the breakpoint (i.e. in the constructor). In Breakpoint and
Watchpoint, m_being_created had no effect. That is to say, removing it
does not change behavior.
However, BreakpointLocation does still use m_being_created. In the
constructor, SetThreadID is called which does broadcast an event only if
`m_being_created` is false. Instead of having this logic be roundabout,
the constructor instead calls `SetThreadIDInternal`, which actually
changes the thread ID. `SetThreadID` also will call
`SetThreadIDInternal` in addition to broadcasting a changed event.

Author: bulbazord

lldb/include/lldb/Breakpoint/Breakpoint.h

@@ -637,7 +637,6 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>,
   Breakpoint(Target &new_target, const Breakpoint &bp_to_copy_from);
 
   // For Breakpoint only
-  bool m_being_created;
   bool
       m_hardware; // If this breakpoint is required to use a hardware breakpoint
   Target &m_target; // The target that holds this breakpoint.

lldb/include/lldb/Breakpoint/BreakpointLocation.h

@@ -313,6 +313,17 @@ class BreakpointLocation
 
   void UndoBumpHitCount();
 
+  /// Updates the thread ID internally.
+  ///
+  /// This method was created to handle actually mutating the thread ID
+  /// internally because SetThreadID broadcasts an event in addition to mutating
+  /// state. The constructor calls this instead of SetThreadID to avoid the
+  /// broadcast.
+  ///
+  /// \param[in] thread_id
+  ///   The new thread ID.
+  void SetThreadIDInternal(lldb::tid_t thread_id);
+
   // Constructors and Destructors
   //
   // Only the Breakpoint can make breakpoint locations, and it owns them.
@@ -337,7 +348,6 @@ class BreakpointLocation
                      bool check_for_resolver = true);
 
   // Data members:
-  bool m_being_created;
   bool m_should_resolve_indirect_functions;
   bool m_is_reexported;
   bool m_is_indirect;

lldb/include/lldb/Breakpoint/Watchpoint.h

@@ -224,11 +224,8 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
   CompilerType m_type;
   Status m_error; // An error object describing errors associated with this
                   // watchpoint.
-  WatchpointOptions
-      m_options; // Settable watchpoint options, which is a delegate to handle
-                 // the callback machinery.
-  bool m_being_created;
-
+  WatchpointOptions m_options; // Settable watchpoint options, which is a
+                               // delegate to handle the callback machinery.
   std::unique_ptr<UserExpression> m_condition_up; // The condition to test.
 
   void SetID(lldb::watch_id_t id) { m_id = id; }

lldb/source/Breakpoint/Breakpoint.cpp

@@ -44,17 +44,14 @@ const char *Breakpoint::g_option_names[static_cast<uint32_t>(
 Breakpoint::Breakpoint(Target &target, SearchFilterSP &filter_sp,
                        BreakpointResolverSP &resolver_sp, bool hardware,
                        bool resolve_indirect_symbols)
-    : m_being_created(true), m_hardware(hardware), m_target(target),
-      m_filter_sp(filter_sp), m_resolver_sp(resolver_sp), m_options(true),
-      m_locations(*this), m_resolve_indirect_symbols(resolve_indirect_symbols),
-      m_hit_counter() {
-  m_being_created = false;
-}
+    : m_hardware(hardware), m_target(target), m_filter_sp(filter_sp),
+      m_resolver_sp(resolver_sp), m_options(true), m_locations(*this),
+      m_resolve_indirect_symbols(resolve_indirect_symbols), m_hit_counter() {}
 
 Breakpoint::Breakpoint(Target &new_target, const Breakpoint &source_bp)
-    : m_being_created(true), m_hardware(source_bp.m_hardware),
-      m_target(new_target), m_name_list(source_bp.m_name_list),
-      m_options(source_bp.m_options), m_locations(*this),
+    : m_hardware(source_bp.m_hardware), m_target(new_target),
+      m_name_list(source_bp.m_name_list), m_options(source_bp.m_options),
+      m_locations(*this),
       m_resolve_indirect_symbols(source_bp.m_resolve_indirect_symbols),
       m_hit_counter() {}
 
@@ -979,9 +976,8 @@ bool Breakpoint::EvaluatePrecondition(StoppointCallbackContext &context) {
 
 void Breakpoint::SendBreakpointChangedEvent(
     lldb::BreakpointEventType eventKind) {
-  if (!m_being_created && !IsInternal() &&
-      GetTarget().EventTypeHasListeners(
-          Target::eBroadcastBitBreakpointChanged)) {
+  if (!IsInternal() && GetTarget().EventTypeHasListeners(
+                           Target::eBroadcastBitBreakpointChanged)) {
     std::shared_ptr<BreakpointEventData> data =
         std::make_shared<BreakpointEventData>(eventKind, shared_from_this());
 
@@ -994,7 +990,7 @@ void Breakpoint::SendBreakpointChangedEvent(
   if (!breakpoint_data_sp)
     return;
 
-  if (!m_being_created && !IsInternal() &&
+  if (!IsInternal() &&
       GetTarget().EventTypeHasListeners(Target::eBroadcastBitBreakpointChanged))
     GetTarget().BroadcastEvent(Target::eBroadcastBitBreakpointChanged,
                                breakpoint_data_sp);

lldb/source/Breakpoint/BreakpointLocation.cpp

@@ -32,18 +32,17 @@ using namespace lldb_private;
 BreakpointLocation::BreakpointLocation(break_id_t loc_id, Breakpoint &owner,
                                        const Address &addr, lldb::tid_t tid,
                                        bool hardware, bool check_for_resolver)
-    : m_being_created(true), m_should_resolve_indirect_functions(false),
-      m_is_reexported(false), m_is_indirect(false), m_address(addr),
-      m_owner(owner), m_condition_hash(0), m_loc_id(loc_id), m_hit_counter() {
+    : m_should_resolve_indirect_functions(false), m_is_reexported(false),
+      m_is_indirect(false), m_address(addr), m_owner(owner),
+      m_condition_hash(0), m_loc_id(loc_id), m_hit_counter() {
   if (check_for_resolver) {
     Symbol *symbol = m_address.CalculateSymbolContextSymbol();
     if (symbol && symbol->IsIndirect()) {
       SetShouldResolveIndirectFunctions(true);
     }
   }
 
-  SetThreadID(tid);
-  m_being_created = false;
+  SetThreadIDInternal(tid);
 }
 
 BreakpointLocation::~BreakpointLocation() { ClearBreakpointSite(); }
@@ -100,14 +99,7 @@ void BreakpointLocation::SetAutoContinue(bool auto_continue) {
 }
 
 void BreakpointLocation::SetThreadID(lldb::tid_t thread_id) {
-  if (thread_id != LLDB_INVALID_THREAD_ID)
-    GetLocationOptions().SetThreadID(thread_id);
-  else {
-    // If we're resetting this to an invalid thread id, then don't make an
-    // options pointer just to do that.
-    if (m_options_up != nullptr)
-      m_options_up->SetThreadID(thread_id);
-  }
+  SetThreadIDInternal(thread_id);
   SendBreakpointLocationChangedEvent(eBreakpointEventTypeThreadChanged);
 }
 
@@ -646,9 +638,8 @@ void BreakpointLocation::Dump(Stream *s) const {
 
 void BreakpointLocation::SendBreakpointLocationChangedEvent(
     lldb::BreakpointEventType eventKind) {
-  if (!m_being_created && !m_owner.IsInternal() &&
-      m_owner.GetTarget().EventTypeHasListeners(
-          Target::eBroadcastBitBreakpointChanged)) {
+  if (!m_owner.IsInternal() && m_owner.GetTarget().EventTypeHasListeners(
+                                   Target::eBroadcastBitBreakpointChanged)) {
     auto data_sp = std::make_shared<Breakpoint::BreakpointEventData>(
         eventKind, m_owner.shared_from_this());
     data_sp->GetBreakpointLocationCollection().Add(shared_from_this());
@@ -665,3 +656,14 @@ void BreakpointLocation::SwapLocation(BreakpointLocationSP swap_from) {
   m_is_indirect = swap_from->m_is_indirect;
   m_user_expression_sp.reset();
 }
+
+void BreakpointLocation::SetThreadIDInternal(lldb::tid_t thread_id) {
+  if (thread_id != LLDB_INVALID_THREAD_ID)
+    GetLocationOptions().SetThreadID(thread_id);
+  else {
+    // If we're resetting this to an invalid thread id, then don't make an
+    // options pointer just to do that.
+    if (m_options_up != nullptr)
+      m_options_up->SetThreadID(thread_id);
+  }
+}

lldb/source/Breakpoint/Watchpoint.cpp

@@ -31,8 +31,7 @@ Watchpoint::Watchpoint(Target &target, lldb::addr_t addr, uint32_t size,
     : StoppointSite(0, addr, size, hardware), m_target(target),
       m_enabled(false), m_is_hardware(hardware), m_is_watch_variable(false),
       m_is_ephemeral(false), m_disabled_count(0), m_watch_read(0),
-      m_watch_write(0), m_watch_modify(0), m_ignore_count(0),
-      m_being_created(true) {
+      m_watch_write(0), m_watch_modify(0), m_ignore_count(0) {
 
   if (type && type->IsValid())
     m_type = *type;
@@ -60,7 +59,6 @@ Watchpoint::Watchpoint(Target &target, lldb::addr_t addr, uint32_t size,
     m_target.GetProcessSP()->CalculateExecutionContext(exe_ctx);
     CaptureWatchedValue(exe_ctx);
   }
-  m_being_created = false;
 }
 
 Watchpoint::~Watchpoint() = default;
@@ -462,8 +460,8 @@ const char *Watchpoint::GetConditionText() const {
 
 void Watchpoint::SendWatchpointChangedEvent(
     lldb::WatchpointEventType eventKind) {
-  if (!m_being_created && GetTarget().EventTypeHasListeners(
-                              Target::eBroadcastBitWatchpointChanged)) {
+  if (GetTarget().EventTypeHasListeners(
+          Target::eBroadcastBitWatchpointChanged)) {
     auto data_sp =
         std::make_shared<WatchpointEventData>(eventKind, shared_from_this());
     GetTarget().BroadcastEvent(Target::eBroadcastBitWatchpointChanged, data_sp);