https://github.com/bulbazord created 
https://github.com/llvm/llvm-project/pull/89690

These are hardcoded strings that are already present in the data section of the 
binary, no need to immediately place them in the ConstString StringPools. Lots 
of code still calls `GetBroadcasterClass` and places the return value into a 
ConstString. Changing that would be a good follow-up.

Additionally, calls to these functions are still wrapped in ConstStrings at the 
SBAPI layer. This is because we must guarantee the lifetime of all strings 
handed out publicly.

>From 406ce4dc0151cf1afdc3ab0fa7b8743029b869f3 Mon Sep 17 00:00:00 2001
From: Alex Langford <alangf...@apple.com>
Date: Mon, 22 Apr 2024 17:07:57 -0700
Subject: [PATCH] [lldb][nfc] Move broadcaster class strings away from
 ConstString

These are hardcoded strings that are already present in the data section
of the binary, no need to immediately place them in the ConstString
StringPools. Lots of code still calls `GetBroadcasterClass` and places
the return value into a ConstString. Changing that would be a good
follow-up.

Additionally, calls to these functions are still wrapped in ConstStrings
at the SBAPI layer. This is because we must guarantee the lifetime of
all strings handed out publicly.
---
 lldb/include/lldb/Core/Debugger.h                  | 2 +-
 lldb/include/lldb/Core/ThreadedCommunication.h     | 4 ++--
 lldb/include/lldb/Interpreter/CommandInterpreter.h | 4 ++--
 lldb/include/lldb/Target/Process.h                 | 4 ++--
 lldb/include/lldb/Target/Target.h                  | 4 ++--
 lldb/include/lldb/Target/TargetList.h              | 4 ++--
 lldb/include/lldb/Target/Thread.h                  | 4 ++--
 lldb/include/lldb/Utility/Broadcaster.h            | 8 ++++----
 lldb/source/API/SBCommandInterpreter.cpp           | 3 ++-
 lldb/source/API/SBCommunication.cpp                | 3 ++-
 lldb/source/API/SBDebugger.cpp                     | 2 +-
 lldb/source/API/SBEvent.cpp                        | 3 ++-
 lldb/source/API/SBProcess.cpp                      | 4 ++--
 lldb/source/API/SBTarget.cpp                       | 2 +-
 lldb/source/API/SBThread.cpp                       | 2 +-
 lldb/source/Core/Debugger.cpp                      | 6 +++---
 lldb/source/Core/ThreadedCommunication.cpp         | 4 ++--
 lldb/source/Interpreter/CommandInterpreter.cpp     | 6 +++---
 lldb/source/Target/Process.cpp                     | 6 +++---
 lldb/source/Target/Target.cpp                      | 6 +++---
 lldb/source/Target/TargetList.cpp                  | 6 +++---
 lldb/source/Target/Thread.cpp                      | 6 +++---
 lldb/source/Utility/Broadcaster.cpp                | 4 ++--
 23 files changed, 50 insertions(+), 47 deletions(-)

diff --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 418c2403d020f4..49ff0737acef82 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -89,7 +89,7 @@ class Debugger : public 
std::enable_shared_from_this<Debugger>,
 
   using DebuggerList = std::vector<lldb::DebuggerSP>;
 
-  static ConstString GetStaticBroadcasterClass();
+  static llvm::StringRef GetStaticBroadcasterClass();
 
   /// Get the public broadcaster for this debugger.
   Broadcaster &GetBroadcaster() { return m_broadcaster; }
diff --git a/lldb/include/lldb/Core/ThreadedCommunication.h 
b/lldb/include/lldb/Core/ThreadedCommunication.h
index 7ebb77beb77f3d..24412b2027932d 100644
--- a/lldb/include/lldb/Core/ThreadedCommunication.h
+++ b/lldb/include/lldb/Core/ThreadedCommunication.h
@@ -216,9 +216,9 @@ class ThreadedCommunication : public Communication, public 
Broadcaster {
   ///
   void SynchronizeWithReadThread();
 
-  static ConstString &GetStaticBroadcasterClass();
+  static llvm::StringRef GetStaticBroadcasterClass();
 
-  ConstString &GetBroadcasterClass() const override {
+  llvm::StringRef GetBroadcasterClass() const override {
     return GetStaticBroadcasterClass();
   }
 
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index d190bcdcab4497..70a55a77465bfe 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -255,9 +255,9 @@ class CommandInterpreter : public Broadcaster,
 
   // These two functions fill out the Broadcaster interface:
 
-  static ConstString &GetStaticBroadcasterClass();
+  static llvm::StringRef GetStaticBroadcasterClass();
 
-  ConstString &GetBroadcasterClass() const override {
+  llvm::StringRef GetBroadcasterClass() const override {
     return GetStaticBroadcasterClass();
   }
 
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 2f3a3c22422efe..aac0cf51680a9e 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -381,7 +381,7 @@ class Process : public 
std::enable_shared_from_this<Process>,
 
   // These two functions fill out the Broadcaster interface:
 
-  static ConstString &GetStaticBroadcasterClass();
+  static llvm::StringRef GetStaticBroadcasterClass();
 
   static constexpr llvm::StringRef AttachSynchronousHijackListenerName =
       "lldb.internal.Process.AttachSynchronous.hijack";
@@ -390,7 +390,7 @@ class Process : public 
std::enable_shared_from_this<Process>,
   static constexpr llvm::StringRef ResumeSynchronousHijackListenerName =
       "lldb.internal.Process.ResumeSynchronous.hijack";
 
-  ConstString &GetBroadcasterClass() const override {
+  llvm::StringRef GetBroadcasterClass() const override {
     return GetStaticBroadcasterClass();
   }
 
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 2c2e6b2831ccee..cade60f3cc8cd9 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -499,9 +499,9 @@ class Target : public std::enable_shared_from_this<Target>,
 
   // These two functions fill out the Broadcaster interface:
 
-  static ConstString &GetStaticBroadcasterClass();
+  static llvm::StringRef GetStaticBroadcasterClass();
 
-  ConstString &GetBroadcasterClass() const override {
+  llvm::StringRef GetBroadcasterClass() const override {
     return GetStaticBroadcasterClass();
   }
 
diff --git a/lldb/include/lldb/Target/TargetList.h 
b/lldb/include/lldb/Target/TargetList.h
index a0bc6f1f820b84..a0cddc6b2966f5 100644
--- a/lldb/include/lldb/Target/TargetList.h
+++ b/lldb/include/lldb/Target/TargetList.h
@@ -37,9 +37,9 @@ class TargetList : public Broadcaster {
 
   // These two functions fill out the Broadcaster interface:
 
-  static ConstString &GetStaticBroadcasterClass();
+  static llvm::StringRef GetStaticBroadcasterClass();
 
-  ConstString &GetBroadcasterClass() const override {
+  llvm::StringRef GetBroadcasterClass() const override {
     return GetStaticBroadcasterClass();
   }
 
diff --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index 1efef93b17ded8..c17bddf4d98b85 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -74,9 +74,9 @@ class Thread : public std::enable_shared_from_this<Thread>,
     eBroadcastBitThreadSelected = (1 << 4)
   };
 
-  static ConstString &GetStaticBroadcasterClass();
+  static llvm::StringRef GetStaticBroadcasterClass();
 
-  ConstString &GetBroadcasterClass() const override {
+  llvm::StringRef GetBroadcasterClass() const override {
     return GetStaticBroadcasterClass();
   }
 
diff --git a/lldb/include/lldb/Utility/Broadcaster.h 
b/lldb/include/lldb/Utility/Broadcaster.h
index f39e677fe9ee04..58436ddb9f26d8 100644
--- a/lldb/include/lldb/Utility/Broadcaster.h
+++ b/lldb/include/lldb/Utility/Broadcaster.h
@@ -39,12 +39,12 @@ namespace lldb_private {
 /// Debugger maintains a list of BroadcastEventSpec's and when it is made
 class BroadcastEventSpec {
 public:
-  BroadcastEventSpec(const ConstString &broadcaster_class, uint32_t event_bits)
+  BroadcastEventSpec(llvm::StringRef broadcaster_class, uint32_t event_bits)
       : m_broadcaster_class(broadcaster_class), m_event_bits(event_bits) {}
 
   ~BroadcastEventSpec() = default;
 
-  ConstString GetBroadcasterClass() const { return m_broadcaster_class; }
+  const std::string &GetBroadcasterClass() const { return m_broadcaster_class; 
}
 
   uint32_t GetEventBits() const { return m_event_bits; }
 
@@ -67,7 +67,7 @@ class BroadcastEventSpec {
   bool operator<(const BroadcastEventSpec &rhs) const;
 
 private:
-  ConstString m_broadcaster_class;
+  std::string m_broadcaster_class;
   uint32_t m_event_bits;
 };
 
@@ -307,7 +307,7 @@ class Broadcaster {
   /// FIXME: Probably should make a ManagedBroadcaster subclass with all the
   /// bits needed to work with the BroadcasterManager, so that it is clearer
   /// how to add one.
-  virtual ConstString &GetBroadcasterClass() const;
+  virtual llvm::StringRef GetBroadcasterClass() const;
 
   lldb::BroadcasterManagerSP GetManager();
 
diff --git a/lldb/source/API/SBCommandInterpreter.cpp 
b/lldb/source/API/SBCommandInterpreter.cpp
index 7b87dc507e4bea..83c0951c56db60 100644
--- a/lldb/source/API/SBCommandInterpreter.cpp
+++ b/lldb/source/API/SBCommandInterpreter.cpp
@@ -512,7 +512,8 @@ SBBroadcaster SBCommandInterpreter::GetBroadcaster() {
 const char *SBCommandInterpreter::GetBroadcasterClass() {
   LLDB_INSTRUMENT();
 
-  return CommandInterpreter::GetStaticBroadcasterClass().AsCString();
+  return ConstString(CommandInterpreter::GetStaticBroadcasterClass())
+      .AsCString();
 }
 
 const char *SBCommandInterpreter::GetArgumentTypeAsCString(
diff --git a/lldb/source/API/SBCommunication.cpp 
b/lldb/source/API/SBCommunication.cpp
index f93898718be6c0..ee33e2abd854e8 100644
--- a/lldb/source/API/SBCommunication.cpp
+++ b/lldb/source/API/SBCommunication.cpp
@@ -170,5 +170,6 @@ SBBroadcaster SBCommunication::GetBroadcaster() {
 const char *SBCommunication::GetBroadcasterClass() {
   LLDB_INSTRUMENT();
 
-  return ThreadedCommunication::GetStaticBroadcasterClass().AsCString();
+  return ConstString(ThreadedCommunication::GetStaticBroadcasterClass())
+      .AsCString();
 }
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index fbcf30e67fc1cd..9c662dfbf4417e 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -112,7 +112,7 @@ SBDebugger &SBDebugger::operator=(const SBDebugger &rhs) {
 const char *SBDebugger::GetBroadcasterClass() {
   LLDB_INSTRUMENT();
 
-  return Debugger::GetStaticBroadcasterClass().AsCString();
+  return ConstString(Debugger::GetStaticBroadcasterClass()).AsCString();
 }
 
 const char *SBDebugger::GetProgressFromEvent(const lldb::SBEvent &event,
diff --git a/lldb/source/API/SBEvent.cpp b/lldb/source/API/SBEvent.cpp
index cc611449e25099..aa9c0ff097d456 100644
--- a/lldb/source/API/SBEvent.cpp
+++ b/lldb/source/API/SBEvent.cpp
@@ -95,7 +95,8 @@ const char *SBEvent::GetBroadcasterClass() const {
 
   const Event *lldb_event = get();
   if (lldb_event)
-    return lldb_event->GetBroadcaster()->GetBroadcasterClass().AsCString();
+    return ConstString(lldb_event->GetBroadcaster()->GetBroadcasterClass())
+        .AsCString();
   else
     return "unknown class";
 }
diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp
index b80664882ebcac..c73348fde3f74d 100644
--- a/lldb/source/API/SBProcess.cpp
+++ b/lldb/source/API/SBProcess.cpp
@@ -77,7 +77,7 @@ SBProcess::~SBProcess() = default;
 const char *SBProcess::GetBroadcasterClassName() {
   LLDB_INSTRUMENT();
 
-  return Process::GetStaticBroadcasterClass().AsCString();
+  return ConstString(Process::GetStaticBroadcasterClass()).AsCString();
 }
 
 const char *SBProcess::GetPluginName() {
@@ -807,7 +807,7 @@ SBBroadcaster SBProcess::GetBroadcaster() const {
 const char *SBProcess::GetBroadcasterClass() {
   LLDB_INSTRUMENT();
 
-  return Process::GetStaticBroadcasterClass().AsCString();
+  return ConstString(Process::GetStaticBroadcasterClass()).AsCString();
 }
 
 size_t SBProcess::ReadMemory(addr_t addr, void *dst, size_t dst_len,
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index cc9f1fdd76afaa..75f0444f629114 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -147,7 +147,7 @@ SBModule SBTarget::GetModuleAtIndexFromEvent(const uint32_t 
idx,
 const char *SBTarget::GetBroadcasterClassName() {
   LLDB_INSTRUMENT();
 
-  return Target::GetStaticBroadcasterClass().AsCString();
+  return ConstString(Target::GetStaticBroadcasterClass()).AsCString();
 }
 
 bool SBTarget::IsValid() const {
diff --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp
index eb9cf063802cd4..ac3e2cd25daa94 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -53,7 +53,7 @@ using namespace lldb_private;
 const char *SBThread::GetBroadcasterClassName() {
   LLDB_INSTRUMENT();
 
-  return Thread::GetStaticBroadcasterClass().AsCString();
+  return ConstString(Thread::GetStaticBroadcasterClass()).AsCString();
 }
 
 // Constructors
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index ebd112110e5f2d..4771ba41550e08 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -823,8 +823,8 @@ TargetSP Debugger::FindTargetWithProcess(Process *process) {
   return target_sp;
 }
 
-ConstString Debugger::GetStaticBroadcasterClass() {
-  static ConstString class_name("lldb.debugger");
+llvm::StringRef Debugger::GetStaticBroadcasterClass() {
+  static constexpr llvm::StringLiteral class_name("lldb.debugger");
   return class_name;
 }
 
@@ -846,7 +846,7 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, 
void *baton)
       m_loaded_plugins(), m_event_handler_thread(), m_io_handler_thread(),
       m_sync_broadcaster(nullptr, "lldb.debugger.sync"),
       m_broadcaster(m_broadcaster_manager_sp,
-                    GetStaticBroadcasterClass().AsCString()),
+                    GetStaticBroadcasterClass().str()),
       m_forward_listener_sp(), m_clear_once() {
   // Initialize the debugger properties as early as possible as other parts of
   // LLDB will start querying them during construction.
diff --git a/lldb/source/Core/ThreadedCommunication.cpp 
b/lldb/source/Core/ThreadedCommunication.cpp
index 7d8aae5d8ff689..2f3dada3ac93b1 100644
--- a/lldb/source/Core/ThreadedCommunication.cpp
+++ b/lldb/source/Core/ThreadedCommunication.cpp
@@ -32,8 +32,8 @@
 using namespace lldb;
 using namespace lldb_private;
 
-ConstString &ThreadedCommunication::GetStaticBroadcasterClass() {
-  static ConstString class_name("lldb.communication");
+llvm::StringRef ThreadedCommunication::GetStaticBroadcasterClass() {
+  static constexpr llvm::StringLiteral class_name("lldb.communication");
   return class_name;
 }
 
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 8c3972a2ba4ce2..4c58ecc3c1848f 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -119,15 +119,15 @@ enum {
 #include "InterpreterPropertiesEnum.inc"
 };
 
-ConstString &CommandInterpreter::GetStaticBroadcasterClass() {
-  static ConstString class_name("lldb.commandInterpreter");
+llvm::StringRef CommandInterpreter::GetStaticBroadcasterClass() {
+  static constexpr llvm::StringLiteral class_name("lldb.commandInterpreter");
   return class_name;
 }
 
 CommandInterpreter::CommandInterpreter(Debugger &debugger,
                                        bool synchronous_execution)
     : Broadcaster(debugger.GetBroadcasterManager(),
-                  CommandInterpreter::GetStaticBroadcasterClass().AsCString()),
+                  CommandInterpreter::GetStaticBroadcasterClass().str()),
       Properties(
           OptionValuePropertiesSP(new OptionValueProperties("interpreter"))),
       IOHandlerDelegate(IOHandlerDelegate::Completion::LLDBCommand),
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 606518ca541267..30c240b064b59c 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -408,8 +408,8 @@ ProcessSP Process::FindPlugin(lldb::TargetSP target_sp,
   return process_sp;
 }
 
-ConstString &Process::GetStaticBroadcasterClass() {
-  static ConstString class_name("lldb.process");
+llvm::StringRef Process::GetStaticBroadcasterClass() {
+  static constexpr llvm::StringLiteral class_name("lldb.process");
   return class_name;
 }
 
@@ -423,7 +423,7 @@ Process::Process(lldb::TargetSP target_sp, ListenerSP 
listener_sp,
                  const UnixSignalsSP &unix_signals_sp)
     : ProcessProperties(this),
       Broadcaster((target_sp->GetDebugger().GetBroadcasterManager()),
-                  Process::GetStaticBroadcasterClass().AsCString()),
+                  Process::GetStaticBroadcasterClass().str()),
       m_target_wp(target_sp), m_public_state(eStateUnloaded),
       m_private_state(eStateUnloaded),
       m_private_state_broadcaster(nullptr,
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 09b0ac42631d1a..aa4895bb5a6d65 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -87,8 +87,8 @@ const Target::Arch &Target::Arch::operator=(const ArchSpec 
&spec) {
   return *this;
 }
 
-ConstString &Target::GetStaticBroadcasterClass() {
-  static ConstString class_name("lldb.target");
+llvm::StringRef Target::GetStaticBroadcasterClass() {
+  static constexpr llvm::StringLiteral class_name("lldb.target");
   return class_name;
 }
 
@@ -96,7 +96,7 @@ Target::Target(Debugger &debugger, const ArchSpec 
&target_arch,
                const lldb::PlatformSP &platform_sp, bool is_dummy_target)
     : TargetProperties(this),
       Broadcaster(debugger.GetBroadcasterManager(),
-                  Target::GetStaticBroadcasterClass().AsCString()),
+                  Target::GetStaticBroadcasterClass().str()),
       ExecutionContextScope(), m_debugger(debugger), 
m_platform_sp(platform_sp),
       m_mutex(), m_arch(target_arch), m_images(this), m_section_load_history(),
       m_breakpoint_list(false), m_internal_breakpoint_list(true),
diff --git a/lldb/source/Target/TargetList.cpp 
b/lldb/source/Target/TargetList.cpp
index b5d308739d0fac..10467753666f88 100644
--- a/lldb/source/Target/TargetList.cpp
+++ b/lldb/source/Target/TargetList.cpp
@@ -29,15 +29,15 @@
 using namespace lldb;
 using namespace lldb_private;
 
-ConstString &TargetList::GetStaticBroadcasterClass() {
-  static ConstString class_name("lldb.targetList");
+llvm::StringRef TargetList::GetStaticBroadcasterClass() {
+  static constexpr llvm::StringLiteral class_name("lldb.targetList");
   return class_name;
 }
 
 // TargetList constructor
 TargetList::TargetList(Debugger &debugger)
     : Broadcaster(debugger.GetBroadcasterManager(),
-                  TargetList::GetStaticBroadcasterClass().AsCString()),
+                  TargetList::GetStaticBroadcasterClass().str()),
       m_target_list(), m_target_list_mutex(), m_selected_target_idx(0) {
   CheckInWithManager();
 }
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 412e44ede9c13b..e75f5a356cec2f 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -205,15 +205,15 @@ Thread::ThreadEventData::GetStackFrameFromEvent(const 
Event *event_ptr) {
 
 // Thread class
 
-ConstString &Thread::GetStaticBroadcasterClass() {
-  static ConstString class_name("lldb.thread");
+llvm::StringRef Thread::GetStaticBroadcasterClass() {
+  static constexpr llvm::StringLiteral class_name("lldb.thread");
   return class_name;
 }
 
 Thread::Thread(Process &process, lldb::tid_t tid, bool use_invalid_index_id)
     : ThreadProperties(false), UserID(tid),
       Broadcaster(process.GetTarget().GetDebugger().GetBroadcasterManager(),
-                  Thread::GetStaticBroadcasterClass().AsCString()),
+                  Thread::GetStaticBroadcasterClass().str()),
       m_process_wp(process.shared_from_this()), m_stop_info_sp(),
       m_stop_info_stop_id(0), m_stop_info_override_stop_id(0),
       m_should_run_before_public_stop(false),
diff --git a/lldb/source/Utility/Broadcaster.cpp 
b/lldb/source/Utility/Broadcaster.cpp
index 12903edc36b1b9..bd65ffd86a1d0f 100644
--- a/lldb/source/Utility/Broadcaster.cpp
+++ b/lldb/source/Utility/Broadcaster.cpp
@@ -373,8 +373,8 @@ void Broadcaster::BroadcasterImpl::RestoreBroadcaster() {
     m_hijacking_masks.pop_back();
 }
 
-ConstString &Broadcaster::GetBroadcasterClass() const {
-  static ConstString class_name("lldb.anonymous");
+llvm::StringRef Broadcaster::GetBroadcasterClass() const {
+  static constexpr llvm::StringLiteral class_name("lldb.anonymous");
   return class_name;
 }
 

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to