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

David Blaikie dblaikie at gmail.com
Tue Sep 16 14:38:39 PDT 2014


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.

================
Comment at: lib/DebugInfo/DWARFUnit.h:77
@@ +76,3 @@
+  UnitType *getUnitAtIndex(unsigned Index) {
+    return Index < this->size() ? (*this)[Index].get() : nullptr;
+  }
----------------
Why the null pointer check?

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

================
Comment at: lib/DebugInfo/DWARFUnit.h:80
@@ +79,3 @@
+
+  void addUnit(std::unique_ptr<UnitType> &&Unit) {
+    this->push_back(std::move(Unit));
----------------
Pass unique_ptrs (& other movable types) by value when passing ownership.

http://reviews.llvm.org/D5369






More information about the llvm-commits mailing list