[lldb][NFCI] Change type of Broadcaster's name

[bulbazord]

Summary

Broadcasters don't need their names in the StringPool. It doesn't
benefit from fast comparisons and doesn't benefit from uniqueness.

[JDevlieghere]

lldb/include/lldb/Utility/Broadcaster.h

157

If we're going to store this as a std::string you should take it it by value and move it into the member.

lldb/source/Utility/Broadcaster.cpp

26–28

[bulbazord]

Address @JDevlieghere's comments

[bulbazord]

LLDB_LOGF -> LLDB_LOG where needed

[JDevlieghere]

LGTM modulo inline question.

lldb/include/lldb/Utility/Broadcaster.h

358–360

Why not return a const std::string & here or, alternatively, why not return a StringRef above?

[bulbazord]

lldb/include/lldb/Utility/Broadcaster.h

358–360

Good question. IMO it'd be nice to return a StringRef here but it's probably more practical to return a const std::string & here since we often pass it to printf-style formatters for logging (which, for StringRef, you'd need to do ref.str().c_str() to do it safely).

[bulbazord]

llvm::StringRef -> const std::string &

lldb/include/lldb/Utility/Broadcaster.h

 ///     \endcode
 class Broadcaster {
   friend class Listener;
   friend class Event;
 
 public:
   /// Construct with a broadcaster with a name.
   ///
+  /// \param[in] manager_sp
+  ///   A shared pointer to the BroadcasterManager that will manage this
+  ///   broadcaster.
   /// \param[in] name
-  ///     A NULL terminated C string that contains the name of the
-  ///     broadcaster object.
-  Broadcaster(lldb::BroadcasterManagerSP manager_sp, const char *name);
+  ///   A std::string of the name that this broadcaster will have.
+  Broadcaster(lldb::BroadcasterManagerSP manager_sp, std::string name);
 
   /// Destructor.
   ///
   /// The destructor is virtual since this class gets subclassed.
   virtual ~Broadcaster();
 
   void CheckInWithManager();
 
public:
   ///
   /// \return
   ///     The actual event bits that were acquired by \a listener.
   uint32_t AddListener(const lldb::ListenerSP &listener_sp,
                        uint32_t event_mask) {
     return m_broadcaster_sp->AddListener(listener_sp, event_mask);
   }
 
-  /// Get the NULL terminated C string name of this Broadcaster object.
+  /// Get this broadcaster's name.
   ///
   /// \return
-  ///     The NULL terminated C string name of this Broadcaster.
-  ConstString GetBroadcasterName() { return m_broadcaster_name; }
+  ///     A reference to a constant std::string containing the name of the
+  ///     broadcaster.
+  const std::string &GetBroadcasterName() { return m_broadcaster_name; }
 
   /// Get the event name(s) for one or more event bits.
   ///
   /// \param[in] event_mask
   ///     A bit mask that indicates which events to get names for.
   ///
   /// \return
   ///     The NULL terminated C string name of this Broadcaster.
public:
     void BroadcastEventIfUnique(uint32_t event_type,
                                 EventData *event_data = nullptr);
 
     void Clear();
 
     uint32_t AddListener(const lldb::ListenerSP &listener_sp,
                          uint32_t event_mask);
 
-    const char *GetBroadcasterName() const {
-      return m_broadcaster.GetBroadcasterName().AsCString();
+    llvm::StringRef GetBroadcasterName() const {
+      return m_broadcaster.GetBroadcasterName();
     }
 
     Broadcaster *GetBroadcaster();
 
     bool GetEventNames(Stream &s, const uint32_t event_mask,
                        bool prefix_with_broadcaster_name) const;
 
     void SetEventName(uint32_t event_mask, const char *name) {
       m_event_names[event_mask] = name;
const char *GetHijackingListenerName() {
     return m_broadcaster_sp->GetHijackingListenerName();
   }
 
 private:
   BroadcasterImplSP m_broadcaster_sp;
   lldb::BroadcasterManagerSP m_manager_sp;
 
   /// The name of this broadcaster object.
-  const ConstString m_broadcaster_name;
+  const std::string m_broadcaster_name;
 
   Broadcaster(const Broadcaster &) = delete;
   const Broadcaster &operator=(const Broadcaster &) = delete;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_UTILITY_BROADCASTER_H

lldb/source/API/SBBroadcaster.cpp

if (m_opaque_ptr)
     return m_opaque_ptr->AddListener(listener.m_opaque_sp, event_mask);
   return 0;
 }
 
 const char *SBBroadcaster::GetName() const {
   LLDB_INSTRUMENT_VA(this);
 
   if (m_opaque_ptr)
-    return m_opaque_ptr->GetBroadcasterName().GetCString();
+    return ConstString(m_opaque_ptr->GetBroadcasterName()).GetCString();
   return nullptr;
 }
 
 bool SBBroadcaster::EventTypeHasListeners(uint32_t event_type) {
   LLDB_INSTRUMENT_VA(this, event_type);
 
   if (m_opaque_ptr)
     return m_opaque_ptr->EventTypeHasListeners(event_type);

lldb/source/Core/ThreadedCommunication.cpp

ThreadedCommunication::ThreadedCommunication(const char *name)
   SetEventName(eBroadcastBitNoMorePendingInput, "no more pending input");
 
   CheckInWithManager();
 }
 
 ThreadedCommunication::~ThreadedCommunication() {
   LLDB_LOG(GetLog(LLDBLog::Object | LLDBLog::Communication),
            "{0} ThreadedCommunication::~ThreadedCommunication (name = {1})",
-           this, GetBroadcasterName().AsCString());
+           this, GetBroadcasterName());
 }
 
 void ThreadedCommunication::Clear() {
   SetReadThreadBytesReceivedCallback(nullptr, nullptr);
   StopReadThread(nullptr);
   Communication::Clear();
 }
 

lldb/source/Utility/Broadcaster.cpp

 #include <utility>
 
 #include <cassert>
 #include <cstddef>
 
 using namespace lldb;
 using namespace lldb_private;
 
-Broadcaster::Broadcaster(BroadcasterManagerSP manager_sp, const char *name)
+Broadcaster::Broadcaster(BroadcasterManagerSP manager_sp, std::string name)
     : m_broadcaster_sp(std::make_shared<BroadcasterImpl>(*this)),
-      m_manager_sp(std::move(manager_sp)), m_broadcaster_name(name) {
+      m_manager_sp(std::move(manager_sp)), m_broadcaster_name(std::move(name)) {
   Log *log = GetLog(LLDBLog::Object);
   LLDB_LOG(log, "{0} Broadcaster::Broadcaster(\"{1}\")",
            static_cast<void *>(this), GetBroadcasterName());
 }
 
 Broadcaster::BroadcasterImpl::BroadcasterImpl(Broadcaster &broadcaster)
     : m_broadcaster(broadcaster), m_listeners(), m_listeners_mutex(),
       m_hijacking_listeners(), m_hijacking_masks() {}
void Broadcaster::BroadcasterImpl::PrivateBroadcastEvent(EventSP &event_sp,
 
   Log *log = GetLog(LLDBLog::Events);
   if (!log && event_sp->GetData())
     log = event_sp->GetData()->GetLogChannel();
 
   if (log) {
     StreamString event_description;
     event_sp->Dump(&event_description);
-    LLDB_LOGF(log,
-              "%p Broadcaster(\"%s\")::BroadcastEvent (event_sp = {%s}, "
-              "unique =%i) hijack = %p",
-              static_cast<void *>(this), GetBroadcasterName(),
-              event_description.GetData(), unique,
-              static_cast<void *>(hijacking_listener_sp.get()));
+    LLDB_LOG(log,
+             "{0:x} Broadcaster(\"{1}\")::BroadcastEvent (event_sp = {2}, "
+             "unique={3}) hijack = {4:x}",
   }
 
   if (hijacking_listener_sp) {
     if (unique && hijacking_listener_sp->PeekAtNextEventForBroadcasterWithType(
                       &m_broadcaster, event_type))
       return;
     hijacking_listener_sp->AddEvent(event_sp);
     if (m_shadow_listener)

lldb/source/Utility/Event.cpp

else
     broadcaster = nullptr;
 
   if (broadcaster) {
     StreamString event_name;
     if (broadcaster->GetEventNames(event_name, m_type, false))
       s->Printf("%p Event: broadcaster = %p (%s), type = 0x%8.8x (%s), data = ",
                 static_cast<const void *>(this),
                 static_cast<void *>(broadcaster),
-                broadcaster->GetBroadcasterName().GetCString(), m_type,
+                broadcaster->GetBroadcasterName().c_str(), m_type,
                 event_name.GetData());
     else
       s->Printf("%p Event: broadcaster = %p (%s), type = 0x%8.8x, data = ",
                 static_cast<const void *>(this),
                 static_cast<void *>(broadcaster),
-                broadcaster->GetBroadcasterName().GetCString(), m_type);
+                broadcaster->GetBroadcasterName().c_str(), m_type);
   } else
     s->Printf("%p Event: broadcaster = NULL, type = 0x%8.8x, data = ",
               static_cast<const void *>(this), m_type);
 
   if (m_data_sp) {
     s->PutChar('{');
     m_data_sp->Dump(s);
     s->PutChar('}');

lldb/source/Utility/Listener.cpp

EventMatcher(Broadcaster *broadcaster, const ConstString *broadcaster_names,
         m_event_type_mask(event_type_mask) {}
 
   bool operator()(const EventSP &event_sp) const {
     if (m_broadcaster && !event_sp->BroadcasterIs(m_broadcaster))
       return false;
 
     if (m_broadcaster_names) {
       bool found_source = false;
-      ConstString event_broadcaster_name =
+      const llvm::StringRef event_broadcaster_name =
           event_sp->GetBroadcaster()->GetBroadcasterName();
       for (uint32_t i = 0; i < m_num_broadcaster_names; ++i) {
         if (m_broadcaster_names[i] == event_broadcaster_name) {
           found_source = true;
           break;
         }
       }
       if (!found_source)