[PATCH] Prevent LoopConvert from shadowing already defined variables with the same name as the loop variable.

[Manuel Klimek]

================
Comment at: cpp11-migrate/LoopConvert/StmtAncestor.h:21
@@ -20,3 +20,3 @@
 /// A map used to walk the AST in reverse: maps child Stmt to parent Stmt.
 typedef llvm::DenseMap<const clang::Stmt*, const clang::Stmt*> StmtParentMap;
 
----------------
Sorry if we went through this before and I forgot: why are you not using the getParent function from the ASTContext? It would be cool if you could add a comment explaining this (not necessarily in this patch).

================
Comment at: cpp11-migrate/LoopConvert/VariableNaming.cpp:111
@@ +110,3 @@
+  while(DeclAncestor != NULL) {
+    if (const TemplateDecl *T = dyn_cast<TemplateDecl>(DeclAncestor)) {
+      // Check that the symbol is not a template parameter of the current
----------------
Edwin Vane wrote:
> I think we could get away with just using DeclContexts directly and not making a whole DeclAncestor map. The information being stored in that map is just a replication of what's available by moving up the DeclContext chain.
+1. Also, if you have some declaration reference in your statement at whose level you're trying to insert, you could get from that declrefexpr to the decl, and then get up the declcontext chain.

Also, I'd be interested why using getParent from ASTContext doesn't give you what you want (and you can even use that with hasAncestor to directly get to the decl you're interested in from the matcher without the need to drill through all this yourself).


http://llvm-reviews.chandlerc.com/D950