[PATCH] D69672: DWARFDebugLoclists: Move to a incremental parsing model

[David Blaikie via Phabricator via llvm-commits]

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

Looks good!



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp:210-219
+  if (DumpOpts.Verbose) {
+    uint64_t OffsetCopy = *Offset;
+    consumeError(
+        visitLocationList(Data, &OffsetCopy, Version, [&](const Entry &E) {
+          MaxEncodingStringLength =
+              std::max(MaxEncodingStringLength,
+                       dwarf::LocListEncodingString(E.Kind).size());
----------------
labath wrote:
> Parsing twice just to get the maximum name length is somewhat wasteful. A different option would be to get the maximum of *all* entry types, not just of those that happen to be used in this location lists.
Yeah, I'd be happy if you switch that to max of all entry types.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp:318
+  bool CanContinue = true;
+  while (CanContinue && Data.isValidOffset(Offset) && Offset < EndOffset) {
+    OS << Separator;
----------------
isValidOffset might be unnecessary given "Offset < EndOffset"? (maybe check that EndOffset is smaller than Data's actual end ofset to start with, then skip the isValidOffset check? (maybe need a lower bound check too))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69672