[PATCH] D80962: [NativeSession] Implement findLineNumbersByAddress functions for native llvm-symbolizer support.

[Pavel Labath via Phabricator via llvm-commits]

labath added inline comments.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/SymbolCache.cpp:499
+      });
+  --LineIter;
+
----------------
akhuang wrote:
> labath wrote:
> > This assumes that the table contains at least one element, but I'm not sure if that is always the case. To an unfamiliar reader it does seem like `populateLineTable` can legitimately return without inserting anything into the table.
> > 
> > I also get a distinct feeling that this iterator manipulation could be simplified, but I'm not sure how because I'm not exactly sure what is this code doing.
> My bad, I meant to put another non-empty check after calling `populateLineTable`. 
> 
> Maybe it can be simplified; it's searching for the first address in the line table that is not greater than `VA`. 
Maybe something like:
```
// This will find the first non-terminal entry whose address is >= VA
auto LineIter = llvm::partition_point(Lines, [&] (const auto &E) { return E.Addr < VA || (E.Addr == VA && E.IsTerminalEntry); });
if (LineIter == Lines.end() || LineIter->Addr > VA) {
  // We've gone too far. Try to back up.
  if (LineIter == Lines.begin() || std::prev(LineIter)->IsTerminalEntry) {
    // Can't back up. It's not clear to me if the existing version handles the case where the `VA` would be in a "hole" between a previous terminal entry ending before VA and the next entry which starts after VA.
    return nullptr;
  }
  --LineIter;
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80962