Move DWARFUnitSection from the vector API to something more semantically accurate.

[friss]

Summary

This just introduces a bunch of wrapper methods that make more sense than push_back or operator[]. NFC.

[dblaikie]

Alexey - I'm assuming this wasn't quite what you had in mind?

I'm not sure of the merits of having a non-container like API if we're exposing this as a range of things, etc.

But we could reduce code duplication by rolling some operations into this type (things like parsing and dumping - which we unnecessarily duplicate for DWO and non-DWO cases) then we wouldn't have to expose such a broad interface (eg: we could probably avoid exposing addUnit).

Personally I'd rather the thing be a range (by having begin/end), if that's what it is, rather than having a "units()" range accessor, but I don't hold strongly to the notion.

"getAddressByteSize" could probably be a first-class operation on a DWARFUnitSection so clients don't have to do the "is !empty then get the address byte size of the first unit"

But this is more Alexey's code than it is mine, in any case.

lib/DebugInfo/DWARFUnit.h

77

Why the null pointer check?

I'd probably just have this return by reference, personally.

80

Pass unique_ptrs (& other movable types) by value when passing ownership.

[friss]

The Units method was suggested by Alexey I have no strong feeling either way. With this patch applied, the inheritance of SmallVector isn't necessary anymore, I didn't fold the move from inheritance to composition in this patch, but it could be done.

The next step for me is to factor the the parsing code into this class. There is an intermediate step though: the actual code is semantically wrong when it comes to type units. Type units might come from different sections, but we put them all in the same DWARFUnitSection object. Once this is fixed, we can move parsing into DWARFUnitSection and remove at least the addUnit method.

Once this is done a bunch of accessors from the context can go away also.

lib/DebugInfo/DWARFContext.h

public:
   void dump(raw_ostream &OS, DIDumpType DumpType = DIDT_All) override;
 
   typedef DWARFUnitSection<DWARFCompileUnit>::iterator_range cu_iterator_range;
   typedef DWARFUnitSection<DWARFTypeUnit>::iterator_range tu_iterator_range;
 
   /// Get compile units in this context.
   cu_iterator_range compile_units() {
     parseCompileUnits();
-    return cu_iterator_range(CUs.begin(), CUs.end());
+    return CUs.Units();
   }
 
   /// Get type units in this context.
   tu_iterator_range type_units() {
     parseTypeUnits();
-    return tu_iterator_range(TUs.begin(), TUs.end());
+    return TUs.Units();
   }
 
   /// Get compile units in the DWO context.
   cu_iterator_range dwo_compile_units() {
     parseDWOCompileUnits();
-    return cu_iterator_range(DWOCUs.begin(), DWOCUs.end());
+    return DWOCUs.Units();
   }
 
   /// Get type units in the DWO context.
   tu_iterator_range dwo_type_units() {
     parseDWOTypeUnits();
-    return tu_iterator_range(DWOTUs.begin(), DWOTUs.end());
+    return DWOTUs.Units();
   }
 
   /// Get the number of compile units in this context.
   unsigned getNumCompileUnits() {
     parseCompileUnits();
-    return CUs.size();
+    return CUs.getNumUnits();
   }
 
   /// Get the number of compile units in this context.
   unsigned getNumTypeUnits() {
     parseTypeUnits();
-    return TUs.size();
+    return TUs.getNumUnits();
   }
 
   /// Get the number of compile units in the DWO context.
   unsigned getNumDWOCompileUnits() {
     parseDWOCompileUnits();
-    return DWOCUs.size();
+    return DWOCUs.getNumUnits();
   }
 
   /// Get the number of compile units in the DWO context.
   unsigned getNumDWOTypeUnits() {
     parseDWOTypeUnits();
-    return DWOTUs.size();
+    return DWOTUs.getNumUnits();
   }
 
   /// Get the compile unit at the specified index for this compile unit.
   DWARFCompileUnit *getCompileUnitAtIndex(unsigned index) {
     parseCompileUnits();
-    return CUs[index].get();
+    return CUs.getUnitAtIndex(index);
   }
 
   /// Get the compile unit at the specified index for the DWO compile units.
   DWARFCompileUnit *getDWOCompileUnitAtIndex(unsigned index) {
     parseDWOCompileUnits();
-    return DWOCUs[index].get();
+    return DWOCUs.getUnitAtIndex(index);
   }
 
   /// Get a pointer to the parsed DebugAbbrev object.
   const DWARFDebugAbbrev *getDebugAbbrev();
 
   /// Get a pointer to the parsed DebugLoc object.
   const DWARFDebugLoc *getDebugLoc();
 

