dblaikie accepted this revision.
dblaikie added a comment.

Few things that could tidy up the tests a bit, but otherwise looks good :)



================
Comment at: test/CodeGen/debug-info-enum.cpp:6-10
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "E0"
+// CHECK-SAME: flags: DIFlagFixedEnum
+// CHECK: !DIBasicType(name: "signed char", size: 8, encoding: 
DW_ATE_signed_char)
+// CHECK: !DIEnumerator(name: "A0", value: -128)
+// CHECK: !DIEnumerator(name: "B0", value: 127)
----------------
chill wrote:
> dblaikie wrote:
> > Rather than relying on specific ordering of output, generally you should 
> > test that the actual references across different metadata records are 
> > correct (eg: check that the DICompositeType's member list elements are the 
> > DIEnumerators (use named matches, rather than hardcoding the metadata node 
> > numbers))
> > 
> > Though, admittedly, this is a lot easier to read as-is. 
> I've done that, to some extent, making sure each enumeration refers to the 
> expected underlying type. Not idea how to do that completely independent of 
> order.
Thanks! Maybe add a comment (like I suggested in the LLVM review too, just now) 
in each case to describe each test case. You have one up the top for the enum 
class over signed char, but someting similar for each enum might be good.

Yeah, it's impossible to do it entirely independent of order.

You could do a bit more than what you've got here - the DIEnumerators could be 
checked:

match the list from the DICompositeType enumeration_type (the same way you 
match the baseType) then match the elements of the list, then match those 
elements to the DIEnumerators same as you do for the DIBasicType) - does that 
make sense?


https://reviews.llvm.org/D42736



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to