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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
