[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