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

[Reid Kleckner via llvm-commits]

rnk added inline comments.

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:879-881
@@ -871,1 +878,5 @@
 
+  // While processing the type being pointed to it is possible we already
+  // created this pointer type.  If so, we check here and return the existing
+  // pointer type.
+  //
----------------
This example works fine today without this extra lookup, though. The only way I could imagine this happening is if translating 'A*' triggers translation of the complete A type, and we can avoid that. The graph of types should be acyclic if you don't follow edges through record types, which is what I was trying to do.

Instead, I think we should queue all complete record types that we've seen during type translation in a separate list, and lower them later. We'll need this anyway to implement S_UDT.

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:1065
@@ +1064,3 @@
+                                               const DISubprogram *SP) {
+  switch (SP->getFlags() & DINode::FlagAccessibility) {
+  case DINode::FlagPrivate:   return MemberAccess::Private;
----------------
Hm, maybe this thing should just take `(unsigned RecordTag, unsigned Flags)` so we can avoid the duplication.

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:1293
@@ +1292,3 @@
+// stripScopesFromName(name)
+//
+// Subprogram names used to contain only the name, now they are fully
----------------
Sure, we can do that. Just modify the metadata in your test case to have the names you expect and we'll fix clang later.


http://reviews.llvm.org/D21011