[PATCH] D50197: [DebugInfo/DWARF] Try to make a loop more readable. NFC

[Paul Robinson via Phabricator via llvm-commits]

probinson added a subscriber: dblaikie.
probinson added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:103
+      if (&(*InsertPt)->getInfoSection() != &Section ||
+          (*InsertPt)->getOffset() == Offset) {
+        ++InsertPt;
----------------
aprantl wrote:
> Is there a comment that could explain which case is being checked for here?
Well, there's the comment above the initialization of InsertPt.
Although... huh.  What is the Offset check actually doing?

I don't think the original loop (pre-r338628) was doing what I thought it was doing. 

Prior to r313659, the loop was generating a unique_ptr for each unit in the section.  In r313659, @dblaikie  wanted to avoid parsing DWO units up front if possible, and have the get-unit-by-offset do them on-demand.  But then here we need to parse all units.  I *think* the intent was to skip units that had already been found lazily, and fill in the rest.

But the r313659 version of the loop doesn't actually do that.  It can successfully skip the offset=0 unit, if it is already present, but then it will generate a second vector entry for the next unit even if it's already present.

I'll have to devise a unittest to prove this, though.  I suspect that in addition to the `++InsertPt` we also need to do a `Offset = getNextUnitOffset` of some kind.


Repository:
  rL LLVM

https://reviews.llvm.org/D50197