[Lldb-commits] [PATCH] D54670: 03/03: .debug_types: Update of D32167 (.debug_types) on top of D51578 (section concatenation)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Few style nits



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp:44
     const DWARFUnit *dwarf_cu = form_value.GetCompileUnit();
     if (dwarf_cu) {
+      // Replace the compile unit with the type signature compile unit
----------------
Invert and make this an early return.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp:51
+        uint64_t type_sig = form_value.Unsigned();
+        auto debug_info = dwarf_cu->GetSymbolFileDWARF()->DebugInfo();
+        auto tu = debug_info->GetTypeUnitForSignature(type_sig);
----------------
This `auto` is not entirely obvious for me, same for the next line.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp:53
+        auto tu = debug_info->GetTypeUnitForSignature(type_sig);
+        if (tu) {
+          cu_offset = tu->GetOffset();
----------------
How about `if (auto tu = ...)`?


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp:59
+        if (dwarf_cu->GetBaseObjOffset() != DW_INVALID_OFFSET)
+          cu_offset = dwarf_cu->GetBaseObjOffset();
+        else
----------------
How about
```
cu_offset = dwarf_cu->GetBaseObjOffset() != DW_INVALID_OFFSET ? dwarf_cu->GetBaseObjOffset() : dwarf_cu->GetOffset();
```


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp:64
     }
     die_offset = form_value.Reference();
   }
----------------
Move this up.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:262
+      auto signature_die = die.GetAttributeValueAsReferenceDIE(DW_AT_signature);
+      if (signature_die) {
+        type_sp = ParseTypeFromDWARF(sc, signature_die, log, type_is_new_ptr);
----------------
How about `if (auto signature_die = ...)`?


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.cpp:44
+  }
+  // reset the offset to where we tried to parse from if anything went wrong
+  *offset_ptr = cu_sp->m_offset;
----------------
Let's make this a sentence with a full stop at the end. 


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFTypeUnit.h:74
+protected:
+  // Type signature contained in a type unit which will be valid (non-zero)
+  // for type units only.
----------------
`///`


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:942
+
+    // reset the offset to where we tried to parse from if anything went wrong
+    *offset_ptr = m_offset;
----------------
Full sentence. 


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54670/new/

https://reviews.llvm.org/D54670