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

2023-08-17 Thread Alex Langford via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e6d48ef6085: [lldb][NFCI] Module constructor should take 
ConstString by value (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158043/new/

https://reviews.llvm.org/D158043

Files:
  lldb/include/lldb/Core/Module.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp


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,
+ 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(
   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/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -233,12 +233,12 @@
 }
 
 Module::Module(const FileSpec &file_spec, const ArchSpec &arch,
-   const ConstString *object_name, lldb::offset_t object_offset,
+   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),
-  m_object_mod_time(object_mod_time), m_file_has_changed(false),
-  m_first_file_changed_log(false) {
+  m_arch(arch), m_file(file_spec), m_object_name(object_name),
+  m_object_offset(object_offset), m_object_mod_time(object_mod_time),
+  m_file_has_changed(false), m_first_file_changed_log(false) {
   // Scope for locker below...
   {
 std::lock_guard guard(
@@ -246,9 +246,6 @@
 GetModuleCollection().push_back(this);
   }
 
-  if (object_name)
-m_object_name = *object_name;
-
   Log *log(GetLog(LLDBLog::Object | LLDBLog::Modules));
   if (log != nullptr)
 LLDB_LOGF(log, "%p Module::Module((%s) '%s%s%s%s')",
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,
+  ConstString object_name = ConstString(), lldb::offset_t object_offset = 
0,
   const llvm::sys::TimePoint<> &object_mod_time = 
llvm::sys::TimePoint<>());
 
   Module(const ModuleSpec &module_spec);


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,
+ 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(
   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/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Mo

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

2023-08-16 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib accepted this revision.
mib added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158043/new/

https://reviews.llvm.org/D158043

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

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

2023-08-16 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits 
fdeazeve accepted this revision.
fdeazeve added a comment.
This revision is now accepted and ready to land.

LGTM! Good catch :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158043/new/

https://reviews.llvm.org/D158043

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

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

2023-08-16 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord updated this revision to Diff 550824.
bulbazord added a comment.

Remove `const` from parameter in DebugMapModule


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158043/new/

https://reviews.llvm.org/D158043

Files:
  lldb/include/lldb/Core/Module.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp


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,
+ 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(
   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/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -233,12 +233,12 @@
 }
 
 Module::Module(const FileSpec &file_spec, const ArchSpec &arch,
-   const ConstString *object_name, lldb::offset_t object_offset,
+   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),
-  m_object_mod_time(object_mod_time), m_file_has_changed(false),
-  m_first_file_changed_log(false) {
+  m_arch(arch), m_file(file_spec), m_object_name(object_name),
+  m_object_offset(object_offset), m_object_mod_time(object_mod_time),
+  m_file_has_changed(false), m_first_file_changed_log(false) {
   // Scope for locker below...
   {
 std::lock_guard guard(
@@ -246,9 +246,6 @@
 GetModuleCollection().push_back(this);
   }
 
-  if (object_name)
-m_object_name = *object_name;
-
   Log *log(GetLog(LLDBLog::Object | LLDBLog::Modules));
   if (log != nullptr)
 LLDB_LOGF(log, "%p Module::Module((%s) '%s%s%s%s')",
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,
+  ConstString object_name = ConstString(), lldb::offset_t object_offset = 
0,
   const llvm::sys::TimePoint<> &object_mod_time = 
llvm::sys::TimePoint<>());
 
   Module(const ModuleSpec &module_spec);


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,
+ 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(
   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/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -233,12 +233,12 @@
 }
 
 Module::Module(const FileSpec &file_

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

2023-08-16 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord updated this revision to Diff 550822.
bulbazord added a comment.

Give the parameter a default value


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158043/new/

https://reviews.llvm.org/D158043

Files:
  lldb/include/lldb/Core/Module.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp


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(
   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/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -233,12 +233,12 @@
 }
 
 Module::Module(const FileSpec &file_spec, const ArchSpec &arch,
-   const ConstString *object_name, lldb::offset_t object_offset,
+   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),
-  m_object_mod_time(object_mod_time), m_file_has_changed(false),
-  m_first_file_changed_log(false) {
+  m_arch(arch), m_file(file_spec), m_object_name(object_name),
+  m_object_offset(object_offset), m_object_mod_time(object_mod_time),
+  m_file_has_changed(false), m_first_file_changed_log(false) {
   // Scope for locker below...
   {
 std::lock_guard guard(
@@ -246,9 +246,6 @@
 GetModuleCollection().push_back(this);
   }
 
-  if (object_name)
-m_object_name = *object_name;
-
   Log *log(GetLog(LLDBLog::Object | LLDBLog::Modules));
   if (log != nullptr)
 LLDB_LOGF(log, "%p Module::Module((%s) '%s%s%s%s')",
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,
+  ConstString object_name = ConstString(), lldb::offset_t object_offset = 
0,
   const llvm::sys::TimePoint<> &object_mod_time = 
llvm::sys::TimePoint<>());
 
   Module(const ModuleSpec &module_spec);


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(
   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/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -233,12 +233,12 @@
 }
 
 Module::Module(const FileSpec &file_s

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

2023-08-16 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord added inline comments.



Comment at: lldb/include/lldb/Core/Module.h:126
   Module(
-  const FileSpec &file_spec, const ArchSpec &arch,
-  const ConstString *object_name = nullptr,
+  const FileSpec &file_spec, const ArchSpec &arch, ConstString object_name,
   lldb::offset_t object_offset = 0,

fdeazeve wrote:
> Out of curiosity, what was the rationale for removing the default parameter 
> here?
I didn't really even think about it, I probably should have just done 
`ConstString object_name = ConstString()` or something.



Comment at: 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:806
+   target.GetArchitecture(),
+   ConstString());
   }

mib wrote:
> Can this be `{}` ?
Yes, although I'll probably make the default value of the parameter be an empty 
ConstString instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158043/new/

https://reviews.llvm.org/D158043

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

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

2023-08-16 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits 
fdeazeve added inline comments.



Comment at: lldb/include/lldb/Core/Module.h:126
   Module(
-  const FileSpec &file_spec, const ArchSpec &arch,
-  const ConstString *object_name = nullptr,
+  const FileSpec &file_spec, const ArchSpec &arch, ConstString object_name,
   lldb::offset_t object_offset = 0,

Out of curiosity, what was the rationale for removing the default parameter 
here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158043/new/

https://reviews.llvm.org/D158043

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

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

2023-08-16 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib added inline comments.



Comment at: 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:806
+   target.GetArchitecture(),
+   ConstString());
   }

Can this be `{}` ?



Comment at: lldb/source/Target/Process.cpp:2389
   }
-  ModuleSP module_sp(new Module(file_spec, ArchSpec()));
+  ModuleSP module_sp(new Module(file_spec, ArchSpec(), ConstString()));
   if (module_sp) {

Should the module constructor have this as a default value for the 
`ConstString` parameter ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158043/new/

https://reviews.llvm.org/D158043

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

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

2023-08-16 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord updated this revision to Diff 550812.
bulbazord added a comment.

Addressing @aprantl's feedback


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158043/new/

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(fspec, aspec);
+  lldb::ModuleSP module = std::make_shared(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(
   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_spec.GetFileSpec(),
-   target.GetArchitecture());
+   target.GetArchitecture(),
+   ConstString());
   }
 }
   }
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -233,12 +233,12 @@
 }
 
 Module::Module(const FileSpec &file_spec, const ArchSpec &arch,
-   const ConstString *object_name, lldb::offset_t object_offset,
+   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),
-  m_object_mod_time(object_mod_time), m_file_has_changed(false),
-  m_first_file_changed_log(false) {
+  m_arch(arch), m_file(file_spec), m_object_name(object_name),
+  m_object_offset(object_offset), m_object_mod_time(object_mod_time),
+  m_file_has_changed(false), m_first_file_changed_log(false) {
   // Scope for locker below...
   {
 std::lock_guard guard(
@@ -246,9 +246,6 @@
 GetModuleCollection().push_back(this);
   }
 
-  if (object_name)
-m_object_name = *object_name;
-
   Log *log(GetLog(LLDBLog::Object | LLDBLog::

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

2023-08-16 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added inline comments.



Comment at: lldb/include/lldb/Core/Module.h:127
   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<>());

If we're passing it by value, why the const qualifier?



Comment at: lldb/source/Core/Module.cpp:249
 
-  if (object_name)
-m_object_name = *object_name;
+  m_object_name = object_name;
 

why not `m_object_name(object_name)` above?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158043/new/

https://reviews.llvm.org/D158043

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

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

2023-08-15 Thread Alex Langford via Phabricator via lldb-commits 
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(fspec, aspec);
+  lldb::ModuleSP module = std::make_shared(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(
   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_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