[lldb][NFCI] Module constructor should take ConstString by value

[bulbazord]

Summary

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.

[aprantl]

lldb/include/lldb/Core/Module.h

127

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

lldb/source/Core/Module.cpp

248–249

why not m_object_name(object_name) above?

[bulbazord]

Addressing @aprantl's feedback

[mib]

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp

806 ↗

(On Diff #550812)

Can this be {} ?

lldb/source/Target/Process.cpp

2389 ↗

(On Diff #550812)

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

[fdeazeve]

lldb/include/lldb/Core/Module.h

126–127

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

[bulbazord]

lldb/include/lldb/Core/Module.h

126–127

I didn't really even think about it, I probably should have just done ConstString object_name = ConstString() or something.

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp

806 ↗

(On Diff #550812)

Yes, although I'll probably make the default value of the parameter be an empty ConstString instead.

[bulbazord]

Give the parameter a default value

[bulbazord]

Remove const from parameter in DebugMapModule

[fdeazeve]

LGTM! Good catch :)

[mib]

LGTM!

lldb/include/lldb/Core/Module.h

public:
   ///     within a module (.a files and modules that contain multiple
   ///     architectures).
   ///
   /// \param[in] object_offset
   ///     The offset within an existing module used to extract a
   ///     module within a module (.a files and modules that contain
   ///     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);
 
   template <typename ObjFilePlugin, typename... Args>
   static lldb::ModuleSP CreateModuleFromObjectFile(Args &&...args) {
     // Must create a module and place it into a shared pointer before we can
     // create an object file since it has a std::weak_ptr back to the module,

lldb/source/Core/Module.cpp

Module::Module(const ModuleSpec &module_spec)
   // Always trust the object offset (file offset) and object modification time
   // (for mod time in a BSD static archive) of from the matching module
   // specification
   m_object_offset = matching_module_spec.GetObjectOffset();
   m_object_mod_time = matching_module_spec.GetObjectModificationTime();
 }
 
 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<std::recursive_mutex> guard(
         GetAllocationModuleCollectionMutex());
     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')",
               static_cast<void *>(this), m_arch.GetArchitectureName(),
               m_file.GetPath().c_str(), m_object_name.IsEmpty() ? "" : "(",
               m_object_name.AsCString(""), m_object_name.IsEmpty() ? "" : ")");
 }
 
 Module::Module() : m_file_has_changed(false), m_first_file_changed_log(false) {

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp

SymbolFileDWARFDebugMap::CompileUnitInfo::GetFileRangeMap(
   }
   return file_range_map;
 }
 
 class DebugMapModule : public Module {
 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) {}
 
   ~DebugMapModule() override = default;
 
   SymbolFile *
   GetSymbolFile(bool can_create = true,
if (pos != m_oso_map.end()) {
       oso_arch.SetTriple(m_objfile_sp->GetModule()
                              ->GetArchitecture()
                              .GetTriple()
                              .getArchName()
                              .str()
                              .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() &&
           FileSystem::Instance().Exists(oso_file)) {
         // If we are loading a .o file from a .a file the "oso_object" will
         // have a valid value name and if the .a file exists, either the .o
         // file didn't exist in the .a file or the mod time didn't match.
         comp_unit_info->oso_load_error.SetErrorStringWithFormat(