[Lldb-commits] [PATCH] D65952: SymbolVendor: Have plugins return symbol files directly

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

So I am confused. Are we keeping SymbolVendor around for locating symbols files or are we getting rid of it entirely?



================
Comment at: include/lldb/Core/Module.h:989-996
+  /// A pointer to the symbol file for this module.
+  std::unique_ptr<SymbolFile> m_symfile_up;
+
+  /// If anyone calls Module::SetSymbolFileFileSpec() and changes the symbol
+  /// file, we need to keep all old symbol files around in case anyone has type
+  /// references to them
+  std::vector<std::unique_ptr<SymbolFile>> m_old_symfiles;
----------------
Maybe we _just_ have "m_symfiles" here and document that the last entry is always the current symbol file? No need for two  ivars here?


================
Comment at: include/lldb/Symbol/SymbolVendor.h:12
 
-#include <vector>
-
-#include "lldb/Core/ModuleChild.h"
-#include "lldb/Core/PluginInterface.h"
-#include "lldb/Symbol/SourceModule.h"
-#include "lldb/Symbol/TypeMap.h"
-#include "lldb/lldb-private.h"
-#include "llvm/ADT/DenseSet.h"
+#include "lldb/lldb-forward.h"
+#include <memory>
----------------
Just delete this entire file?


================
Comment at: source/Core/Module.cpp:1049
+        m_symfile_up =
+            SymbolVendor::FindPlugin(shared_from_this(), feedback_strm);
         m_did_load_symfile = true;
----------------
rename to SymbolFile::FindPlugin?


================
Comment at: source/Core/PluginManager.cpp:1743
 
 #pragma mark SymbolVendor
 
----------------
Rename to SymbolFile?


================
Comment at: source/Core/PluginManager.cpp:1745
 
-struct SymbolVendorInstance {
-  SymbolVendorInstance() : name(), description(), create_callback(nullptr) {}
-
-  ConstString name;
-  std::string description;
-  SymbolVendorCreateInstance create_callback;
-};
-
-typedef std::vector<SymbolVendorInstance> SymbolVendorInstances;
+typedef std::vector<SymbolVendorCreateInstance> SymbolVendorInstances;
 
----------------
ditto


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

https://reviews.llvm.org/D65952