[PATCH] D19236: Add DITypeIndex for CodeView and emit it for locals

[Adrian Prantl via llvm-commits]

aprantl added inline comments.

================
Comment at: include/llvm/IR/DebugInfoMetadata.h:2391
@@ +2390,3 @@
+/// Represents a type in a DITypeStream at the given index. Uses the
+/// DITypeStream in the current CU. Doesn't contain metadata operands, so this
+/// inherits directly from Metadata.
----------------
Does CU here mean DICompileUnit or llvm::Module? It's probably better to spell it out.

================
Comment at: lib/AsmParser/LLParser.cpp:4287
@@ -4273,3 +4286,3 @@
 ///  ::= !{...}
 ///  ::= !"string"
 ///  ::= !DILocation(...)
----------------
Comment update?
Mehdi already mentioned that this should also be in LangRef.rst.

================
Comment at: lib/IR/DIBuilder.cpp:689
@@ -688,3 +688,3 @@
     DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *File,
-    unsigned LineNo, DISubroutineType *Ty, bool isLocalToUnit,
+    unsigned LineNo, DITypeRef Ty, bool isLocalToUnit,
     bool isDefinition, unsigned ScopeLine, unsigned Flags, bool isOptimized,
----------------
Are we loosing type safety through this change? Can you double-check that there is a Verifier check that ensures that Function->getType() resolves to a DISubroutineType?

================
Comment at: lib/IR/Metadata.cpp:422
@@ -421,2 +421,3 @@
 
+
 //===----------------------------------------------------------------------===//
----------------
whitespace

================
Comment at: lib/IR/Verifier.cpp:990
@@ -990,1 +989,3 @@
+    Assert(isa<DISubroutineType>(T) || isa<DITypeIndex>(T),
+           "invalid subroutine type", &N, T);
   Assert(isTypeRef(N, N.getRawContainingType()), "invalid containing type", &N,
----------------
Ah there it is :-)


http://reviews.llvm.org/D19236