[Lldb-commits] [PATCH] D151919: [lldb][NFCI] Apply IndexEntry to DWARFUnitHeader outside of extraction

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.


================
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);
----------------
fdeazeve wrote:
> 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`
> 
Yes, this was my thought process as well. I wrote it this way because LLVM's function is just like this. I'm also touching that equivalent function in LLVM in a different way (adding error handling, see https://reviews.llvm.org/D151933). Let's rework this in a follow-up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151919