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

[probinson]

Summary

In a previous refactoring patch, Adrian didn't find this loop particularly readable, although admitted it was like that when I found it. :-)
I've tried renaming a variable and breaking up a condition, hopefully this will be clearer. I am very open to other ideas.

[aprantl]

Thanks!

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

103

Is there a comment that could explain which case is being checked for here?

[probinson]

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

103

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.

[probinson]

I'm abandoning this minor tweak as I am convinced that there is something more fundamentally wrong here.
llvm-dwarfdump doesn't call the right API sequence to show the bug; it requires a DWP, looking up a specific unit by hash via the DWP index, and then scanning all the units. I can envision how to do a unittest for that, but the unittest infrastructure doesn't currently support building a DWP. I can't take the time right now to address this, but I'll keep it on The List.

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

Parser = [=, &Context, &Obj, &Section, &SOS, &LS](
         U = llvm::make_unique<DWARFCompileUnit>(Context, InfoSection, Header,
                                                 DA, RS, SS, SOS, AOS, LS, LE,
                                                 IsDWO, *this);
       return U;
     };
   }
   if (Lazy)
     return;
+
   // Find a reasonable insertion point within the vector.  We skip over
   // (a) units from a different section, (b) units from the same section
   // but with lower offset-within-section.  This keeps units in order
   // within a section, although not necessarily within the object file,
   // even if we do lazy parsing.
-  auto I = this->begin();
+  auto InsertPt = this->begin();
   uint32_t Offset = 0;
   while (Data.isValidOffset(Offset)) {
-    if (I != this->end() &&
-        (&(*I)->getInfoSection() != &Section || (*I)->getOffset() == Offset)) {
-      ++I;
+    if (InsertPt != this->end()) {
+      if (&(*InsertPt)->getInfoSection() != &Section ||
+          (*InsertPt)->getOffset() == Offset) {
+        ++InsertPt;
         continue;
       }
+    }
     auto U = Parser(Offset, SectionKind, &Section);
     // If parsing failed, we're done with this section.
     if (!U)
       break;
     Offset = U->getNextUnitOffset();
-    I = std::next(this->insert(I, std::move(U)));
+    InsertPt = std::next(this->insert(InsertPt, std::move(U)));
   }
 }
 
 DWARFUnit *DWARFUnitVector::getUnitForOffset(uint32_t Offset) const {
   auto *CU = std::upper_bound(
     this->begin(), this->end(), Offset,
     [](uint32_t LHS, const std::unique_ptr<DWARFUnit> &RHS) {
     return LHS < RHS->getNextUnitOffset();