[PATCH] D30357: [ObjectYAML] NFC. Refactor DWARFYAML CompileUnit dump code
Paul Robinson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 2 15:33:59 PST 2017
probinson added inline comments.
================
Comment at: lib/ObjectYAML/DWARFVisitor.cpp:59
+ writeSize =
+ DebugInfo.DebugLines[0].TotalLength == UINT32_MAX ? 8 : 4;
+ OnVariableSizeValue(FormVal->Value, writeSize);
----------------
beanz wrote:
> 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.
Why do you need the line table? Every unit has a length that lets you determine its 32/64 format.
https://reviews.llvm.org/D30357
More information about the llvm-commits
mailing list