[Lldb-commits] [lldb] [lldb][NFCI] Remove m_being_created from Breakpoint classes (PR #79716)

2024-01-27 Thread Alex Langford via lldb-commits 

https://github.com/bulbazord created 
https://github.com/llvm/llvm-project/pull/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.

>From 2f5ccf2e8fea01cd60ce521fede82d3789b383eb Mon Sep 17 00:00:00 2001
From: Alex Langford 
Date: Sat, 27 Jan 2024 15:54:16 -0800
Subject: [PATCH] [lldb][NFCI] Remove m_being_created from Breakpoint classes

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.
---
 lldb/include/lldb/Breakpoint/Breakpoint.h |  1 -
 .../lldb/Breakpoint/BreakpointLocation.h  |  3 +-
 lldb/include/lldb/Breakpoint/Watchpoint.h |  2 --
 lldb/source/Breakpoint/Breakpoint.cpp | 22 +---
 lldb/source/Breakpoint/BreakpointLocation.cpp | 34 ++-
 lldb/source/Breakpoint/Watchpoint.cpp |  8 ++---
 6 files changed, 32 insertions(+), 38 deletions(-)

diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h 
b/lldb/include/lldb/Breakpoint/Breakpoint.h
index 8c4308ab0bc12d..0b6bf593be37a9 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -637,7 +637,6 @@ class Breakpoint : public 
std::enable_shared_from_this,
   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.
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h 
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 2a4f9fc01bf326..273132c950825a 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -313,6 +313,8 @@ class BreakpointLocation
 
   void UndoBumpHitCount();
 
+  void SetThreadIDInternal(lldb::tid_t thread_id);
+
   // Constructors and Destructors
   //
   // Only the Breakpoint can make breakpoint locations, and it owns them.
@@ -337,7 +339,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;
diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h 
b/lldb/include/lldb/Breakpoint/Watchpoint.h
index 22fdfd686c3f11..fc11a466ba6054 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -227,8 +227,6 @@ class Watchpoint : public 
std::enable_shared_from_this,
   WatchpointOptions
   m_options; // Settable watchpoint options, which is a delegate to handle
  // the callback machinery.
-  bool m_being_created;
-
   std::unique_ptr m_condition_up; // The condition to test.
 
   void SetID(lldb::watch_id_t id) { m_id = id; }
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp 
b/lldb/source/Breakpoint/Breakpoint.cpp
index af5dcc9cd88d4f..ae845e92762b97 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -44,17 +44,14 @@ const char 
*Breakpoint::g_option_names[static_cast(
 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

[Lldb-commits] [lldb] [lldb][NFCI] Remove m_being_created from Breakpoint classes (PR #79716)

2024-01-27 Thread via lldb-commits 

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Alex Langford (bulbazord)


Changes

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/79716.diff


6 Files Affected:

- (modified) lldb/include/lldb/Breakpoint/Breakpoint.h (-1) 
- (modified) lldb/include/lldb/Breakpoint/BreakpointLocation.h (+2-1) 
- (modified) lldb/include/lldb/Breakpoint/Watchpoint.h (-2) 
- (modified) lldb/source/Breakpoint/Breakpoint.cpp (+9-13) 
- (modified) lldb/source/Breakpoint/BreakpointLocation.cpp (+18-16) 
- (modified) lldb/source/Breakpoint/Watchpoint.cpp (+3-5) 


``diff
diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h 
b/lldb/include/lldb/Breakpoint/Breakpoint.h
index 8c4308ab0bc12d5..0b6bf593be37a97 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -637,7 +637,6 @@ class Breakpoint : public 
std::enable_shared_from_this,
   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.
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h 
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 2a4f9fc01bf3261..273132c950825af 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -313,6 +313,8 @@ class BreakpointLocation
 
   void UndoBumpHitCount();
 
+  void SetThreadIDInternal(lldb::tid_t thread_id);
+
   // Constructors and Destructors
   //
   // Only the Breakpoint can make breakpoint locations, and it owns them.
@@ -337,7 +339,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;
diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h 
b/lldb/include/lldb/Breakpoint/Watchpoint.h
index 22fdfd686c3f115..fc11a466ba60540 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -227,8 +227,6 @@ class Watchpoint : public 
std::enable_shared_from_this,
   WatchpointOptions
   m_options; // Settable watchpoint options, which is a delegate to handle
  // the callback machinery.
-  bool m_being_created;
-
   std::unique_ptr m_condition_up; // The condition to test.
 
   void SetID(lldb::watch_id_t id) { m_id = id; }
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp 
b/lldb/source/Breakpoint/Breakpoint.cpp
index af5dcc9cd88d4fa..ae845e92762b970 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -44,17 +44,14 @@ const char 
*Breakpoint::g_option_names[static_cast(
 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

[Lldb-commits] [lldb] [lldb][NFCI] Remove m_being_created from Breakpoint classes (PR #79716)

2024-01-27 Thread via lldb-commits 

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 02d3a799e7eb2997950d6a288a08a5e51ff0ff59 
2f5ccf2e8fea01cd60ce521fede82d3789b383eb -- 
lldb/include/lldb/Breakpoint/Breakpoint.h 
lldb/include/lldb/Breakpoint/BreakpointLocation.h 
lldb/include/lldb/Breakpoint/Watchpoint.h lldb/source/Breakpoint/Breakpoint.cpp 
lldb/source/Breakpoint/BreakpointLocation.cpp 
lldb/source/Breakpoint/Watchpoint.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h 
b/lldb/include/lldb/Breakpoint/Watchpoint.h
index fc11a466ba..16b83fbf7f 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -224,9 +224,8 @@ private:
   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.
+  WatchpointOptions m_options; // Settable watchpoint options, which is a
+   // delegate to handle the callback machinery.
   std::unique_ptr m_condition_up; // The condition to test.
 
   void SetID(lldb::watch_id_t id) { m_id = id; }

``




https://github.com/llvm/llvm-project/pull/79716
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove m_being_created from Breakpoint classes (PR #79716)

2024-01-29 Thread via lldb-commits 


@@ -313,6 +313,8 @@ class BreakpointLocation
 
   void UndoBumpHitCount();
 
+  void SetThreadIDInternal(lldb::tid_t thread_id);

jimingham wrote:

Could you put a comment in here saying how this is different from SetThreadID?

https://github.com/llvm/llvm-project/pull/79716
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove m_being_created from Breakpoint classes (PR #79716)

2024-01-29 Thread via lldb-commits 

jimingham wrote:

LGTM - the new API could use a comment but otherwise this is fine.

https://github.com/llvm/llvm-project/pull/79716
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove m_being_created from Breakpoint classes (PR #79716)

2024-01-29 Thread Alex Langford via lldb-commits 


@@ -313,6 +313,8 @@ class BreakpointLocation
 
   void UndoBumpHitCount();
 
+  void SetThreadIDInternal(lldb::tid_t thread_id);

bulbazord wrote:

Sure

https://github.com/llvm/llvm-project/pull/79716
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove m_being_created from Breakpoint classes (PR #79716)

2024-01-29 Thread Alex Langford via lldb-commits 

https://github.com/bulbazord updated 
https://github.com/llvm/llvm-project/pull/79716

>From fc0c5da871d32a7cf5a6c96ef7f36c49aeccd074 Mon Sep 17 00:00:00 2001
From: Alex Langford 
Date: Sat, 27 Jan 2024 15:54:16 -0800
Subject: [PATCH 1/2] [lldb][NFCI] Remove m_being_created from Breakpoint
 classes

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.
---
 lldb/include/lldb/Breakpoint/Breakpoint.h |  1 -
 .../lldb/Breakpoint/BreakpointLocation.h  |  3 +-
 lldb/include/lldb/Breakpoint/Watchpoint.h |  2 --
 lldb/source/Breakpoint/Breakpoint.cpp | 22 +---
 lldb/source/Breakpoint/BreakpointLocation.cpp | 34 ++-
 lldb/source/Breakpoint/Watchpoint.cpp |  8 ++---
 6 files changed, 32 insertions(+), 38 deletions(-)

diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h 
b/lldb/include/lldb/Breakpoint/Breakpoint.h
index 8c4308ab0bc12d..0b6bf593be37a9 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -637,7 +637,6 @@ class Breakpoint : public 
std::enable_shared_from_this,
   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.
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h 
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 2a4f9fc01bf326..273132c950825a 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -313,6 +313,8 @@ class BreakpointLocation
 
   void UndoBumpHitCount();
 
+  void SetThreadIDInternal(lldb::tid_t thread_id);
+
   // Constructors and Destructors
   //
   // Only the Breakpoint can make breakpoint locations, and it owns them.
@@ -337,7 +339,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;
diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h 
b/lldb/include/lldb/Breakpoint/Watchpoint.h
index 22fdfd686c3f11..fc11a466ba6054 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -227,8 +227,6 @@ class Watchpoint : public 
std::enable_shared_from_this,
   WatchpointOptions
   m_options; // Settable watchpoint options, which is a delegate to handle
  // the callback machinery.
-  bool m_being_created;
-
   std::unique_ptr m_condition_up; // The condition to test.
 
   void SetID(lldb::watch_id_t id) { m_id = id; }
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp 
b/lldb/source/Breakpoint/Breakpoint.cpp
index af5dcc9cd88d4f..ae845e92762b97 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -44,17 +44,14 @@ const char 
*Breakpoint::g_option_names[static_cast(
 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

[Lldb-commits] [lldb] [lldb][NFCI] Remove m_being_created from Breakpoint classes (PR #79716)

2024-01-29 Thread Alex Langford via lldb-commits 


@@ -313,6 +313,8 @@ class BreakpointLocation
 
   void UndoBumpHitCount();
 
+  void SetThreadIDInternal(lldb::tid_t thread_id);

bulbazord wrote:

@jimingham how does this look to you?

https://github.com/llvm/llvm-project/pull/79716
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove m_being_created from Breakpoint classes (PR #79716)

2024-01-29 Thread via lldb-commits 


@@ -313,6 +313,8 @@ class BreakpointLocation
 
   void UndoBumpHitCount();
 
+  void SetThreadIDInternal(lldb::tid_t thread_id);

jimingham wrote:

I don't think you need to mention crashing lldb.  Even if we fix that, telling 
the world about something that's not fully constructed yet is enough of a bad 
idea, we can leave it at that.
Otherwise this is good.

https://github.com/llvm/llvm-project/pull/79716
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFCI] Remove m_being_created from Breakpoint classes (PR #79716)

2024-01-29 Thread Alex Langford via lldb-commits 

https://github.com/bulbazord updated 
https://github.com/llvm/llvm-project/pull/79716

>From 1f81c3e01a12682ad514038d2e9f1baea80dcf03 Mon Sep 17 00:00:00 2001
From: Alex Langford 
Date: Sat, 27 Jan 2024 15:54:16 -0800
Subject: [PATCH 1/3] [lldb][NFCI] Remove m_being_created from Breakpoint
 classes

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.
---
 lldb/include/lldb/Breakpoint/Breakpoint.h |  1 -
 .../lldb/Breakpoint/BreakpointLocation.h  |  3 +-
 lldb/include/lldb/Breakpoint/Watchpoint.h |  2 --
 lldb/source/Breakpoint/Breakpoint.cpp | 22 +---
 lldb/source/Breakpoint/BreakpointLocation.cpp | 34 ++-
 lldb/source/Breakpoint/Watchpoint.cpp |  8 ++---
 6 files changed, 32 insertions(+), 38 deletions(-)

diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h 
b/lldb/include/lldb/Breakpoint/Breakpoint.h
index 8c4308ab0bc12..0b6bf593be37a 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -637,7 +637,6 @@ class Breakpoint : public 
std::enable_shared_from_this,
   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.
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h 
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 2a4f9fc01bf32..273132c950825 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -313,6 +313,8 @@ class BreakpointLocation
 
   void UndoBumpHitCount();
 
+  void SetThreadIDInternal(lldb::tid_t thread_id);
+
   // Constructors and Destructors
   //
   // Only the Breakpoint can make breakpoint locations, and it owns them.
@@ -337,7 +339,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;
diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h 
b/lldb/include/lldb/Breakpoint/Watchpoint.h
index 22fdfd686c3f1..fc11a466ba605 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -227,8 +227,6 @@ class Watchpoint : public 
std::enable_shared_from_this,
   WatchpointOptions
   m_options; // Settable watchpoint options, which is a delegate to handle
  // the callback machinery.
-  bool m_being_created;
-
   std::unique_ptr m_condition_up; // The condition to test.
 
   void SetID(lldb::watch_id_t id) { m_id = id; }
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp 
b/lldb/source/Breakpoint/Breakpoint.cpp
index af5dcc9cd88d4..ae845e92762b9 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -44,17 +44,14 @@ const char 
*Breakpoint::g_option_names[static_cast(
 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 Breakpoi

[Lldb-commits] [lldb] [lldb][NFCI] Remove m_being_created from Breakpoint classes (PR #79716)

2024-01-31 Thread Alex Langford via lldb-commits 

https://github.com/bulbazord updated 
https://github.com/llvm/llvm-project/pull/79716

>From f7bc2e013fb4a5cac93d2c5856983796459fb84b Mon Sep 17 00:00:00 2001
From: Alex Langford 
Date: Sat, 27 Jan 2024 15:54:16 -0800
Subject: [PATCH 1/4] [lldb][NFCI] Remove m_being_created from Breakpoint
 classes

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.
---
 lldb/include/lldb/Breakpoint/Breakpoint.h |  1 -
 .../lldb/Breakpoint/BreakpointLocation.h  |  3 +-
 lldb/include/lldb/Breakpoint/Watchpoint.h |  2 --
 lldb/source/Breakpoint/Breakpoint.cpp | 22 +---
 lldb/source/Breakpoint/BreakpointLocation.cpp | 34 ++-
 lldb/source/Breakpoint/Watchpoint.cpp |  8 ++---
 6 files changed, 32 insertions(+), 38 deletions(-)

diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h 
b/lldb/include/lldb/Breakpoint/Breakpoint.h
index 8c4308ab0bc12..0b6bf593be37a 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -637,7 +637,6 @@ class Breakpoint : public 
std::enable_shared_from_this,
   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.
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h 
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 2a4f9fc01bf32..273132c950825 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -313,6 +313,8 @@ class BreakpointLocation
 
   void UndoBumpHitCount();
 
+  void SetThreadIDInternal(lldb::tid_t thread_id);
+
   // Constructors and Destructors
   //
   // Only the Breakpoint can make breakpoint locations, and it owns them.
@@ -337,7 +339,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;
diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h 
b/lldb/include/lldb/Breakpoint/Watchpoint.h
index 22fdfd686c3f1..fc11a466ba605 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -227,8 +227,6 @@ class Watchpoint : public 
std::enable_shared_from_this,
   WatchpointOptions
   m_options; // Settable watchpoint options, which is a delegate to handle
  // the callback machinery.
-  bool m_being_created;
-
   std::unique_ptr m_condition_up; // The condition to test.
 
   void SetID(lldb::watch_id_t id) { m_id = id; }
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp 
b/lldb/source/Breakpoint/Breakpoint.cpp
index af5dcc9cd88d4..ae845e92762b9 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -44,17 +44,14 @@ const char 
*Breakpoint::g_option_names[static_cast(
 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 Breakpoi

[Lldb-commits] [lldb] [lldb][NFCI] Remove m_being_created from Breakpoint classes (PR #79716)

2024-01-31 Thread Alex Langford via lldb-commits 

https://github.com/bulbazord closed 
https://github.com/llvm/llvm-project/pull/79716
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits