[PATCH] D33155: [DWARFv5] Support FORM_strp in the line table header

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFFormValue.h:42
+  struct FormValueInfo {
+    FormValueInfo() {}
+    FormValueInfo(const DWARFUnit *U) { Init(U); }
----------------
  = default


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFFormValue.h:58-66
+    void Init(const DWARFUnit *U);
+    void Init(const DWARFUnit *U, const RelocAddrMap *RMap, uint16_t Ver,
+              uint8_t AddrByteSize, bool IsDwarf64) {
+      Unit = U;
+      RelocMap = RMap;
+      Version = Ver;
+      AddrSize = AddrByteSize;
----------------
I'd consider keeping the ctors, dropping these Init functions, and where you have:

  x.Init(...);

use:
 
  x = FormValueInfo(...);

perhaps? since it's simple to copy/move, etc. Seems like less API surface, surprises (no/less chance that Init leaves some stale state, etc), etc :)


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFFormValue.h:86
   ValueType Value;              // Contains all data for the form.
-  const DWARFUnit *U = nullptr; // Remember the DWARFUnit at extract time.
+  FormValueInfo Info;           // Info useful during extraction.
 
----------------
The other point I was making here was that maybe the members of FormValueInfo could be members of DWARFFormValue directly.

I see there are a few cases where FormValueInfo objects are constructed separately from a DWARFFormValue - but they're all /inside/ a DWARFFormValue member function if I'm not mistaken. Why are those cases not using the FormValueInfo in the DWARFFormValue they're a member of?


https://reviews.llvm.org/D33155