[PATCH] Make getUnitForOffset a DWARFUnit method instead of a DWARFContext method.
David Blaikie
dblaikie at gmail.com
Thu Sep 11 09:20:28 PDT 2014
================
Comment at: lib/DebugInfo/DWARFContext.cpp:394
@@ -411,9 +393,3 @@
parseCompileUnits();
-
- std::unique_ptr<DWARFCompileUnit> *CU =
- std::lower_bound(CUs.begin(), CUs.end(), Offset, OffsetComparator());
- if (CU != CUs.end()) {
- return CU->get();
- }
- return nullptr;
+ return DWARFUnit::getUnitForOffset<DWARFCompileUnit>(CUs, Offset);
}
----------------
You shouldn't need to specify UnitType here, should you? Template argument deduction should fire on the first parameter. (similarly at the other call site)
================
Comment at: lib/DebugInfo/DWARFUnit.h:188
@@ +187,3 @@
+ static UnitType *
+ getUnitForOffset(const SmallVectorImpl<std::unique_ptr<UnitType>> &Units,
+ uint32_t Offset) {
----------------
This doesn't necessarily have to be moved to the DWARFUnit - it could remain in the DWARFContext - though perhaps there's a circular referencing problem that makes this more convenient/possible here rather than where it was originally?
================
Comment at: lib/DebugInfo/DWARFUnit.h:190
@@ +189,3 @@
+ uint32_t Offset) {
+ auto *Unit = std::upper_bound(Units.begin(), Units.end(), Offset,
+ UnitOffsetComparator<UnitType>());
----------------
This looks like it's also changing the semantics based on one of your other code reviews - it'd be good to keep these things separate. (similarly I see the getOffset -> getNextUnitOffset change in the UnitOffsetComparator above)
================
Comment at: lib/DebugInfo/DWARFUnit.h:216
@@ +215,3 @@
+ DWARFUnit *getUnitForOffset(uint32_t Offset) const override {
+ return DWARFUnit::getUnitForOffset<UnitType>(Units, Offset);
+ }
----------------
You shouldn't need to specify UnitType here, should you? Template argument deduction should fire on the first parameter.
http://reviews.llvm.org/D5310
More information about the llvm-commits
mailing list