labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Looks good, just please write more explanatory commit message than what's 
present in this patch description.



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp:75-80
+  DWARFDIE die = m_dwarf.GetDIE(ref);
+  if (!die) {
+    m_index.ReportInvalidDIERef(ref, m_name);
+    return true;
+  }
+  return m_callback(die);
----------------
reverse the if condition
```
if (DWARFDIE die = m_dwarf.GETDIE(ref))
  return m_callback(die);
...
```


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h:93
+
+  void ReportInvalidDIERef(DIERef ref, llvm::StringRef name) const;
 };
----------------
jankratochvil wrote:
> Invalid `DIERef` is now reported for all Indexes, not just `AppleDWARFIndex`.
> 
That's fine. The error message that produces is really geared towards 
accelerator tables, but the manual index should not produce such an error 
anyway (I would have put an assert there it was convenient (but it isn't)). And 
I think I'm going to change the debug_names index to not use DIERefs once this 
lands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77970



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to