[PATCH] D30357: [ObjectYAML] NFC. Refactor DWARFYAML CompileUnit dump code

[Chris Bieneman via Phabricator via llvm-commits]

beanz added inline comments.


================
Comment at: lib/ObjectYAML/DWARFVisitor.cpp:59
+              writeSize =
+                  DebugInfo.DebugLines[0].TotalLength == UINT32_MAX ? 8 : 4;
+            OnVariableSizeValue(FormVal->Value, writeSize);
----------------
aprantl wrote:
> This would be easier to read if written like this:
> ```
> uint64_t?? writeSize;
> if (!DebugInfo.DebugLines.empty())
>   writeSize =  DebugInfo.DebugLines[0].TotalLength == UINT32_MAX ? 8 : 4;
> else if (Unit.Version == 2)
>   writeSize = Unit.AddrSize
> else
>   writeSize = 4;
> ```
There is actually some slightly incorrect logic here. The correct logic should be more like:


```
size_t writeSize = 4;
if (Unit.Version == 2)
  writeSize = Unit.AddrSize;
else if (!DebugInfo.DebugLines.empty())
  writeSize = DebugInfo.DebugLines[0].TotalLength == UINT32_MAX ? 8 : 4;
```

The idea being that size 4 as a default is sane. If version == 2, then you use AddrSize, otherwise it is based on whether it is DWARF32 or DWARF64 which requires the Line table.


================
Comment at: lib/ObjectYAML/DWARFVisitor.cpp:138
+          case dwarf::DW_FORM_ref_sup: {
+            auto writeSize = 4;
+            if (!DebugInfo.DebugLines.empty())
----------------
aprantl wrote:
> Can we factor this out into a tiny static helper function?
Yes, will do.


https://reviews.llvm.org/D30357