- Now it handles using directives, I missed that before.
- Added tests to verify naming conflicts.
Hi revane, klimek, doug.gregor,
http://llvm-reviews.chandlerc.com/D950
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D950?vs=2452&id=2513#toc
Files:
cpp11-migrate/LoopConvert/VariableNaming.cpp
test/cpp11-migrate/LoopConvert/naming-conflict.cpp
Index: cpp11-migrate/LoopConvert/VariableNaming.cpp
===================================================================
--- cpp11-migrate/LoopConvert/VariableNaming.cpp
+++ cpp11-migrate/LoopConvert/VariableNaming.cpp
@@ -82,6 +82,103 @@
return true;
}
+ // Determine if the symbol was used in a parent scope to avoid shadowing any
+ // previously defined variable.
+ const Stmt *PrevStmt = SourceStmt;
+ const Decl *PrevDecl = OldIndex;
+ // Reverse AST from the loop statement.
+ ast_type_traits::DynTypedNode Start =
+ clang::ast_type_traits::DynTypedNode::create(*SourceStmt);
+ do {
+ const ASTContext::ParentVector Parents =
+ const_cast<ASTContext *>(Context)->getParents(Start);
+ if (Parents.empty())
+ break;
+ ASTContext::ParentVector::const_iterator Parent = Parents.begin();
+ if (const Stmt *S = Parent->get<Stmt>()) {
+ // Check for VarDecls in a parent statement.
+ for (Stmt::const_child_iterator I = S->child_begin(), E = S->child_end();
+ I != E; ++I) {
+ // Skip all statements after the current statement.
+ if (*I == PrevStmt)
+ break;
+ // Skip null statements e.g. in a ForStmt where the condition variable
+ // is not specified.
+ if (*I == NULL)
+ continue;
+ if (const DeclStmt *D = dyn_cast<DeclStmt>(*I)) {
+ for (DeclStmt::const_decl_iterator ID = D->decl_begin(),
+ IE = D->decl_end();
+ ID != IE; ++ID)
+ // Check for using directives that may have been used in the same
+ // compound statement, CXXMethod or function.
+ if (const UsingDirectiveDecl *U =
+ dyn_cast<UsingDirectiveDecl>(*ID)) {
+ const DeclContext *DC =
+ dyn_cast<DeclContext>(U->getNominatedNamespace());
+ for (DeclContext::decl_iterator IU = DC->decls_begin(),
+ EU = DC->decls_end();
+ IU != EU; ++IU)
+ if (dyn_cast<VarDecl>(*IU)->getName() == Symbol)
+ return true;
+ } else if (const VarDecl *V = dyn_cast<VarDecl>(*ID))
+ if (V->getName() == Symbol)
+ return true;
+ }
+ }
+ PrevStmt = S;
+ } else if (const Decl *D = Parent->get<Decl>()) {
+ if (const TemplateDecl *T = dyn_cast<TemplateDecl>(D)) {
+ // 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>(D)) {
+ // Check that the symbol is not a function or CXXMethodDecl 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>(D)) {
+ for (DeclContext::decl_iterator I = DC->decls_begin(),
+ E = DC->decls_end();
+ I != E; ++I) {
+ // Check for using directives that might have been used in the same
+ // namespace.
+ if (const UsingDirectiveDecl *U = dyn_cast<UsingDirectiveDecl>(*I)) {
+ const DeclContext *DC =
+ dyn_cast<DeclContext>(U->getNominatedNamespace());
+ for (DeclContext::decl_iterator ID = DC->decls_begin(),
+ ED = DC->decls_end();
+ ID != ED; ++ID)
+ if (dyn_cast<VarDecl>(*ID)->getName() == Symbol)
+ return true;
+ } else if (const NamedDecl *V = dyn_cast<NamedDecl>(*I)) {
+ // Check for variable or field declarations with the same name in a
+ // parent context.
+ // Skip declarations after current Decl.
+ if (V == PrevDecl)
+ break;
+ if (isa<VarDecl>(*I) || isa<FieldDecl>(*I))
+ if (dyn_cast<NamedDecl>(*I)->getName() == Symbol)
+ return true;
+ }
+ }
+ }
+ PrevDecl = D;
+ }
+ Start = *Parent;
+ } while (1);
+ // FIXME: We might want to get the search for previously defined variables
+ // in a parent scope into a more common library in clang itself.
+
// 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: test/cpp11-migrate/LoopConvert/naming-conflict.cpp
===================================================================
--- test/cpp11-migrate/LoopConvert/naming-conflict.cpp
+++ test/cpp11-migrate/LoopConvert/naming-conflict.cpp
@@ -117,3 +117,226 @@
// CHECK: for (auto & sts_it : sts)
// CHECK-NEXT: sts_it = sizeof(st);
}
+
+// Template parameters conflicts
+// Check for conflicts with template functions wtih TemplateType parameters.
+// T cannot be used since it's a function template parameter.
+template <typename K, typename T>
+void templateFuctionParamConflict1() {
+ const int N = 10;
+ int Ts[N];
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & Ts_i : Ts)
+ // CHECK-NEXT: int k = Ts_i + 2 * Ts_i;
+}
+
+// Check for conflicts with template functions wtih NonTypeTemplate parameters.
+// T cannot be used since it's a function template parameter.
+template <typename K, unsigned T>
+void templateFunctionParamConflict2() {
+ const int N = 10;
+ int Ts[N];
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & Ts_i : Ts)
+ // CHECK-NEXT: int k = Ts_i + 2 * Ts_i;
+}
+
+// Check for template parameters conflicts in class template declarations.
+template <typename K, typename T>
+class TemplateClassParamConflict {
+ // T cannot be used since it is class template parameter.
+ void foo() {
+ const int N = 10;
+ int Ts[N];
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & Ts_i : Ts)
+ // CHECK-NEXT: int k = Ts_i + 2 * Ts_i;
+ }
+};
+
+
+// The following tests check for variable shadowing resulting in warnings when
+// -Wshadow is specified
+
+void shadowVariableFunction1() {
+ const int N = 10;
+ int Ts[N];
+ int S, T;
+ // T cannot be use since was previously declared.
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & Ts_i : Ts)
+ // CHECK-NEXT: int k = Ts_i + 2 * Ts_i;
+}
+
+void shadowVariableFunction2() {
+ const int N = 10;
+ int Ts[N];
+ {
+ int S, T;
+ }
+ // it its safe to use T since T is defined in a different compound statemt.
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & T : Ts)
+ // CHECK-NEXT: int k = T + 2 * T;
+}
+
+// Nested compound statements.
+void shadowVariableFunction3() {
+ const int N = 10;
+ int Ts[N];
+ {
+ int S, T;
+ {
+ {
+ // T cannot be use since was previously declared in a parent compound
+ // statement.
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & Ts_i : Ts)
+ // CHECK-NEXT: int k = Ts_i + 2 * Ts_i;
+ }
+ }
+ }
+}
+
+// Nested compound statemens.
+void shadowVariableFunction5() {
+ const int N = 10;
+ int Ts[N];
+ {
+ {
+ {
+ // it its safe to use T since T is defined afterwards.
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & T : Ts)
+ // CHECK-NEXT: int k = T + 2 * T;
+ int S, T;
+ }
+ int S, T;
+ }
+ int S, T;
+ }
+ int S, T;
+}
+
+void shadowVariableFunction6(int S, int T) {
+ const int N = 10;
+ int Ts[N];
+ // T cannot be use since it's a function parameter.
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & Ts_i : Ts)
+ // CHECK-NEXT: int k = Ts_i + 2 * Ts_i;
+}
+
+class ClassFieldShadow {
+public:
+ int S, T;
+ void foo() {
+ const int N = 10;
+ int Ts[N];
+ // T cannot be used since T was previously defined.
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & Ts_i : Ts)
+ // CHECK-NEXT: int k = Ts_i + 2 * Ts_i;
+ }
+};
+
+class DerivedClassShadow : ClassFieldShadow {
+ void foo() {
+ const int N = 10;
+ int Ts[N];
+ // T can be used here since base class members are just hidden not shadowed,
+ // at least clang and gcc don't complain when using -Wshadow.
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & T : Ts)
+ // CHECK-NEXT: int k = T + 2 * T;
+ }
+};
+
+namespace shadow0 {
+ int S, T;
+ const int N = 10;
+ int Ts[N];
+ // T cannot be use here since it shadows a previously defined variable T in
+ // the same parent context.
+ void f() {
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & Ts_i : Ts)
+ // CHECK-NEXT: int k = Ts_i + 2 * Ts_i;
+ }
+}
+
+namespace shadow1 {
+ const int N = 10;
+ int Ts[N];
+ // It is safe to use T as a loop variable since T is defined afterwards.
+ void f() {
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & T : Ts)
+ // CHECK-NEXT: int k = T + 2 * T;
+ }
+ int S, T;
+}
+
+namespace shadow2 {
+ namespace ns1 {
+ namespace ns2 {
+ int S, T;
+ }
+ }
+
+ const int N = 10;
+ int Ts[N];
+ // It is safe to use T as a loop variable since T is defined in a different
+ // namespace and the using directive happens afterwards.
+ void f() {
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & T : Ts)
+ // CHECK-NEXT: int k = T + 2 * T;
+ }
+
+ // Check for using directives included in the same CompoundStmt.
+ void g() {
+ using namespace ns1::ns2;
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & Ts_i : Ts)
+ // CHECK-NEXT: int k = Ts_i + 2 * Ts_i;
+ }
+
+ // Check for using directives included in a parent context.
+ using namespace ns1::ns2;
+ void h() {
+ for (int i = 0; i < N; ++i) {
+ int k = Ts[i] + 2 * Ts[i];
+ }
+ // CHECK: for (auto & Ts_i : Ts)
+ // CHECK-NEXT: int k = Ts_i + 2 * Ts_i;
+ }
+}
\ No newline at end of file
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
