[PATCH] D34570: [DWARF] NFC: Collect info needed by DWARFFormValue into a helper

[Robinson, Paul via llvm-commits]

================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp:64
+DWARFFormValue::getFixedByteSize(dwarf::Form Form,
+                                 const DWARFFormParams *Params) {
   switch (Form) {
----------------
dblaikie wrote:
> Should this be optional? The only place that doesn't pass it looks to be DWARFAbbreviationDeclaration::extract - is that usage correct? Does the abbrev table implicit_const not support the extra features? Should there be some error path/check for any forms that would require the params?
There is a use-case for scanning the abbreviations table to determine which ones are fixed-size in all cases.  And the abbreviations table can be shared by units/line-tables with different formats/versions.  This was an important point for @clayborg and LLDB, I think.
So yes, I think it should remain optional.  We can change our minds later if I'm wrong.

Still might be worth moving the optionality to that one use case & adding some comments about why it's probably valid there so it doesn't get accidentally propagated to other users.

So we would have something like:
    Optional<uint8_t> getFixedByteSize(dwarf::Form Form, const DWARFFormParams &Params);
    Optional<uint8_t> getFixedByteSizeForAbbrev(dwarf::Form Form);
with commentary reinforcing the special case.  I can do that.
--paulr

https://reviews.llvm.org/D34570
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170624/82736d96/attachment.html>