Remove Module::GetSymbolVendor

[labath]

Summary

This patch removes the GetSymbolVendor function, and the various
mentions of the SymbolVendor in the Module class. The implementation of
GetSymbolVendor is "inlined" into the GetSymbolFile class which I
created earlier.

After this patch, the SymbolVendor class still exists inside the Module
object, but only as an implementation detail -- a fancy holder for the
SymbolFile. That will be removed in the next patch.

[clayborg]

Optional rename in inline comments, but looks good.

include/lldb/Core/Module.h

1006 ↗

(On Diff #213857)

maybe name "m_did_load_symfile"?

[labath]

include/lldb/Core/Module.h

1006 ↗

(On Diff #213857)

good idea.

lldb/trunk/include/lldb/Core/Module.h

 class ObjectFile;
 class RegularExpression;
 class SectionList;
 class Stream;
 class Symbol;
 class SymbolContext;
 class SymbolContextList;
 class SymbolFile;
-class SymbolVendor;
 class Symtab;
 class Target;
 class TypeList;
 class TypeMap;
 class VariableList;
 
 /// \class Module Module.h "lldb/Core/Module.h"
 /// A class that describes an executable image and its associated
 ///        object and symbol files.
 ///
 /// The module is designed to be able to select a single slice of an
 /// executable image as it would appear on disk and during program execution.
 ///
 /// Modules control when and if information is parsed according to which
 /// accessors are called. For example the object file (ObjectFile)
 /// representation will only be parsed if the object file is requested using
 /// the Module::GetObjectFile() is called. The debug symbols will only be
-/// parsed if the symbol vendor (SymbolVendor) is requested using the
-/// Module::GetSymbolVendor() is called.
+/// parsed if the symbol file (SymbolFile) is requested using the
+/// Module::GetSymbolFile() method.
 ///
 /// The module will parse more detailed information as more queries are made.
 class Module : public std::enable_shared_from_this<Module>,
                public SymbolContextScope {
 public:
   // Static functions that can track the lifetime of module objects. This is
   // handy because we might have Module objects that are in shared pointers
   // that aren't in the global module list (from ModuleList). If this is the
public:
   ///
   /// \return
   ///     The number of matches added to \a variable_list.
   size_t FindGlobalVariables(const RegularExpression &regex, size_t max_matches,
                              VariableList &variable_list);
 
   /// Find types by name.
   ///
-  /// Type lookups in modules go through the SymbolVendor (which will use one
-  /// or more SymbolFile subclasses). The SymbolFile needs to be able to
-  /// lookup types by basename and not the fully qualified typename. This
-  /// allows the type accelerator tables to stay small, even with heavily
+  /// Type lookups in modules go through the SymbolFile. The SymbolFile needs to
+  /// be able to lookup types by basename and not the fully qualified typename.
+  /// This allows the type accelerator tables to stay small, even with heavily
   /// templatized C++. The type search will then narrow down the search
   /// results. If "exact_match" is true, then the type search will only match
   /// exact type name matches. If "exact_match" is false, the type will match
   /// as long as the base typename matches and as long as any immediate
   /// containing namespaces/class scopes that are specified match. So to
   /// search for a type "d" in "b::c", the name "b::c::d" can be specified and
   /// it will match any class/namespace "b" which contains a class/namespace
   /// "c" which contains type "d". We do this to allow users to not always
public:
   /// without going back to the process.
   ///
   /// \return
   ///     The object file loaded from memory or nullptr, if the operation
   ///     failed (see the `error` for more information in that case).
   ObjectFile *GetMemoryObjectFile(const lldb::ProcessSP &process_sp,
                                   lldb::addr_t header_addr, Status &error,
                                   size_t size_to_read = 512);
-  /// Get the symbol vendor interface for the current architecture.
-  ///
-  /// If the symbol vendor file has not been located yet, this function will
-  /// find the best SymbolVendor plug-in that can use the current object file.
-  ///
-  /// \return
-  ///     If this module does not have a valid object file, or no
-  ///     plug-in can be found that can use the object file, nullptr will
-  ///     be returned, else a valid symbol vendor plug-in interface
-  ///     will be returned. The returned pointer is owned by this
-  ///     object and remains valid as long as the object is around.
-  virtual SymbolVendor *
-  GetSymbolVendor(bool can_create = true,
-                  lldb_private::Stream *feedback_strm = nullptr);
 
-  SymbolFile *GetSymbolFile(bool can_create = true,
+  /// Get the module's symbol file
+  ///
+  /// If the symbol file has already been loaded, this function returns it. All
+  /// arguments are ignored. If the symbol file has not been located yet, and
+  /// the can_create argument is false, the function returns nullptr. If
+  /// can_create is true, this function will find the best SymbolFile plug-in
+  /// that can use the current object file. feedback_strm, if not null, is used
+  /// to report the details of the search process.
+  virtual SymbolFile *GetSymbolFile(bool can_create = true,
                                     Stream *feedback_strm = nullptr);
 
   Symtab *GetSymtab();
 
   /// Get a reference to the UUID value contained in this object.
   ///
   /// If the executable image file doesn't not have a UUID value built into
   /// the file format, an MD5 checksum of the entire file, or slice of the
   /// file for the current architecture should be used.
public:
   void ReportErrorIfModifyDetected(const char *format, ...)
       __attribute__((format(printf, 2, 3)));
 
   // Return true if the file backing this module has changed since the module
   // was originally created  since we saved the initial file modification time
   // when the module first gets created.
   bool FileHasChanged() const;
 
-  // SymbolVendor, SymbolFile and ObjectFile member objects should lock the
+  // SymbolFile and ObjectFile member objects should lock the
   // module mutex to avoid deadlocks.
   std::recursive_mutex &GetMutex() const { return m_mutex; }
 
   PathMappingList &GetSourceMappingList() { return m_source_mappings; }
 
   const PathMappingList &GetSourceMappingList() const {
     return m_source_mappings;
   }
public:
   ///     is filled in with a new source path, \b false otherwise.
   bool RemapSourceFile(llvm::StringRef path, std::string &new_path) const;
   bool RemapSourceFile(const char *, std::string &) const = delete;
 
   /// \class LookupInfo Module.h "lldb/Core/Module.h"
   /// A class that encapsulates name lookup information.
   ///
   /// Users can type a wide variety of partial names when setting breakpoints
-  /// by name or when looking for functions by name. SymbolVendor and
-  /// SymbolFile objects are only required to implement name lookup for
-  /// function basenames and for fully mangled names. This means if the user
-  /// types in a partial name, we must reduce this to a name lookup that will
-  /// work with all SymbolFile objects. So we might reduce a name lookup to
-  /// look for a basename, and then prune out any results that don't match.
+  /// by name or when looking for functions by name. The SymbolFile object is
+  /// only required to implement name lookup for function basenames and for
+  /// fully mangled names. This means if the user types in a partial name, we
+  /// must reduce this to a name lookup that will work with all SymbolFile
+  /// objects. So we might reduce a name lookup to look for a basename, and then
+  /// prune out any results that don't match.
   ///
   /// The "m_name" member variable represents the name as it was typed by the
   /// user. "m_lookup_name" will be the name we actually search for through
   /// the symbol or objects files. Lanaguage is included in case we need to
   /// filter results by language at a later date. The "m_name_type_mask"
   /// member variable tells us what kinds of names we are looking for and can
   /// help us prune out unwanted results.
   ///
PathMappingList m_source_mappings; ///< Module specific source remappings for
                                      ///when you have debug info for a module
                                      ///that doesn't match where the sources
                                      ///currently are
   lldb::SectionListUP m_sections_up; ///< Unified section list for module that
                                      /// is used by the ObjectFile and and
                                      /// ObjectFile instances for the debug info
 
   std::atomic<bool> m_did_load_objfile{false};
-  std::atomic<bool> m_did_load_symbol_vendor{false};
+  std::atomic<bool> m_did_load_symfile{false};
   std::atomic<bool> m_did_set_uuid{false};
   mutable bool m_file_has_changed : 1,
       m_first_file_changed_log : 1; /// See if the module was modified after it
                                     /// was initially opened.
 
   /// Resolve a file or load virtual address.
   ///
   /// Tries to resolve \a vm_addr as a file address (if \a

lldb/trunk/source/Core/Module.cpp

size_t Module::FindTypes(
   if (num_matches > 0) {
     SymbolContext sc;
     sc.module_sp = shared_from_this();
     sc.SortTypeList(typesmap, types);
   }
   return num_matches;
 }
 
-SymbolVendor *Module::GetSymbolVendor(bool can_create,
-                                      lldb_private::Stream *feedback_strm) {
-  if (!m_did_load_symbol_vendor.load()) {
+SymbolFile *Module::GetSymbolFile(bool can_create, Stream *feedback_strm) {
+  if (!m_did_load_symfile.load()) {
     std::lock_guard<std::recursive_mutex> guard(m_mutex);
-    if (!m_did_load_symbol_vendor.load() && can_create) {
+    if (!m_did_load_symfile.load() && can_create) {
       ObjectFile *obj_file = GetObjectFile();
       if (obj_file != nullptr) {
         static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
         Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
         m_symfile_up.reset(
             SymbolVendor::FindPlugin(shared_from_this(), feedback_strm));
-        m_did_load_symbol_vendor = true;
-      }
+        m_did_load_symfile = true;
       }
     }
-  return m_symfile_up.get();
   }
-
-SymbolFile *Module::GetSymbolFile(bool can_create, Stream *feedback_strm) {
-  if (SymbolVendor *vendor = GetSymbolVendor(can_create, feedback_strm))
-    return vendor->GetSymbolFile();
-  return nullptr;
+  return m_symfile_up ? m_symfile_up->GetSymbolFile() : nullptr;
 }
 
 Symtab *Module::GetSymtab() {
   if (SymbolFile *symbols = GetSymbolFile())
     return symbols->GetSymtab();
   return nullptr;
 }
 
if (section_list && symbol_file) {
       }
     }
     // Keep all old symbol files around in case there are any lingering type
     // references in any SBValue objects that might have been handed out.
     m_old_symfiles.push_back(std::move(m_symfile_up));
   }
   m_symfile_spec = file;
   m_symfile_up.reset();
-  m_did_load_symbol_vendor = false;
+  m_did_load_symfile = false;
 }
 
 bool Module::IsExecutable() {
   if (GetObjectFile() == nullptr)
     return false;
   else
     return GetObjectFile()->IsExecutable();
 }

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

 
 #include <memory>
 
 using namespace lldb;
 using namespace lldb_private;
 
 // Subclass lldb_private::Module so we can intercept the
 // "Module::GetObjectFile()" (so we can fixup the object file sections) and
-// also for "Module::GetSymbolVendor()" (so we can fixup the symbol file id.
+// also for "Module::GetSymbolFile()" (so we can fixup the symbol file id.
 
 const SymbolFileDWARFDebugMap::FileRangeMap &
 SymbolFileDWARFDebugMap::CompileUnitInfo::GetFileRangeMap(
     SymbolFileDWARFDebugMap *exe_symfile) {
   if (file_range_map_valid)
     return file_range_map;
 
   file_range_map_valid = true;
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 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;
 
-  SymbolVendor *
-  GetSymbolVendor(bool can_create = true,
-                  lldb_private::Stream *feedback_strm = nullptr) override {
+  SymbolFile *
+  GetSymbolFile(bool can_create = true,
     // Scope for locker
     if (m_symfile_up.get() || !can_create)
-      return m_symfile_up.get();
+      return m_symfile_up ? m_symfile_up->GetSymbolFile() : nullptr;
 
     ModuleSP exe_module_sp(m_exe_module_wp.lock());
     if (exe_module_sp) {
       // Now get the object file outside of a locking scope
       ObjectFile *oso_objfile = GetObjectFile();
       if (oso_objfile) {
         std::lock_guard<std::recursive_mutex> guard(m_mutex);
-        SymbolVendor *symbol_vendor =
-            Module::GetSymbolVendor(can_create, feedback_strm);
-        if (symbol_vendor) {
+        if (SymbolFile *symfile =
+                Module::GetSymbolFile(can_create, feedback_strm)) {
           // Set a pointer to this class to set our OSO DWARF file know that
           // the DWARF is being used along with a debug map and that it will
           // have the remapped sections that we do below.
           SymbolFileDWARF *oso_symfile =
-              SymbolFileDWARFDebugMap::GetSymbolFileAsSymbolFileDWARF(
-                  symbol_vendor->GetSymbolFile());
+              SymbolFileDWARFDebugMap::GetSymbolFileAsSymbolFileDWARF(symfile);
 
           if (!oso_symfile)
             return nullptr;
 
           ObjectFile *exe_objfile = exe_module_sp->GetObjectFile();
-          SymbolVendor *exe_sym_vendor = exe_module_sp->GetSymbolVendor();
+          SymbolFile *exe_symfile = exe_module_sp->GetSymbolFile();
 
-          if (exe_objfile && exe_sym_vendor) {
+          if (exe_objfile && exe_symfile) {
             oso_symfile->SetDebugMapModule(exe_module_sp);
             // Set the ID of the symbol file DWARF to the index of the OSO
             // shifted left by 32 bits to provide a unique prefix for any
             // UserID's that get created in the symbol file.
             oso_symfile->SetID(((uint64_t)m_cu_idx + 1ull) << 32ull);
           }
-          return symbol_vendor;
+          return symfile;
         }
       }
     }
     return nullptr;
   }
 
 protected:
   ModuleWP m_exe_module_wp;
SymbolFileDWARFDebugMap::GetSymbolFileAsSymbolFileDWARF(SymbolFile *sym_file) {
   if (sym_file &&
       sym_file->GetPluginName() == SymbolFileDWARF::GetPluginNameStatic())
     return (SymbolFileDWARF *)sym_file;
   return nullptr;
 }
 
 SymbolFileDWARF *SymbolFileDWARFDebugMap::GetSymbolFileByCompUnitInfo(
     CompileUnitInfo *comp_unit_info) {
-  Module *oso_module = GetModuleByCompUnitInfo(comp_unit_info);
-  if (oso_module) {
-    SymbolVendor *sym_vendor = oso_module->GetSymbolVendor();
-    if (sym_vendor)
-      return GetSymbolFileAsSymbolFileDWARF(sym_vendor->GetSymbolFile());
-  }
+  if (Module *oso_module = GetModuleByCompUnitInfo(comp_unit_info))
+    return GetSymbolFileAsSymbolFileDWARF(oso_module->GetSymbolFile());
   return nullptr;
 }
 
 uint32_t SymbolFileDWARFDebugMap::CalculateAbilities() {
   // In order to get the abilities of this plug-in, we look at the list of
   // N_OSO entries (object files) from the symbol table and make sure that
   // these files exist and also contain valid DWARF. If we get any of that then
   // we return the abilities of the first N_OSO's DWARF.