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? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62894/new/ https://reviews.llvm.org/D62894 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits