[PATCH] D30357: [ObjectYAML] NFC. Refactor DWARFYAML CompileUnit dump code
Adrian Prantl via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 2 11:13:33 PST 2017
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.
Sorry for the delay. Few comments inline, LGTM with these addressed.
================
Comment at: lib/ObjectYAML/DWARFVisitor.cpp:19
+void DWARFYAML::VisitorImpl<T>::OnVariableSizeValue(uint64_t U, size_t Size) {
+ if (8 == Size)
+ OnValue((uint64_t)U);
----------------
switch(Size)
================
Comment at: lib/ObjectYAML/DWARFVisitor.cpp:43
+ auto AbbrForm = Abbrev.Attributes.begin();
+ for (;
+ FormVal != Entry.Values.end() && AbbrForm != Abbrev.Attributes.end();
----------------
Personally, I'd rather use a while loop here, but I'll leave that up to you.
================
Comment at: lib/ObjectYAML/DWARFVisitor.cpp:48
+ dwarf::Form Form = AbbrForm->Form;
+ bool Indirect = false;
+ do {
----------------
this is redundant
================
Comment at: lib/ObjectYAML/DWARFVisitor.cpp:59
+ writeSize =
+ DebugInfo.DebugLines[0].TotalLength == UINT32_MAX ? 8 : 4;
+ OnVariableSizeValue(FormVal->Value, writeSize);
----------------
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;
```
================
Comment at: lib/ObjectYAML/DWARFVisitor.cpp:138
+ case dwarf::DW_FORM_ref_sup: {
+ auto writeSize = 4;
+ if (!DebugInfo.DebugLines.empty())
----------------
Can we factor this out into a tiny static helper function?
================
Comment at: lib/ObjectYAML/DWARFVisitor.h:33
+
+ // Visitor Functions
+ virtual void OnStartCompileUnit(Unit &CU) {}
----------------
A thing we do in other headers is:
```
/// Visitor Functions.
/// @{
virtual void OnStartCompileUnit(Unit &CU) {}
virtual void OnEndCompileUnit(Unit &CU) {}
virtual void OnStartDIE(Unit &CU, Entry &DIE) {}
virtual void OnEndDIE(Unit &CU, Entry &DIE) {}
virtual void OnForm(AttributeAbbrev &AttAbbrev, FormValue &Value) {}
/// @}
```
https://reviews.llvm.org/D30357
More information about the llvm-commits
mailing list