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

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 20 11:31:01 PST 2017

dblaikie added inline comments.

Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:620-653
+    StringRef *SectionData = MapSectionToMember(name);
     if (SectionData) {
       *SectionData = data;
       if (name == "debug_ranges") {
         // FIXME: Use the other dwo range section when we emit it.
         RangeDWOSection = data;
dblaikie wrote:
> You can roll the variable declaration into the 'if' here, if you like
Did this regress when you committed it (if I'm reading the patch/changes/Phabricator correctly) - looks like the StringSwitch came back, rather than using the common utility function, MapSectionToMember.

Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1187
   // Verify the number of compile units is correct.
   uint32_t NumCUs = DwarfContext.getNumCompileUnits();
   EXPECT_EQ(NumCUs, 1u);
This should probably be an ASSERT - otherwise it'll crash/UB on the next line anyway.

Did you chat to Lang about the fact that this would produce a follow-on assert if it failed? (since the code doesn't handle the error, if there is one)

Perhaps something like HandleExpectedError would be suitable/necessary here.

Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1188
   uint32_t NumCUs = DwarfContext.getNumCompileUnits();
   EXPECT_EQ(NumCUs, 1u);
   DWARFCompileUnit *U = DwarfContext.getCompileUnitAtIndex(0);
Could write this as **Obj, if you like.



More information about the llvm-commits mailing list