[PATCH] Factor the Unit section parsing into the DWARFUnitSection class.

David Blaikie dblaikie at gmail.com
Mon Sep 29 12:06:43 PDT 2014


================
Comment at: lib/DebugInfo/DWARFUnit.h:59
@@ -57,3 +58,3 @@
   DWARFUnitSection(DWARFUnitSection &&DUS) :
     SmallVector<std::unique_ptr<UnitType>, 1>(std::move(DUS)) {}
 
----------------
samsonov wrote:
> initialize Parsed in this constructor as well.
I /think/ you can just use "SmallVector(std::move(DUS))" here.

================
Comment at: lib/DebugInfo/DWARFUnit.h:68
@@ +67,3 @@
+             StringRef AOS, const RelocAddrMap *M, bool LE) {
+    if (Parsed)
+      return;
----------------
(non-actionable gripe: I'm not a huge fan of the extra flag here (not that it's any worse than the previous check - and it's marginally better in some ways)... calling parse repeatedly only to have it no-op because it's already been parsed does feel a bit weird.

What would happen if the users instead had an Optional<DWARFUnitSection> and would check that, then construct a DWARFUnitSection passing in the various sections, etc - so there was no possibility of a non-parsed DWARFUnitSection. They were constructed with the data and parsed during construction only?

(OK, maybe semi-actionable, but could just be fanciful ideas on my part))

http://reviews.llvm.org/D5522






More information about the llvm-commits mailing list