[PATCH] D21011: [codeview] Add complex record type translation

[Reid Kleckner via llvm-commits]

rnk added inline comments.

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:1202
@@ +1201,3 @@
+    if (auto *SP = dyn_cast<DISubprogram>(Element)) {
+      Info.Methods[SP->getRawName()].push_back({ SP, true });
+    } else if (auto *DDTy = dyn_cast<DIDerivedType>(Element)) {
----------------
aaboud wrote:
> rnk wrote:
> > Should we claim that all methods are introduced virtual methods, or just the virtual ones?
> Introduce is only for virtual methods, however the "Methods" container holds all methods of the class (virtual and non-virtual).
> I am using the same container to be able to output the methods in the right order, though I am not sure if the order is important!
> 
> See "translateMethodKindFlags" at line 1091, where it consider the "Introduced" variable only for virtual functions.
I guess I'm questioning the usefulness of MethodInfo. The 'Introduced' boolean is always set to true in this code.

How do you propose to calculate whether a method was introduced? Remember, we can't actually walk the class hierarchy because the frontend may not emit complete debug information about base classes, unless -fstandalone-debug-info is on. I think this is something that we will need the frontend to tell us directly, see https://llvm.org/bugs/show_bug.cgi?id=28150.

Anyway, this doesn't need to hold up the patch. With tests I think this is ready and we can iterate on it from there.


http://reviews.llvm.org/D21011