[Lldb-commits] [PATCH] D62395: WIP: Dont vend lldb_private::CompileUnits for DWARFTypeUnits

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:318-319
           case DW_AT_decl_file:
-            decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-                form_value.Unsigned()));
             break;
----------------
We should still be able to fill this in. We just need to unique all of the DWARF line tables internally and then access the line table using the DWARF compile/type unit, and not the lldb_private::CompileUnit


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:470-471
       bool translation_unit_is_objc =
-          (sc.comp_unit->GetLanguage() == eLanguageTypeObjC ||
-           sc.comp_unit->GetLanguage() == eLanguageTypeObjC_plus_plus);
+          (die.GetLanguage() == eLanguageTypeObjC ||
+           die.GetLanguage() == eLanguageTypeObjC_plus_plus);
 
----------------
Cache this in local. Kind of expensive to get twice from the DIE since it will have to grab the DW_AT_language attribute from the DIE and not just use a cached version? Unless this just grabs it from the DWARFUnit underneath? And actually a DIE's language could differ from the CU/TU language. We might have a C type in C++, so it is theoretically possible to have a die with a DW_AT_language attribute? If "die.GetLanguage()" is just using the language of the DWARFUnit, then this is fine, otherwise we should explicitly use the DWARFUnit language.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:567-568
           case DW_AT_decl_file:
-            decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-                form_value.Unsigned()));
             break;
----------------
Again, we can still fill this in correctly, just don't use the lldb_private::CompileUnit, grab the uniqued line table from DWARFContext or SymbolFileDWARF. We need to cache line tables anyway because many type units re-use existing compile unit line tables and we don't want to re-parse line table for each TU.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:667
     if (byte_size && *byte_size == 0 && type_name_cstr && !die.HasChildren() &&
-        sc.comp_unit->GetLanguage() == eLanguageTypeObjC) {
+        die.GetLanguage() == eLanguageTypeObjC) {
       // Work around an issue with clang at the moment where forward
----------------
Again, do we want CU/TU language or DIE language?


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:998-999
           case DW_AT_decl_file:
-            decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-                form_value.Unsigned()));
             break;
----------------
fill in using uniqued DWARF line tables


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1169-1170
           case DW_AT_decl_file:
-            decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-                form_value.Unsigned()));
             break;
----------------
fill in using uniqued DWARF line tables


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1677-1678
           case DW_AT_decl_file:
-            decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-                form_value.Unsigned()));
             break;
----------------
fill in using uniqued DWARF line tables


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2471-2472
             case DW_AT_decl_file:
-              decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-                  form_value.Unsigned()));
               break;
----------------
fill in using uniqued DWARF line tables


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2720-2721
             case DW_AT_decl_file:
-              decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-                  form_value.Unsigned()));
               break;
----------------
fill in using uniqued DWARF line tables


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3188-3189
             case DW_AT_decl_file:
-              decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-                  form_value.Unsigned()));
               break;
----------------
fill in using uniqued DWARF line tables


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:22
+  static bool classof(const DWARFUnit *unit) {
+    return unit->GetUnitType() != DW_UT_type;
+  }
----------------
What about DW_UT_split_type?


================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:715
 
+void SymbolFileDWARF::BuildLldbCuTranslationTable() {
+  if (!m_lldb_cu_to_dwarf_unit.empty())
----------------
BuildCUTranslationTable? Not sure we need the "Lldb" in the name?


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

https://reviews.llvm.org/D62395