fdeazeve accepted this revision.
fdeazeve added a comment.

I'm not exactly familiar with DWOs, but the code motions LGTM!



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:882
+  // we have a valid one to set it to.
+  assert(index_entry);
+  assert(!m_index_entry);
----------------
If LLVM's function is like this as well, we shouldn't do anything now and just 
refactor it later once we make the switch. But a pointer parameter in a 
function that asserts not null immediately is more expressive as a reference. 
Note that, in the code where this is called, we have a:

```
if (ptr)
  ApplyIndex(ptr)
```

So the assert is not needed, as the callee respects the contract and can do a 
`*ptr`



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151919

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits]... Alex Langford via Phabricator via lldb-commits
    • [Lldb-com... Adrian Prantl via Phabricator via lldb-commits
    • [Lldb-com... Alex Langford via Phabricator via lldb-commits
    • [Lldb-com... Felipe de Azevedo Piovezan via Phabricator via lldb-commits
    • [Lldb-com... Alex Langford via Phabricator via lldb-commits
    • [Lldb-com... Alex Langford via Phabricator via lldb-commits

Reply via email to