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

[Ariel Bernal]

arielbernal added you to the CC list for the revision "Prevent LoopConvert from shadowing already defined variables with the same name as the loop variable.".

Hi revane,

This patch is work in progress.
Reverse AST from the loop statement upwards to check for any previous declaration with the same name.


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

Files:
  cpp11-migrate/LoopConvert/LoopActions.cpp
  cpp11-migrate/LoopConvert/StmtAncestor.cpp
  cpp11-migrate/LoopConvert/StmtAncestor.h
  cpp11-migrate/LoopConvert/VariableNaming.cpp
  cpp11-migrate/LoopConvert/VariableNaming.h

Index: cpp11-migrate/LoopConvert/LoopActions.cpp
===================================================================
--- cpp11-migrate/LoopConvert/LoopActions.cpp
+++ cpp11-migrate/LoopConvert/LoopActions.cpp
@@ -821,6 +821,7 @@
     // was used exactly once - in the initialization of AliasVar.
   } else {
     VariableNamer Namer(GeneratedDecls, &ParentFinder->getStmtToParentStmtMap(),
+                        &ParentFinder->getDeclToAncestorDeclMap(),
                         TheLoop, IndexVar, MaybeContainer, Context);
     VarName = Namer.createIndexName();
     // First, replace all usages of the array subscript expression with our new
Index: cpp11-migrate/LoopConvert/StmtAncestor.cpp
===================================================================
--- cpp11-migrate/LoopConvert/StmtAncestor.cpp
+++ cpp11-migrate/LoopConvert/StmtAncestor.cpp
@@ -30,6 +30,22 @@
   return true;
 }
 
+/// \brief Map Decl to parent Decl.
+bool StmtAncestorASTVisitor::VisitDecl(Decl *Declaration) {
+  if (const TemplateDecl *T = dyn_cast<TemplateDecl>(Declaration)) {
+    DeclAncestors.insert(std::make_pair(T->getTemplatedDecl(), Declaration));
+  }
+  else if (const DeclContext *DC = dyn_cast<DeclContext>(Declaration)) {
+    for (clang::DeclContext::decl_iterator I = DC->decls_begin(),
+                                              E = DC->decls_end();
+                I != E; ++I) {
+      if (const NamedDecl *V = dyn_cast<NamedDecl>(*I))
+        DeclAncestors.insert(std::make_pair(V, Declaration));
+    }
+  }
+  return true;
+}
+
 /// \brief Keep track of the DeclStmt associated with each VarDecl.
 ///
 /// Combined with StmtAncestors, this provides roughly the same information as
Index: cpp11-migrate/LoopConvert/StmtAncestor.h
===================================================================
--- cpp11-migrate/LoopConvert/StmtAncestor.h
+++ cpp11-migrate/LoopConvert/StmtAncestor.h
@@ -20,6 +20,9 @@
 /// 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;
 
+/// A map used to walk the AST in reverse: maps child Decl to parent Decl.
+typedef llvm::DenseMap<const clang::Decl*, const clang::Decl*> DeclAncestorMap;
+
 /// A map used to walk the AST in reverse:
 ///  maps VarDecl to the to parent DeclStmt.
 typedef
@@ -63,15 +66,22 @@
     return DeclParents;
   }
 
+  /// Accessor for DeclAncestors.
+  const DeclAncestorMap &getDeclToAncestorDeclMap() {
+    return DeclAncestors;
+  }
+
   friend class clang::RecursiveASTVisitor<StmtAncestorASTVisitor>;
 
 private:
   StmtParentMap StmtAncestors;
   DeclParentMap DeclParents;
+  DeclAncestorMap DeclAncestors;
   llvm::SmallVector<const clang::Stmt*, 16> StmtStack;
 
   bool TraverseStmt(clang::Stmt *Statement);
   bool VisitDeclStmt(clang::DeclStmt *Statement);
+  bool VisitDecl(clang::Decl *Declaration);
 };
 
 /// Class used to find the variables and member expressions on which an
Index: cpp11-migrate/LoopConvert/VariableNaming.cpp
===================================================================
--- cpp11-migrate/LoopConvert/VariableNaming.cpp
+++ cpp11-migrate/LoopConvert/VariableNaming.cpp
@@ -82,6 +82,72 @@
       return true;
   }
 
