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

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a subscriber: lhames.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good - I might experiment with raw string literals, see how it looks/feels personally at some point. Reckon it'd be easier to maintain, at least. (not having to wrap every line in "" and newlines, etc.



================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:620-621
 
-    StringRef *SectionData =
-        StringSwitch<StringRef *>(name)
-            .Case("debug_info", &InfoSection.Data)
-            .Case("debug_abbrev", &AbbrevSection)
-            .Case("debug_loc", &LocSection.Data)
-            .Case("debug_line", &LineSection.Data)
-            .Case("debug_aranges", &ARangeSection)
-            .Case("debug_frame", &DebugFrameSection)
-            .Case("eh_frame", &EHFrameSection)
-            .Case("debug_str", &StringSection)
-            .Case("debug_ranges", &RangeSection)
-            .Case("debug_macinfo", &MacinfoSection)
-            .Case("debug_pubnames", &PubNamesSection)
-            .Case("debug_pubtypes", &PubTypesSection)
-            .Case("debug_gnu_pubnames", &GnuPubNamesSection)
-            .Case("debug_gnu_pubtypes", &GnuPubTypesSection)
-            .Case("debug_info.dwo", &InfoDWOSection.Data)
-            .Case("debug_abbrev.dwo", &AbbrevDWOSection)
-            .Case("debug_loc.dwo", &LocDWOSection.Data)
-            .Case("debug_line.dwo", &LineDWOSection.Data)
-            .Case("debug_str.dwo", &StringDWOSection)
-            .Case("debug_str_offsets.dwo", &StringOffsetDWOSection)
-            .Case("debug_addr", &AddrSection)
-            .Case("apple_names", &AppleNamesSection.Data)
-            .Case("apple_types", &AppleTypesSection.Data)
-            .Case("apple_namespaces", &AppleNamespacesSection.Data)
-            .Case("apple_namespac", &AppleNamespacesSection.Data)
-            .Case("apple_objc", &AppleObjCSection.Data)
-            .Case("debug_cu_index", &CUIndexSection)
-            .Case("debug_tu_index", &TUIndexSection)
-            .Case("gdb_index", &GdbIndexSection)
-            // Any more debug info sections go here.
-            .Default(nullptr);
+    StringRef *SectionData = MapSectionToMember(name);
     if (SectionData) {
       *SectionData = data;
----------------
You can roll the variable declaration into the 'if' here, if you like


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:787-788
+  for (const auto &SecIt : Sections) {
+    StringRef *SectionData = MapSectionToMember(SecIt.first());
+    if (SectionData)
+      *SectionData = SecIt.second->getBuffer();
----------------
And here


================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1180
+  auto ErrOrSections = DWARFYAML::EmitDebugSections(StringRef(yamldata));
+  EXPECT_TRUE((bool)ErrOrSections);
+
----------------
Probably ASSERT_TRUE - if there is an error and you continue after this, it'll assert later from the Error not being handled which will probably be confusing.

@lhames might have some ideas about how to handle this sort of thing - I think it came up before/in other bits of Greg's work, so might be nice to have a solid idiom of "expect this to not Error" and log useful stuff about the error if it does fail, etc.


================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1182
+
+  auto &DebugSections = ErrOrSections.get();
+
----------------
Can write this as '*ErrOrSections' if you like


https://reviews.llvm.org/D28828