[Lldb-commits] [PATCH] D61498: 01/06: Merge GetCompileUnit + GetCompileUnitContainingDIEOffset

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

What you could do instead is have a single function which returns the CU if it is in the range `cu.GetOffset()  <= input < cu.GetNextUnitOffset()`, then the wrapper functions could just perform a more stricter check on the returned cu (I.e. no templates or callbacks).

This may lower the need for this refactoring, but independently of that, I don't believe we should touch this function without simplifying it into something more like

  uint32_t index = DW_INVALID_INDEX;
  DWARFUnit result = nullptr;
  auto pos = llvm::lower_bound(m_compile_units, offset, OffsetLessThanCompileUnitOffset);
  if (pos != m_compile_units.end() && ???) {
    index = std::distance(...);
    result = ...
  }
  if (idx_ptr) *idx_ptr = index;
  return result;

as the current implementation is hard to understand for no good reason.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D61498