lib/DebugInfo/DWARFContext.cpp

 const DWARFDebugLoc *DWARFContext::getDebugLoc() {
   if (Loc)
     return Loc.get();
 
   DataExtractor LocData(getLocSection().Data, isLittleEndian(), 0);
   Loc.reset(new DWARFDebugLoc(getLocSection().Relocs));
   // assume all compile units have the same address byte size
   if (getNumCompileUnits())
-    Loc->parse(LocData, getCompileUnitAtIndex(0)->getAddressByteSize());
+    Loc->parse(LocData, CUs.getUnitAtIndex(0)->getAddressByteSize());
   return Loc.get();
 }
 
 const DWARFDebugLocDWO *DWARFContext::getDebugLocDWO() {
   if (LocDWO)
     return LocDWO.get();
 
   DataExtractor LocData(getLocDWOSection().Data, isLittleEndian(), 0);
DWARFContext::getLineTableForUnit(DWARFUnit *cu) {
 
   // We have to parse it first.
   DataExtractor lineData(getLineSection().Data, isLittleEndian(),
                          cu->getAddressByteSize());
   return Line->getOrParseLineTable(lineData, stmtOffset);
 }
 
 void DWARFContext::parseCompileUnits() {
-  if (!CUs.empty())
+  if (CUs.getNumUnits() != 0)
     return;
   uint32_t offset = 0;
   const DataExtractor &DIData = DataExtractor(getInfoSection().Data,
                                               isLittleEndian(), 0);
   while (DIData.isValidOffset(offset)) {
     std::unique_ptr<DWARFCompileUnit> CU(new DWARFCompileUnit(*this,
         getDebugAbbrev(), getInfoSection().Data, getRangeSection(),
         getStringSection(), StringRef(), getAddrSection(),
         &getInfoSection().Relocs, isLittleEndian(), CUs));
     if (!CU->extract(DIData, &offset)) {
       break;
     }
-    CUs.push_back(std::move(CU));
-    offset = CUs.back()->getNextUnitOffset();
+    offset = CU->getNextUnitOffset();
+    CUs.addUnit(std::move(CU));
   }
 }
 
 void DWARFContext::parseTypeUnits() {
-  if (!TUs.empty())
+  if (TUs.getNumUnits() != 0)
     return;
   for (const auto &I : getTypesSections()) {
     uint32_t offset = 0;
     const DataExtractor &DIData =
         DataExtractor(I.second.Data, isLittleEndian(), 0);
     while (DIData.isValidOffset(offset)) {
       std::unique_ptr<DWARFTypeUnit> TU(new DWARFTypeUnit(*this,
            getDebugAbbrev(), I.second.Data, getRangeSection(),
            getStringSection(), StringRef(), getAddrSection(),
            &I.second.Relocs, isLittleEndian(), TUs));
       if (!TU->extract(DIData, &offset))
         break;
-      TUs.push_back(std::move(TU));
-      offset = TUs.back()->getNextUnitOffset();
+      offset = TU->getNextUnitOffset();
+      TUs.addUnit(std::move(TU));
     }
   }
 }
 
 void DWARFContext::parseDWOCompileUnits() {
-  if (!DWOCUs.empty())
+  if (DWOCUs.getNumUnits() != 0)
     return;
   uint32_t offset = 0;
   const DataExtractor &DIData =
       DataExtractor(getInfoDWOSection().Data, isLittleEndian(), 0);
   while (DIData.isValidOffset(offset)) {
     std::unique_ptr<DWARFCompileUnit> DWOCU(new DWARFCompileUnit(*this,
         getDebugAbbrevDWO(), getInfoDWOSection().Data, getRangeDWOSection(),
         getStringDWOSection(), getStringOffsetDWOSection(), getAddrSection(),
         &getInfoDWOSection().Relocs, isLittleEndian(), DWOCUs));
     if (!DWOCU->extract(DIData, &offset)) {
       break;
     }
-    DWOCUs.push_back(std::move(DWOCU));
-    offset = DWOCUs.back()->getNextUnitOffset();
+    offset = DWOCU->getNextUnitOffset();
+    DWOCUs.addUnit(std::move(DWOCU));
   }
 }
 
 void DWARFContext::parseDWOTypeUnits() {
-  if (!DWOTUs.empty())
+  if (DWOTUs.getNumUnits() != 0)
     return;
   for (const auto &I : getTypesDWOSections()) {
     uint32_t offset = 0;
     const DataExtractor &DIData =
         DataExtractor(I.second.Data, isLittleEndian(), 0);
     while (DIData.isValidOffset(offset)) {
       std::unique_ptr<DWARFTypeUnit> TU(new DWARFTypeUnit(*this,
           getDebugAbbrevDWO(), I.second.Data, getRangeDWOSection(),
           getStringDWOSection(), getStringOffsetDWOSection(), getAddrSection(),
           &I.second.Relocs, isLittleEndian(), DWOTUs));
       if (!TU->extract(DIData, &offset))
         break;
-      DWOTUs.push_back(std::move(TU));
-      offset = DWOTUs.back()->getNextUnitOffset();
+      offset = TU->getNextUnitOffset();
+      DWOTUs.addUnit(std::move(TU));
     }
   }
 }
 
 DWARFCompileUnit *DWARFContext::getCompileUnitForOffset(uint32_t Offset) {
   parseCompileUnits();
   return CUs.getUnitForOffset(Offset);
 }

lib/DebugInfo/DWARFUnit.h

struct UnitOffsetComparator {
     }
   };
 
 public:
   typedef llvm::SmallVectorImpl<std::unique_ptr<UnitType>> UnitVector;
   typedef typename UnitVector::iterator iterator;
   typedef llvm::iterator_range<typename UnitVector::iterator> iterator_range;
 
+  iterator_range Units() {
+    return iterator_range(this->begin(), this->end());
+  }
+
+  unsigned getNumUnits() const {
+    return this->size();
+  }
+
+  UnitType *getUnitAtIndex(unsigned Index) {
+    return Index < this->size() ? (*this)[Index].get() : nullptr;
+  }
+
+  void addUnit(std::unique_ptr<UnitType> &&Unit) {
+    this->push_back(std::move(Unit));
+  }
+
   UnitType *getUnitForOffset(uint32_t Offset) const {
     auto *CU = std::lower_bound(this->begin(), this->end(), Offset,
                                 UnitOffsetComparator());
     if (CU != this->end())
       return CU->get();
     return nullptr;
   }
 };