[Lldb-commits] [PATCH] D62894: DWARF: Share line tables of type units

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Still want to resolve getting files from a DWARFUnit a bit better. See inlined comment.



================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:309
+      decl.SetFile(
+          die.GetDWARF()->GetFile(*die.GetCU(), form_value.Unsigned()));
       break;
----------------
At the very least we should be asking the unit for the file? That is why I wanted to be able to ask the DWARFDIE for the file because it contains the right unit. If we just put the API on the unit, then we have the chance someone will use the wrong unit for the file. Also, as the DWARF gets fancier as time goes on (DWO, DWZ, etc), the unit might refer to another unit. But at the very least here I would feel better if we ask the DWARFUnit for the file. The GetDWARF() will ask the DWARFUnit in the DIE for the is DWARF file, then we will call the DWARF file class (SymbolFileDWARF or DWARFContext to get a file from the unit by passing a reference? Seems convoluted. Fine not adding the API as a FileSpec if we are trying to keep the API the same as LLVM.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2261
+              decl.SetFile(
+                  die.GetDWARF()->GetFile(*die.GetCU(), form_value.Unsigned()));
               break;
----------------
DWARFUnit::GetFile() or what ever solution we come up with from my previous long inlined comment.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2418
+        decl_up.reset(
+            new Declaration(die.GetDWARF()->GetFile(*die.GetCU(), decl_file),
+                            decl_line, decl_column));
----------------
DWARFUnit::GetFile() or what ever solution we come up with from my previous long inlined comment.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:289-326
+  DWARFAttributes attributes;
+  size_t num_attributes = cu_die.GetAttributes(this, attributes);
+  for (size_t i = 0; i < num_attributes; ++i) {
+    dw_attr_t attr = attributes.AttributeAtIndex(i);
+    DWARFFormValue form_value;
+    if (!attributes.ExtractFormValueAtIndex(i, form_value))
+      continue;
----------------
much better! 


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

https://reviews.llvm.org/D62894