zturner created this revision.
zturner added reviewers: clayborg, labath, davide, jingham.
Herald added a subscriber: JDevlieghere.

The function `SymbolFile::ParseTypes` previously accepted a `SymbolContext`.  
This makes it extremely difficult to implement faithfully, because you have to 
account for all possible combinations of members being set in the 
`SymbolContext`.  On the other hand, no clients of this function actually care 
about implementing this function to this strict of a standard.  AFAICT, there 
is actually only 1 client in the entire codebase, and it is the function 
`ParseAllDebugSymbols`, which is itself only called for testing purposes when 
dumping information.  At this call-site, the only field it sets is the 
`CompileUnit`, meaning that an implementer of a `SymbolFile` need not worry 
about any examining or handling any other fields which might be set.

By restricting this API to accept exactly a `CompileUnit&` and nothing more, we 
can simplify the life of new `SymbolFile` plugin implementers by making it 
clear exactly what the necessary and sufficient set of functionality they need 
to implement is, while at the same time removing some dead code that tried to 
handle other types of `SymbolContext` fields that were never going to be set 
anyway.


https://reviews.llvm.org/D56462

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolVendor.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  lldb/source/Symbol/SymbolVendor.cpp

Index: lldb/source/Symbol/SymbolVendor.cpp
===================================================================
--- lldb/source/Symbol/SymbolVendor.cpp
+++ lldb/source/Symbol/SymbolVendor.cpp
@@ -200,12 +200,12 @@
   return 0;
 }
 
-size_t SymbolVendor::ParseTypes(const SymbolContext &sc) {
+size_t SymbolVendor::ParseTypesForCompileUnit(CompileUnit &comp_unit) {
   ModuleSP module_sp(GetModule());
   if (module_sp) {
     std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
     if (m_sym_file_ap.get())
-      return m_sym_file_ap->ParseTypes(sc);
+      return m_sym_file_ap->ParseTypesForCompileUnit(comp_unit);
   }
   return 0;
 }
Index: lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
===================================================================
--- lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
+++ lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
@@ -70,7 +70,8 @@
 
   size_t ParseFunctionBlocks(const lldb_private::SymbolContext &sc) override;
 
-  size_t ParseTypes(const lldb_private::SymbolContext &sc) override;
+  size_t
+  ParseTypesForCompileUnit(lldb_private::CompileUnit &comp_unit) override;
 
   size_t
   ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
Index: lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
+++ lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
@@ -229,7 +229,9 @@
   return 0;
 }
 
-size_t SymbolFileSymtab::ParseTypes(const SymbolContext &sc) { return 0; }
+size_t SymbolFileSymtab::ParseTypesForCompileUnit(CompileUnit &comp_unit) {
+  return 0;
+}
 
 size_t SymbolFileSymtab::ParseVariablesForContext(const SymbolContext &sc) {
   return 0;
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===================================================================
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -81,7 +81,8 @@
 
   size_t ParseFunctionBlocks(const lldb_private::SymbolContext &sc) override;
 
-  size_t ParseTypes(const lldb_private::SymbolContext &sc) override;
+  size_t
+  ParseTypesForCompileUnit(lldb_private::CompileUnit &comp_unit) override;
 
   size_t
   ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -464,13 +464,10 @@
   return num_added;
 }
 
-size_t SymbolFilePDB::ParseTypes(const lldb_private::SymbolContext &sc) {
-  lldbassert(sc.module_sp.get());
-  if (!sc.comp_unit)
-    return 0;
+size_t SymbolFilePDB::ParseTypesForCompileUnit(CompileUnit &comp_unit) {
 
   size_t num_added = 0;
-  auto compiland = GetPDBCompilandByUID(sc.comp_unit->GetID());
+  auto compiland = GetPDBCompilandByUID(comp_unit.GetID());
   if (!compiland)
     return 0;
 
@@ -505,23 +502,15 @@
     }
   };
 
-  if (sc.function) {
-    auto pdb_func = m_session_up->getConcreteSymbolById<PDBSymbolFunc>(
-        sc.function->GetID());
-    if (!pdb_func)
-      return 0;
-    ParseTypesByTagFn(*pdb_func);
-  } else {
-    ParseTypesByTagFn(*compiland);
-
-    // Also parse global types particularly coming from this compiland.
-    // Unfortunately, PDB has no compiland information for each global type. We
-    // have to parse them all. But ensure we only do this once.
-    static bool parse_all_global_types = false;
-    if (!parse_all_global_types) {
-      ParseTypesByTagFn(*m_global_scope_up);
-      parse_all_global_types = true;
-    }
+  ParseTypesByTagFn(*compiland);
+
+  // Also parse global types particularly coming from this compiland.
+  // Unfortunately, PDB has no compiland information for each global type. We
+  // have to parse them all. But ensure we only do this once.
+  static bool parse_all_global_types = false;
+  if (!parse_all_global_types) {
+    ParseTypesByTagFn(*m_global_scope_up);
+    parse_all_global_types = true;
   }
   return num_added;
 }
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
===================================================================
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
@@ -104,7 +104,8 @@
                                uint32_t max_matches,
                                VariableList &variables) override;
 
