[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