[lldb] Remove Listener::SetShadow

[labath]

It's not used since https://reviews.llvm.org/D157556.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)

Changes

It's not used since https://reviews.llvm.org/D157556.

---

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

4 Files Affected:

• (modified) lldb/include/lldb/Utility/Listener.h (-3)
• (modified) lldb/source/API/SBAttachInfo.cpp (+1-7)
• (modified) lldb/source/API/SBLaunchInfo.cpp (+1-7)
• (modified) lldb/source/Utility/Listener.cpp (+3-6)

diff --git a/lldb/include/lldb/Utility/Listener.h b/lldb/include/lldb/Utility/Listener.h
index daa7deb345f30..f687852d558bd 100644
--- a/lldb/include/lldb/Utility/Listener.h
+++ b/lldb/include/lldb/Utility/Listener.h
@@ -94,8 +94,6 @@ class Listener : public std::enable_shared_from_this<Listener> {
 
   size_t HandleBroadcastEvent(lldb::EventSP &event_sp);
 
-  void SetShadow(bool is_shadow) { m_is_shadow = is_shadow; }
-
 private:
   // Classes that inherit from Listener can see and modify these
   struct BroadcasterInfo {
@@ -132,7 +130,6 @@ class Listener : public std::enable_shared_from_this<Listener> {
   std::mutex m_events_mutex; // Protects m_broadcasters and m_events
   std::condition_variable m_events_condition;
   broadcaster_manager_collection m_broadcaster_managers;
-  bool m_is_shadow = false;
 
   void BroadcasterWillDestruct(Broadcaster *);
 
diff --git a/lldb/source/API/SBAttachInfo.cpp b/lldb/source/API/SBAttachInfo.cpp
index 8ce1f1d65c496..a9f712c79c7fe 100644
--- a/lldb/source/API/SBAttachInfo.cpp
+++ b/lldb/source/API/SBAttachInfo.cpp
@@ -266,13 +266,7 @@ SBListener SBAttachInfo::GetShadowListener() {
 void SBAttachInfo::SetShadowListener(SBListener &listener) {
   LLDB_INSTRUMENT_VA(this, listener);
 
-  ListenerSP listener_sp = listener.GetSP();
-  if (listener_sp && listener.IsValid())
-    listener_sp->SetShadow(true);
-  else
-    listener_sp = nullptr;
-
-  m_opaque_sp->SetShadowListener(listener_sp);
+  m_opaque_sp->SetShadowListener(listener.GetSP());
 }
 
 const char *SBAttachInfo::GetScriptedProcessClassName() const {
diff --git a/lldb/source/API/SBLaunchInfo.cpp b/lldb/source/API/SBLaunchInfo.cpp
index d5f935083e6c1..d6b52e8a67a49 100644
--- a/lldb/source/API/SBLaunchInfo.cpp
+++ b/lldb/source/API/SBLaunchInfo.cpp
@@ -402,11 +402,5 @@ SBListener SBLaunchInfo::GetShadowListener() {
 void SBLaunchInfo::SetShadowListener(SBListener &listener) {
   LLDB_INSTRUMENT_VA(this, listener);
 
-  ListenerSP listener_sp = listener.GetSP();
-  if (listener_sp && listener.IsValid())
-    listener_sp->SetShadow(true);
-  else
-    listener_sp = nullptr;
-
-  m_opaque_sp->SetShadowListener(listener_sp);
+  m_opaque_sp->SetShadowListener(listener.GetSP());
 }
diff --git a/lldb/source/Utility/Listener.cpp b/lldb/source/Utility/Listener.cpp
index 0b28cb5cdc642..317525335f0f6 100644
--- a/lldb/source/Utility/Listener.cpp
+++ b/lldb/source/Utility/Listener.cpp
@@ -18,13 +18,10 @@
 using namespace lldb;
 using namespace lldb_private;
 
-Listener::Listener(const char *name)
-    : m_name(name), m_broadcasters(), m_broadcasters_mutex(), m_events(),
-      m_events_mutex(), m_is_shadow() {
+Listener::Listener(const char *name) : m_name(name) {
   Log *log = GetLog(LLDBLog::Object);
-  if (log != nullptr)
-    LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast<void *>(this),
-              m_name.c_str());
+  LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast<void *>(this),
+            m_name.c_str());
 }
 
 Listener::~Listener() {

[JDevlieghere]

Nit: why not get rid of log altogether:

LLDB_LOGF(GetLog(LLDBLog::Object), "%p Listener::Listener('%s')", static_cast<void *>(this),
             m_name.c_str());

[labath]

Done. I though about it myself, but I had to stop with the drive-by changes somewhere.

[medismailben]

LGTM :)

[jimingham]

I have a bug caused by the fact that we aren't properly marking shadow listeners as shadow listeners, causing them to steal events from the primary listener. I just got it, and haven't started looking at it. I might need SetShadow for that.

[jimingham]

The problem was that currently you can make a process, then do:

listener = lldb.SBListener("my_listener")
process.broadcaster.AddListener(listener, lldb.SBProcess.eBroadcastBitStateChanged)

and nothing marks this as a Shadow listener, so when we go to deliver events, we don't wait on the consumption by the Primary listener.

But I don't think I should need SetShadow for this. I don't think we need to mark the listener outside the "Add" action as Shadow. Instead, AddListener should see if there IS a primary listener, and if there is one, then any other listener has to be a shadow listener.

Actually, I wonder if we really need this distinction to be marked explicitly, I might have been being too general. If you have a primary listener, then all the other listeners have to be shadow listeners. I don't think anything else makes sense. And conversely, if you don't have a primary listener, then I marking some of the listeners as shadow doesn't do anything useful.

[jimingham]

Just to be clear, I have no problem with removing this. Shadow wasn't really a useful property "broadcaster having a primary listener" was the only thing that really mattered.