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

Jonas Devlieghere via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Jun 10 08:59:03 PDT 2019

JDevlieghere added inline comments.

Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:148
   dw_addr_t GetStrOffsetsBase() const { return m_str_offsets_base; }
+  dw_offset_t GetLineTableOffset();
   void SetAddrBase(dw_addr_t addr_base);
Nit: move this next to `GetAbbrevOffset`

Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:200
   const lldb_private::FileSpec &GetCompilationDirectory();
+  const lldb_private::FileSpec &GetFileSpec();
   lldb_private::FileSpec::Style GetPathStyle();
Given the implementation, how about `GetAbsolutePath`? I think the "FileSpec" of a DWARF unit is a little weird, but feel free to ignore this. 

Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:290
   dw_offset_t m_str_offsets_base = 0; // Value of DW_AT_str_offsets_base.
+  dw_offset_t m_line_table_offset =
+      DW_INVALID_OFFSET; // Value of DW_AT_stmt_list.
Let's make this a Doxygen comment with `///<` or even better just `///` on the line above it so it doesn't wrap. 

Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:821
+  if (iter_bool.second) {
+    list.Append(FileSpec());
+    DWARFDebugLine::ParseSupportFiles(GetObjectFile()->GetModule(),
Not your responsibility but we should really get rid of this thing as it doesn't make sense anymore in > DWARF5.

Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:303
+  lldb_private::FileSpec GetSupportFile(DWARFUnit &unit, size_t file_idx);
clayborg wrote:
> Can probably just be named GetFile
Should this return an optional?



More information about the lldb-commits mailing list