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

Paul Robinson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 19 16:30:14 PDT 2017


probinson added a comment.

FYI I'm away at a conference next week and I might not get back to this until the week after.



================
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.
 
----------------
dblaikie wrote:
> 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?
Those cases are static member functions.

it sounds like you would prefer each method to have several overloads for the different use-cases.  I was thinking that FormValueInfo could be factored out into an independent class, constructed by the caller and then passed in, reducing the DWARFFormValue API surface (as there could be simply one copy of skipValue and getFixedByteSize instead of several of each).


https://reviews.llvm.org/D33155





More information about the llvm-commits mailing list