[Lldb-commits] [PATCH] D62570: [WIP] Use LLVM's debug line parser in LLDB

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/include/lldb/Core/FileSpecList.h:191-192
+  const_iterator end() const { return m_files.end(); }
+  const_iterator begin() { return m_files.begin(); }
+  const_iterator end() { return m_files.end(); }
+
----------------
If both of these are returning the const iterators, then you don't need non-const versions..


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:804
+  support_files.Append(comp_unit);
+  for (const FileSpec &file : m_support_files[&comp_unit])
+    support_files.Append(file);
----------------
What's the reason for this global m_support_files map? Is it just because you cannot set the "support" file list from inside the `ParseLineTable` call, and parsing the line table produces the file list as a "side effect". I've ran into the same problem in SymbolFileBreakpad, and I've had to bend over backwards to ensure we don't store the file list twice, so maybe it's time to have a more general solution to that. I think it would suffice to add `CompileUnit::SetSupportFiles` (to complement `CompileUnit::SetLineTable`). That way, you could call both `comp_unit->SetSupportFiles` and `->SetLineTable` regardless of which one was being asked for.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:832
+#if 0
   // Many type units can share a line table, so parse the support file list
   // once, and cache it based on the offset field.
----------------
I guess here you ought to to something similar to the block of code on line 981. (i.e., parse the line table, fetch files from it, and remap them. The only difference would be that you don't use the compilation directory from the type unit to resolve relative paths (because type units have no DW_AT_comp_dir).


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

https://reviews.llvm.org/D62570