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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
