[PATCH] D36740: [llvm-dwarfdump] - Refactor section name/uniqueness gathering.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFSection.h:22
 
+struct SectionInfo {
+  StringRef Name;
----------------
Maybe SectionName or SectionNameInfo?


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:1188-1191
+
+    for (SectionInfo &Info : Sections)
+      if (SectionAmountMap[Info.Name] > 1)
+        Info.IsNameUnique = false;
----------------
Reckon it might be worth doing this lazily (inside getSectionNames)? So it's not computed if it's not needed?

I guess if we're already walking all the sections and getting their names, this is probably cheap enough to do unconditionally.


================
Comment at: lib/DebugInfo/DWARF/DWARFDie.cpp:60-62
+  ArrayRef<SectionInfo> Sections;
+  if (!DumpOpts.Brief)
+    Sections = Obj.getSections();
----------------
Similarly, probably call these SectionNames and getSectionNames (& similarly inside DWARFObject)


================
Comment at: lib/DebugInfo/DWARF/DWARFDie.cpp:72
 
-    if (SectionNames.empty() || R.SectionIndex == -1ULL)
+    if (DumpOpts.Brief || Sections.empty() || R.SectionIndex == -1ULL)
       continue;
----------------
Can probably skip DumpOpts.Brief here, since Sections will be empty in that case?


================
Comment at: lib/DebugInfo/DWARF/DWARFDie.cpp:76
+    StringRef Name = Sections[R.SectionIndex].Name;
     OS << format(" \"%s\"", Name.str().c_str());
 
----------------
does format not support StringRef? Or stream it out directly:

  OS << " \"" << Name << '\"';

Save having to str().c_str() it.


https://reviews.llvm.org/D36740