-  size_t ParseTypes(const SymbolContext &sc) override;
+  size_t
+  ParseTypesForCompileUnit(lldb_private::CompileUnit &comp_unit) override;
   size_t ParseVariablesForContext(const SymbolContext &sc) override;
 
   void AddSymbols(Symtab &symtab) override;
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1274,7 +1274,7 @@
   return match_count;
 }
 
-size_t SymbolFileNativePDB::ParseTypes(const SymbolContext &sc) {
+size_t SymbolFileNativePDB::ParseTypesForCompileUnit(CompileUnit &comp_unit) {
   // Only do the full type scan the first time.
   if (m_done_full_type_scan)
     return 0;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -72,7 +72,8 @@
       const lldb_private::SymbolContext &sc,
       std::vector<lldb_private::ConstString> &imported_modules) override;
   size_t ParseFunctionBlocks(const lldb_private::SymbolContext &sc) override;
-  size_t ParseTypes(const lldb_private::SymbolContext &sc) override;
+  size_t
+  ParseTypesForCompileUnit(lldb_private::CompileUnit &comp_unit) override;
   size_t
   ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -678,10 +678,13 @@
   return 0;
 }
 
-size_t SymbolFileDWARFDebugMap::ParseTypes(const SymbolContext &sc) {
+size_t
+SymbolFileDWARFDebugMap::ParseTypesForCompileUnit(CompileUnit &comp_unit) {
+  SymbolContext sc;
+  sc.comp_unit = &comp_unit;
   SymbolFileDWARF *oso_dwarf = GetSymbolFile(sc);
   if (oso_dwarf)
-    return oso_dwarf->ParseTypes(sc);
+    return oso_dwarf->ParseTypesForCompileUnit(comp_unit);
   return 0;
 }
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -131,7 +131,8 @@
 
   size_t ParseFunctionBlocks(const lldb_private::SymbolContext &sc) override;
 
-  size_t ParseTypes(const lldb_private::SymbolContext &sc) override;
+  size_t
+  ParseTypesForCompileUnit(lldb_private::CompileUnit &comp_unit) override;
 
   size_t
   ParseVariablesForContext(const lldb_private::SymbolContext &sc) override;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3149,24 +3149,16 @@
   return functions_added;
 }
 
-size_t SymbolFileDWARF::ParseTypes(const SymbolContext &sc) {
+size_t SymbolFileDWARF::ParseTypesForCompileUnit(CompileUnit &comp_unit) {
   ASSERT_MODULE_LOCK(this);
-  // At least a compile unit must be valid
-  assert(sc.comp_unit);
   size_t types_added = 0;
-  DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
+  DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
   if (dwarf_cu) {
-    if (sc.function) {
-      dw_offset_t function_die_offset = sc.function->GetID();
-      DWARFDIE func_die = dwarf_cu->GetDIE(function_die_offset);
-      if (func_die && func_die.HasChildren()) {
-        types_added = ParseTypes(sc, func_die.GetFirstChild(), true, true);
-      }
-    } else {
-      DWARFDIE dwarf_cu_die = dwarf_cu->DIE();
-      if (dwarf_cu_die && dwarf_cu_die.HasChildren()) {
-        types_added = ParseTypes(sc, dwarf_cu_die.GetFirstChild(), true, true);
-      }
+    DWARFDIE dwarf_cu_die = dwarf_cu->DIE();
+    if (dwarf_cu_die && dwarf_cu_die.HasChildren()) {
+      SymbolContext sc;
+      sc.comp_unit = &comp_unit;
+      types_added = ParseTypes(sc, dwarf_cu_die.GetFirstChild(), true, true);
     }
   }
 
Index: lldb/source/Core/Module.cpp
===================================================================
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -380,8 +380,7 @@
       });
 
       // Parse all types for this compile unit
-      sc.function = nullptr;
-      symbols->ParseTypes(sc);
+      symbols->ParseTypesForCompileUnit(*sc.comp_unit);
     }
   }
 }
Index: lldb/include/lldb/Symbol/SymbolVendor.h
===================================================================
--- lldb/include/lldb/Symbol/SymbolVendor.h
+++ lldb/include/lldb/Symbol/SymbolVendor.h
@@ -64,7 +64,7 @@
 
   virtual size_t ParseFunctionBlocks(const SymbolContext &sc);
 
-  virtual size_t ParseTypes(const SymbolContext &sc);
+  virtual size_t ParseTypesForCompileUnit(CompileUnit &comp_unit);
 
   virtual size_t ParseVariablesForContext(const SymbolContext &sc);
 
Index: lldb/include/lldb/Symbol/SymbolFile.h
===================================================================
--- lldb/include/lldb/Symbol/SymbolFile.h
+++ lldb/include/lldb/Symbol/SymbolFile.h
@@ -140,7 +140,7 @@
   ParseImportedModules(const SymbolContext &sc,
                        std::vector<ConstString> &imported_modules) = 0;
   virtual size_t ParseFunctionBlocks(const SymbolContext &sc) = 0;
-  virtual size_t ParseTypes(const SymbolContext &sc) = 0;
+  virtual size_t ParseTypesForCompileUnit(CompileUnit &comp_unit) = 0;
   virtual size_t ParseVariablesForContext(const SymbolContext &sc) = 0;
   virtual Type *ResolveTypeUID(lldb::user_id_t type_uid) = 0;
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to