+  // Determine if the symbol was already used in parent statement within the
+  // FunctionDecl.
+  const Stmt *StmtChild = SourceStmt;
+  const Stmt *StmtAncestor = ReverseAST->lookup(StmtChild);
+  while (StmtAncestor != NULL) {
+    for (Stmt::const_child_iterator I = StmtAncestor->child_begin(),
+                                    E = StmtAncestor->child_end();
+        I != E; ++I) {
+      if (*I == StmtChild) break;
+      if (const DeclStmt *D = dyn_cast<DeclStmt>(*I)) {
+        for (DeclStmt::const_decl_iterator ID = D->decl_begin(),
+                                           IE = D->decl_end();
+            ID != IE; ++ID)
+          if (const VarDecl *V = dyn_cast<VarDecl>(*ID))
+            if (V->getName() == Symbol)
+              return true;
+      }
+    }
+    StmtChild = StmtAncestor;
+    StmtAncestor = ReverseAST->lookup(StmtChild);
+  }
+
+  // Determine if the symbol shadows any previous definition.
+  const Decl *DeclChild = OldIndex;
+  const Decl *DeclAncestor = DeclAncestors->lookup(DeclChild);
+  while(DeclAncestor != NULL) {
+    if (const TemplateDecl *T = dyn_cast<TemplateDecl>(DeclAncestor)) {
+      // Check that the symbol is not a template parameter of the current
+      // function or class.
+      const TemplateParameterList * TP = T->getTemplateParameters();
+      for (TemplateParameterList::const_iterator I = TP->begin(), E = TP->end();
+         I != E; ++I) {
+        if ((*I)->getName() == Symbol)
+          return true;
+      }
+    }
+    else if (const FunctionDecl *F = dyn_cast<FunctionDecl>(DeclAncestor)) {
+      // Check that the symbol is not a function parameter.
+      for (FunctionDecl::param_const_iterator I = F->param_begin(),
+                                              E = F->param_end();
+              I != E; ++I) {
+        if ((*I)->getName() == Symbol)
+          return true;
+      }
+    }
+    else if (const DeclContext *DC = dyn_cast<DeclContext>(DeclAncestor)) {
+      // Check that the symbol is not a variable or field declaration in a
+      // parent context.
+      for (DeclContext::decl_iterator I = DC->decls_begin(),
+                                      E = DC->decls_end();
+          I != E; ++I) {
+        // Stop searching whenever we reach the actual declaration, since any
+        // declaration after this point will not be shadowed by the new
+        // variable declaration.
+        if ((*I) == DeclChild)
+          break;
+
+        if (isa<VarDecl>(*I) || isa<FieldDecl>(*I))
+          if (dyn_cast<NamedDecl>(*I)->getName() == Symbol)
+            return true;
+      }
+    }
+    DeclChild = DeclAncestor;
+    DeclAncestor = DeclAncestors->lookup(DeclAncestor);
+  }
+
   // FIXME: Rather than detecting conflicts at their usages, we should check the
   // parent context.
   // For some reason, lookup() always returns the pair (NULL, NULL) because its
Index: cpp11-migrate/LoopConvert/VariableNaming.h
===================================================================
--- cpp11-migrate/LoopConvert/VariableNaming.h
+++ cpp11-migrate/LoopConvert/VariableNaming.h
@@ -29,9 +29,11 @@
  public:
   VariableNamer(
       StmtGeneratedVarNameMap *GeneratedDecls, const StmtParentMap *ReverseAST,
+      const DeclAncestorMap *DeclAncestors,
       const clang::Stmt *SourceStmt, const clang::VarDecl *OldIndex,
       const clang::VarDecl *TheContainer, const clang::ASTContext *Context)
       : GeneratedDecls(GeneratedDecls), ReverseAST(ReverseAST),
+        DeclAncestors(DeclAncestors),
         SourceStmt(SourceStmt), OldIndex(OldIndex), TheContainer(TheContainer),
         Context(Context) {}
 
@@ -45,6 +47,7 @@
  private:
   StmtGeneratedVarNameMap *GeneratedDecls;
   const StmtParentMap *ReverseAST;
+  const DeclAncestorMap *DeclAncestors;
   const clang::Stmt *SourceStmt;
   const clang::VarDecl *OldIndex;
   const clang::VarDecl *TheContainer;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D950.2.patch
Type: text/x-patch
Size: 7135 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130614/a33ea6b5/attachment.bin>