ikudrin added a comment. The code looks good, but please improve the comments and wait for approval from a more LLDB-knowledgeable person than me,
================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:441 + if (offset == 0) { + // Caller must not use this default initializater for GetRnglistOffset. + return ListTableType(); ---------------- Please extend the comment to emphasize that even if `DW_AT_rnglists_base` is missing and `DW_FORM_rnglistx` cannot be handled, returning a default-constructed Table allows `DW_FORM_sec_offset` to be supported. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:1005 + + // As \a offset can be zero we need to call setAddressSize. + data.setAddressSize(m_header.GetAddressByteSize()); ---------------- This comment is quite misleading as it references `offset` which is the argument of the method. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106466/new/ https://reviews.llvm.org/D106466 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits