aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land.
Overall seems good to me. Minor comments inline. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp:27 lldb::offset_t *offset_ptr) { + llvm::DataExtractor llvm_data = data.GetAsLLVM(); const lldb::offset_t begin_offset = *offset_ptr; ---------------- Is this intentionally a copy or should this be a reference? I have no idea how heavyweight this object is... ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp:56 if (m_idx_offset == UINT32_MAX) { - DWARFAbbreviationDeclarationCollConstIter pos; - DWARFAbbreviationDeclarationCollConstIter end = m_decls.end(); - for (pos = m_decls.begin(); pos != end; ++pos) { - if (pos->Code() == abbrCode) - return &(*pos); + for (const auto &decl : m_decls) { + if (decl.getCode() == abbrCode) ---------------- would std::find_if be shorter or would it look worse? ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h:18 +#include "llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h" + ---------------- Nit: we usually do local includes first and then the stdlib at the bottom. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150716/new/ https://reviews.llvm.org/D150716 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits