[Lldb-commits] [PATCH] D158043: [lldb][NFCI] Module constructor should take ConstString by value

Alex Langford via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Aug 15 18:08:41 PDT 2023


bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, jingham, mib, aprantl.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

ConstStrings are super cheap to copy around. It is often more expensive
to pass a pointer and potentially dereference it than just to always copy it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158043

Files:
  lldb/include/lldb/Core/Module.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Target/Process.cpp
  lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp


Index: lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
===================================================================
--- lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
@@ -57,7 +57,7 @@
   // Test that when we have Dwarf debug info, SymbolFileDWARF is used.
   FileSpec fspec(m_dwarf_test_exe);
   ArchSpec aspec("i686-pc-windows");
-  lldb::ModuleSP module = std::make_shared<Module>(fspec, aspec);
+  lldb::ModuleSP module = std::make_shared<Module>(fspec, aspec, ConstString());
 
   SymbolFile *symfile = module->GetSymbolFile();
   ASSERT_NE(nullptr, symfile);
Index: lldb/source/Target/Process.cpp
===================================================================
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2386,7 +2386,7 @@
               "Process::ReadModuleFromMemory reading %s binary from memory",
               file_spec.GetPath().c_str());
   }
-  ModuleSP module_sp(new Module(file_spec, ArchSpec()));
+  ModuleSP module_sp(new Module(file_spec, ArchSpec(), ConstString()));
   if (module_sp) {
     Status error;
     ObjectFile *objfile = module_sp->GetMemoryObjectFile(
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -170,7 +170,7 @@
 public:
   DebugMapModule(const ModuleSP &exe_module_sp, uint32_t cu_idx,
                  const FileSpec &file_spec, const ArchSpec &arch,
-                 const ConstString *object_name, off_t object_offset,
+                 const ConstString object_name, off_t object_offset,
                  const llvm::sys::TimePoint<> object_mod_time)
       : Module(file_spec, arch, object_name, object_offset, object_mod_time),
         m_exe_module_wp(exe_module_sp), m_cu_idx(cu_idx) {}
@@ -459,7 +459,7 @@
                              .c_str());
       comp_unit_info->oso_sp->module_sp = std::make_shared<DebugMapModule>(
           obj_file->GetModule(), GetCompUnitInfoIndex(comp_unit_info), oso_file,
-          oso_arch, oso_object ? &oso_object : nullptr, 0,
+          oso_arch, oso_object, 0,
           oso_object ? comp_unit_info->oso_mod_time : llvm::sys::TimePoint<>());
 
       if (oso_object && !comp_unit_info->oso_sp->module_sp->GetObjectFile() &&
Index: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===================================================================
--- lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -802,7 +802,8 @@
                                                  kernel_search_error, true)) {
           if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
             m_module_sp = std::make_shared<Module>(module_spec.GetFileSpec(),
-                                                   target.GetArchitecture());
+                                                   target.GetArchitecture(),
+                                                   ConstString());
           }
         }
       }
Index: lldb/source/Core/Module.cpp
===================================================================
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -233,7 +233,7 @@
 }
 
 Module::Module(const FileSpec &file_spec, const ArchSpec &arch,
-               const ConstString *object_name, lldb::offset_t object_offset,
+               const ConstString object_name, lldb::offset_t object_offset,
                const llvm::sys::TimePoint<> &object_mod_time)
     : m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)),
       m_arch(arch), m_file(file_spec), m_object_offset(object_offset),
@@ -246,8 +246,7 @@
     GetModuleCollection().push_back(this);
   }
 
-  if (object_name)
-    m_object_name = *object_name;
+  m_object_name = object_name;
 
   Log *log(GetLog(LLDBLog::Object | LLDBLog::Modules));
   if (log != nullptr)
Index: lldb/include/lldb/Core/Module.h
===================================================================
--- lldb/include/lldb/Core/Module.h
+++ lldb/include/lldb/Core/Module.h
@@ -124,8 +124,7 @@
   ///     multiple architectures).
   Module(
       const FileSpec &file_spec, const ArchSpec &arch,
-      const ConstString *object_name = nullptr,
-      lldb::offset_t object_offset = 0,
+      const ConstString object_name, lldb::offset_t object_offset = 0,
       const llvm::sys::TimePoint<> &object_mod_time = llvm::sys::TimePoint<>());
 
   Module(const ModuleSpec &module_spec);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D158043.550566.patch
Type: text/x-patch
Size: 4752 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20230816/3051743f/attachment.bin>


More information about the lldb-commits mailing list