[PATCH] D15977: [Clang] Supporting all entities declared in lexical scope in LLVM debug info

[David Blaikie via cfe-commits]

dblaikie added inline comments.

================
Comment at: lib/CodeGen/CGDebugInfo.cpp:1479
@@ +1478,3 @@
+void CGDebugInfo::recordDeclarationLexicalScope(const Decl &D) {
+  assert(LexicalBlockMap.find(&D) == LexicalBlockMap.end() &&
+         "D is already mapped to lexical block scope");
----------------
Dose this assert not fire if there are two decls in the same lexical scope?

================
Comment at: lib/CodeGen/CGDebugInfo.cpp:1481
@@ +1480,3 @@
+         "D is already mapped to lexical block scope");
+  if (!LexicalBlockStack.empty())
+    LexicalBlockMap[&D] = LexicalBlockStack.back();
----------------
Should we assert that LexicalBlockStack isn't empty?

================
Comment at: lib/CodeGen/CGDebugInfo.cpp:1488
@@ +1487,3 @@
+  auto I = LexicalBlockMap.find(&D);
+  if (I != LexicalBlockMap.end()) {
+    RetainedTypes.push_back(Ty.getAsOpaquePtr());
----------------
Perhaps we should assert here that if D's scope is a lexical block, that it should have a corresponding entry in the LexicalBlockMap?

================
Comment at: lib/CodeGen/CGDebugInfo.cpp:1489
@@ +1488,3 @@
+  if (I != LexicalBlockMap.end()) {
+    RetainedTypes.push_back(Ty.getAsOpaquePtr());
+    return I->second;
----------------
Why does the type need to be added to the retained types list?

================
Comment at: lib/CodeGen/CGDebugInfo.cpp:1491
@@ +1490,3 @@
+    return I->second;
+  } else
+    return getDeclContextDescriptor(cast<Decl>(&D));
----------------
else-after-return (just drop the else and let the following code remain unconditionally)

================
Comment at: lib/CodeGen/CGDebugInfo.cpp:2066
@@ -2045,3 +2065,3 @@
   if (isImportedFromModule || !ED->getDefinition()) {
-    llvm::DIScope *EDContext = getDeclContextDescriptor(ED);
+    llvm::DIScope *EDContext = getDeclarationLexicalScope(*ED, QualType(Ty, 0));
     llvm::DIFile *DefUnit = getOrCreateFile(ED->getLocation());
----------------
Would it be reasonable/possible to do these changes one at a time & demonstrate the different situations in which types were not correctly nested in lexical scopes? Or are they all necessary for some correctness/invariant that must hold?


http://reviews.llvm.org/D15977