[PATCH] D28581: Change DWARFDie::getAttributeValue() to DWARFDie::find(), add DWARFDie::findRecurse() and dwarf::toString() helper functions.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Please include tests for all the toFoo helpers (in both flavors)

Ideally by creating DWARFFormValue objects directly (if that's not possible currently - ideally make it possible) so the tests don't have to be so long/complicated as generating and parsing DWARF to test these utilities.



================
Comment at: lib/DebugInfo/DWARF/DWARFDie.cpp:257-266
   const char *name = nullptr;
   // Try to get mangled name only if it was asked for.
   if (Kind == DINameKind::LinkageName) {
-    if ((name = getAttributeValueAsString(DW_AT_MIPS_linkage_name, nullptr)))
+    if ((name = dwarf::toString(find(DW_AT_MIPS_linkage_name), nullptr)))
       return name;
-    if ((name = getAttributeValueAsString(DW_AT_linkage_name, nullptr)))
+    if ((name = dwarf::toString(find(DW_AT_linkage_name), nullptr)))
       return name;
----------------
Change these all to:

  if (auto Name = toString(find(DW_AT_name)))
    return *Name;

If that's correct


================
Comment at: lib/DebugInfo/DWARF/DWARFTypeUnit.cpp:29-31
+  const char *Name = llvm::dwarf::toString(TD.find(llvm::dwarf::DW_AT_name), "");
 
   if (SummarizeTypes) {
----------------
Similarly:

  if (auto Name = toString(TD.find(dwarf::DW_AT_name)))
    ... *Name ...


================
Comment at: lib/DebugInfo/DWARF/DWARFUnit.cpp:269
     return false;
-  const char *CompilationDir =
-      UnitDie.getAttributeValueAsString(DW_AT_comp_dir, nullptr);
+  const char *CompilationDir = toString(UnitDie.find(DW_AT_comp_dir), nullptr);
   SmallString<16> AbsolutePath;
----------------
Probably remove the nullptr-y-ness here too.

Could remove it from the DWOFileName too in a variety of ways, but *shrug* if you like.


================
Comment at: tools/dsymutil/DwarfLinker.cpp:208
     auto CUDie = OrigUnit.getUnitDIE(false);
-    unsigned Lang =
-        CUDie.getAttributeValueAsUnsignedConstant(dwarf::DW_AT_language)
-            .getValueOr(0);
+    unsigned Lang = dwarf::toUnsigned(CUDie.find(dwarf::DW_AT_language), 0);
     HasODR = CanUseODR && (Lang == dwarf::DW_LANG_C_plus_plus ||
----------------
Could remove the defaulting here as well - placing the HasODR assignment below in a conditional:

  if (auto Lang = toUnsigned(CUDie.find(...)))
    HasODR = ...


================
Comment at: tools/dsymutil/DwarfLinker.cpp:844-845
   int64_t UnitPcOffset = 0;
-  auto OrigLowPc = OrigUnitDie.getAttributeValueAsAddress(dwarf::DW_AT_low_pc);
+  auto OrigLowPc = dwarf::toAddress(OrigUnitDie.find(dwarf::DW_AT_low_pc));
   if (OrigLowPc)
     UnitPcOffset = int64_t(*OrigLowPc) - Unit.getLowPc();
----------------
Roll the variable declaration into the conditional


https://reviews.llvm.org/D28581