[PATCH] D42736: [DebugInfo] Improvements to representation of enumeration types (PR36168)

[Momchil Velikov via Phabricator via cfe-commits]

chill marked 3 inline comments as done.
chill added inline comments.


================
Comment at: test/CodeGen/debug-info-enum.cpp:2
+// RUN: %clang -target x86_64-linux -g -S -emit-llvm -o - %s | FileCheck %s
+enum class E0 : signed char {
+  A0 = -128,
----------------
dblaikie wrote:
> Could you summarize the purpose of each of these tests (possibly in a comment above the enum in each case) - there look to be more test cases than I'd imagine being necessary, but I haven't carefully analyzed them.
> 
> For example: I wouldn't expect to test every integer type, if the code handling them is general enough to be demonstrated by one or two cases?
Yes, in hindsight, there are more tests that strictly necessary, I just enumerated all the different integer type sizes cross signed/usigned, for some there were no errors, for some there were always errors, for some there were sometimes errors only sometimes (i.e. depending on compiler target and/or options).



================
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)
----------------
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.


https://reviews.llvm.org/D42736