[Lldb-commits] [PATCH] D150716: [lldb][NFCI] Switch to using llvm::DWARFAbbreviationDeclaration

[Alex Langford via Phabricator via lldb-commits]

bulbazord marked 2 inline comments as done.
bulbazord added inline comments.


================
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;
----------------
aprantl wrote:
> Is this intentionally a copy or should this be a reference? I have no idea how heavyweight this object is...
This isn't exactly a copy. `DataExtractor::GetAsLLVM` creates an entirely new object, it can't be a reference. Luckily it is fairly lightweight -- We just copy a few numbers and a pointer I believe.


================
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)
----------------
aprantl wrote:
> would std::find_if be shorter or would it look worse?
```
for (const auto &decl : m_decls) {
  if (decl.getCode() == abbrCode)
    return &decl;
}
```
vs.
```
auto pos = std::find_if(
     m_decls.begin(), m_decls.end(),
     [abbrCode](const auto &decl) { return decl.getCode() == abbrCode; });
if (pos != m_decls.end())
      return &*pos;
```

I think it would look worse, personally.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150716/new/

https://reviews.llvm.org/D150716