[PATCH] D28828: [DWARF] [ObjectYAML] Adding APIs for unittesting

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Looks neat - few ideas/questions.



================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:819-851
+    StringRef *SectionData =
+        StringSwitch<StringRef *>(SecIt.first())
+            .Case("debug_info", &InfoSection.Data)
+            .Case("debug_abbrev", &AbbrevSection)
+            .Case("debug_loc", &LocSection.Data)
+            .Case("debug_line", &LineSection.Data)
+            .Case("debug_aranges", &ARangeSection)
----------------
Looks like a duplicate of the StringSwitch in the other ctor - could be refactored to a common utility function?


================
Comment at: lib/ObjectYAML/DWARFEmitter.cpp:356-358
+  YIn >> DI;
+  if (YIn.error())
+    return DebugSections;
----------------
Should this perhaps return ErrorOr<StringMap<...>> & so the error can be propagated/reported?


================
Comment at: lib/ObjectYAML/DWARFEmitter.cpp:360-369
+  EmitDebugSectionImpl(DI, &DWARFYAML::EmitDebugInfo, "debug_info",
+                       DebugSections);
+  EmitDebugSectionImpl(DI, &DWARFYAML::EmitDebugLine, "debug_line",
+                       DebugSections);
+  EmitDebugSectionImpl(DI, &DWARFYAML::EmitDebugStr, "debug_str",
+                       DebugSections);
+  EmitDebugSectionImpl(DI, &DWARFYAML::EmitDebugAbbrev, "debug_abbrev",
----------------
These are the supported sections at the moment - but I guess it'll grow support over time? Any way to refactor this so it goes along with the support as it's added? (is there similar code elsewhere already doing something like this to generate all the outputs in yaml2obj?)


================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1163-1177
+  const char *yamldata = "debug_abbrev:\n"
+                         "  - Code:            0x00000001\n"
+                         "    Tag:             DW_TAG_compile_unit\n"
+                         "    Children:        DW_CHILDREN_yes\n"
+                         "    Attributes:\n"
+                         "debug_info:\n"
+                         "  - Length:          9\n"
----------------
Are raw string literals an option here/yet? (are they compatible with all LLVM's supported compilers)


https://reviews.llvm.org/D28828