hokein updated this revision to Diff 81413.
hokein marked 18 inline comments as done.
hokein added a comment.
- Address review comments.
- Add more test cases.
https://reviews.llvm.org/D27673
Files:
clang-move/CMakeLists.txt
clang-move/ClangMove.cpp
clang-move/ClangMove.h
clang-move/UsedHelperDeclFinder.cpp
clang-move/UsedHelperDeclFinder.h
test/clang-move/Inputs/helper_decls_test.cpp
test/clang-move/Inputs/helper_decls_test.h
test/clang-move/Inputs/multiple_class_test.cpp
test/clang-move/move-multiple-classes.cpp
test/clang-move/move-used-helper-decls.cpp
unittests/clang-move/ClangMoveTests.cpp
Index: unittests/clang-move/ClangMoveTests.cpp
===================================================================
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -73,13 +73,21 @@
"\n"
"// comment5\n"
"// comment5\n"
- "void Foo::f() { f1(); }\n"
+ "void Foo::f() {\n"
+ " f1();\n"
+ " kConstInt1;\n"
+ " kConstInt2;\n"
+ " help();\n"
+ "}\n"
"\n"
"/////////////\n"
"// comment //\n"
"/////////////\n"
"int Foo::b = 2;\n"
"int Foo2::f() {\n"
+ " kConstInt1;\n"
+ " kConstInt2;\n"
+ " help();\n"
" f1();\n"
" return 1;\n"
"}\n"
@@ -119,6 +127,9 @@
"}\n"
"\n"
"int Foo2::f() {\n"
+ " kConstInt1;\n"
+ " kConstInt2;\n"
+ " help();\n"
" f1();\n"
" return 1;\n"
"}\n"
@@ -154,6 +165,7 @@
"namespace {\n"
"// comment1.\n"
"void f1() {}\n"
+ "\n"
"/// comment2.\n"
"int kConstInt1 = 0;\n"
"} // namespace\n"
@@ -170,7 +182,12 @@
"\n"
"// comment5\n"
"// comment5\n"
- "void Foo::f() { f1(); }\n"
+ "void Foo::f() {\n"
+ " f1();\n"
+ " kConstInt1;\n"
+ " kConstInt2;\n"
+ " help();\n"
+ "}\n"
"\n"
"/////////////\n"
"// comment //\n"
Index: test/clang-move/move-used-helper-decls.cpp
===================================================================
--- /dev/null
+++ test/clang-move/move-used-helper-decls.cpp
@@ -0,0 +1,195 @@
+// RUN: mkdir -p %T/used-helper-decls
+// RUN: cp %S/Inputs/helper_decls_test* %T/used-helper-decls/
+// RUN: cd %T/used-helper-decls
+
+// RUN: clang-move -names="a::Class1" -new_cc=%T/used-helper-decls/new_helper_decls_test.cpp -new_header=%T/used-helper-decls/new_helper_decls_test.h -old_cc=%T/used-helper-decls/helper_decls_test.cpp -old_header=../used-helper-decls/helper_decls_test.h %T/used-helper-decls/helper_decls_test.cpp -- -std=c++11
+// RUN: FileCheck -input-file=%T/used-helper-decls/new_helper_decls_test.cpp -check-prefix=CHECK-NEW-CLASS1-CPP %s
+// RUN: FileCheck -input-file=%T/used-helper-decls/helper_decls_test.cpp -check-prefix=CHECK-OLD-CLASS1-CPP %s
+
+// CHECK-NEW-CLASS1-CPP: #include "{{.*}}new_helper_decls_test.h"
+// CHECK-NEW-CLASS1-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS1-CPP-NEXT: namespace {
+// CHECK-NEW-CLASS1-CPP-NEXT: void HelperFun1() {}
+// CHECK-NEW-CLASS1-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS1-CPP-NEXT: void HelperFun2() { HelperFun1(); }
+// CHECK-NEW-CLASS1-CPP-NEXT: } // namespace
+// CHECK-NEW-CLASS1-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS1-CPP-NEXT: namespace a {
+// CHECK-NEW-CLASS1-CPP-NEXT: void Class1::f() { HelperFun2(); }
+// CHECK-NEW-CLASS1-CPP-NEXT: } // namespace a
+//
+// CHECK-OLD-CLASS1-CPP: void HelperFun1() {}
+// CHECK-OLD-CLASS1-CPP-NOT: void HelperFun2() { HelperFun1(); }
+// CHECK-OLD-CLASS1-CPP-NOT: void Class1::f() { HelperFun2(); }
+// CHECK-OLD-CLASS1-CPP: void Class2::f() {
+// CHECK-OLD-CLASS1-CPP: HelperFun1();
+
+// RUN: cp %S/Inputs/helper_decls_test* %T/used-helper-decls/
+// RUN: clang-move -names="a::Class2" -new_cc=%T/used-helper-decls/new_helper_decls_test.cpp -new_header=%T/used-helper-decls/new_helper_decls_test.h -old_cc=%T/used-helper-decls/helper_decls_test.cpp -old_header=../used-helper-decls/helper_decls_test.h %T/used-helper-decls/helper_decls_test.cpp -- -std=c++11
+// RUN: FileCheck -input-file=%T/used-helper-decls/new_helper_decls_test.cpp -check-prefix=CHECK-NEW-CLASS2-CPP %s
+// RUN: FileCheck -input-file=%T/used-helper-decls/helper_decls_test.cpp -check-prefix=CHECK-OLD-CLASS2-CPP %s
+
+// CHECK-NEW-CLASS2-CPP: #include "{{.*}}new_helper_decls_test.h"
+// CHECK-NEW-CLASS2-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS2-CPP-NEXT: namespace {
+// CHECK-NEW-CLASS2-CPP-NEXT: void HelperFun1() {}
+// CHECK-NEW-CLASS2-CPP-NEXT: } // namespace
+// CHECK-NEW-CLASS2-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS2-CPP-NEXT: static const int K2 = 2;
+// CHECK-NEW-CLASS2-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS2-CPP-NEXT: static void HelperFun3() { K2; }
+// CHECK-NEW-CLASS2-CPP-NEXT: namespace a {
+// CHECK-NEW-CLASS2-CPP-NEXT: void Class2::f() {
+// CHECK-NEW-CLASS2-CPP-NEXT: HelperFun1();
+// CHECK-NEW-CLASS2-CPP-NEXT: HelperFun3();
+// CHECK-NEW-CLASS2-CPP-NEXT: }
+// CHECK-NEW-CLASS2-CPP-NEXT: } // namespace a
+
+// CHECK-OLD-CLASS2-CPP: void HelperFun1() {}
+// CHECK-OLD-CLASS2-CPP: void HelperFun2() { HelperFun1(); }
+// CHECK-OLD-CLASS2-CPP: const int K1 = 1;
+// CHECK-OLD-CLASS2-CPP: static const int K2 = 2;
+// CHECK-OLD-CLASS2-CPP-NOT: static void HelperFun3() { K2; }
+// CHECK-OLD-CLASS2-CPP-NOT: void Class2::f() {
+// CHECK-OLD-CLASS2-CPP-NOT: HelperFun1();
+// CHECK-OLD-CLASS2-CPP-NOT: HelperFun3();
+// CHECK-OLD-CLASS2-CPP: void Class5::f() {
+// CHECK-OLD-CLASS2-CPP-NEXT: K1;
+// CHECK-OLD-CLASS2-CPP-NEXT: K2;
+
+// RUN: cp %S/Inputs/helper_decls_test* %T/used-helper-decls/
+// RUN: clang-move -names="a::Class3" -new_cc=%T/used-helper-decls/new_helper_decls_test.cpp -new_header=%T/used-helper-decls/new_helper_decls_test.h -old_cc=%T/used-helper-decls/helper_decls_test.cpp -old_header=../used-helper-decls/helper_decls_test.h %T/used-helper-decls/helper_decls_test.cpp -- -std=c++11
+// RUN: FileCheck -input-file=%T/used-helper-decls/new_helper_decls_test.cpp -check-prefix=CHECK-NEW-CLASS3-CPP %s
+// RUN: FileCheck -input-file=%T/used-helper-decls/helper_decls_test.cpp -check-prefix=CHECK-OLD-CLASS3-CPP %s
+
+// CHECK-NEW-CLASS3-CPP: #include "{{.*}}new_helper_decls_test.h"
+// CHECK-NEW-CLASS3-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS3-CPP-NEXT: namespace {
+// CHECK-NEW-CLASS3-CPP-NEXT: class HelperC1 {
+// CHECK-NEW-CLASS3-CPP-NEXT: public:
+// CHECK-NEW-CLASS3-CPP-NEXT: static int I;
+// CHECK-NEW-CLASS3-CPP-NEXT: };
+// CHECK-NEW-CLASS3-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS3-CPP-NEXT: int HelperC1::I = 0;
+// CHECK-NEW-CLASS3-CPP-NEXT: } // namespace
+// CHECK-NEW-CLASS3-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS3-CPP-NEXT: namespace a {
+// CHECK-NEW-CLASS3-CPP-NEXT: void Class3::f() { HelperC1::I; }
+// CHECK-NEW-CLASS3-CPP-NEXT: } // namespace a
+
+// CHECK-OLD-CLASS3-CPP: namespace {
+// CHECK-OLD-CLASS3-CPP-NOT: class HelperC1 {
+// CHECK-OLD-CLASS3-CPP-NOT: public:
+// CHECK-OLD-CLASS3-CPP-NOT: static int I;
+// CHECK-OLD-CLASS3-CPP-NOT: };
+// CHECK-OLD-CLASS3-CPP-NOT: int HelperC1::I = 0;
+// CHECK-OLD-CLASS3-CPP: class HelperC2 {};
+
+
+// RUN: cp %S/Inputs/helper_decls_test* %T/used-helper-decls/
+// RUN: clang-move -names="a::Class4" -new_cc=%T/used-helper-decls/new_helper_decls_test.cpp -new_header=%T/used-helper-decls/new_helper_decls_test.h -old_cc=%T/used-helper-decls/helper_decls_test.cpp -old_header=../used-helper-decls/helper_decls_test.h %T/used-helper-decls/helper_decls_test.cpp -- -std=c++11
+// RUN: FileCheck -input-file=%T/used-helper-decls/new_helper_decls_test.cpp -check-prefix=CHECK-NEW-CLASS4-CPP %s
+// RUN: FileCheck -input-file=%T/used-helper-decls/helper_decls_test.cpp -check-prefix=CHECK-OLD-CLASS4-CPP %s
+
+// CHECK-NEW-CLASS4-CPP: #include "{{.*}}new_helper_decls_test.h"
+// CHECK-NEW-CLASS4-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS4-CPP-NEXT: namespace {
+// CHECK-NEW-CLASS4-CPP-NEXT: class HelperC2 {};
+// CHECK-NEW-CLASS4-CPP-NEXT: } // namespace
+// CHECK-NEW-CLASS4-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS4-CPP-NEXT: namespace a {
+// CHECK-NEW-CLASS4-CPP-NEXT: void Class4::f() { HelperC2 c2; }
+// CHECK-NEW-CLASS4-CPP-NEXT: } // namespace a
+
+// CHECK-OLD-CLASS4-CPP-NOT: class HelperC2 {};
+
+
+// RUN: cp %S/Inputs/helper_decls_test* %T/used-helper-decls/
+// RUN: clang-move -names="a::Class5" -new_cc=%T/used-helper-decls/new_helper_decls_test.cpp -new_header=%T/used-helper-decls/new_helper_decls_test.h -old_cc=%T/used-helper-decls/helper_decls_test.cpp -old_header=../used-helper-decls/helper_decls_test.h %T/used-helper-decls/helper_decls_test.cpp -- -std=c++11
+// RUN: FileCheck -input-file=%T/used-helper-decls/new_helper_decls_test.cpp -check-prefix=CHECK-NEW-CLASS5-CPP %s
+// RUN: FileCheck -input-file=%T/used-helper-decls/helper_decls_test.cpp -check-prefix=CHECK-OLD-CLASS5-CPP %s
+
+// CHECK-NEW-CLASS5-CPP: #include "{{.*}}new_helper_decls_test.h"
+// CHECK-NEW-CLASS5-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS5-CPP-NEXT: namespace {
+// CHECK-NEW-CLASS5-CPP-NEXT: const int K1 = 1;
+// CHECK-NEW-CLASS5-CPP-NEXT: } // namespace
+// CHECK-NEW-CLASS5-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS5-CPP-NEXT: static const int K2 = 2;
+// CHECK-NEW-CLASS5-CPP-NEXT: namespace a {
+// CHECK-NEW-CLASS5-CPP-NEXT: static const int K3 = 3;
+// CHECK-NEW-CLASS5-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS5-CPP-NEXT: static void HelperFun4() {}
+// CHECK-NEW-CLASS5-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS5-CPP-NEXT: void Class5::f() {
+// CHECK-NEW-CLASS5-CPP-NEXT: K1;
+// CHECK-NEW-CLASS5-CPP-NEXT: K2;
+// CHECK-NEW-CLASS5-CPP-NEXT: K3;
+// CHECK-NEW-CLASS5-CPP-NEXT: HelperFun4();
+// CHECK-NEW-CLASS5-CPP-NEXT: }
+// CHECK-NEW-CLASS5-CPP-NEXT: } // namespace a
+
+// CHECK-OLD-CLASS5-CPP-NOT: const int K1 = 1;
+// CHECK-OLD-CLASS5-CPP: static const int K2 = 2;
+// CHECK-OLD-CLASS5-CPP: static void HelperFun3() { K2; }
+// CHECK-OLD-CLASS5-CPP-NOT: void Class5::f() {
+
+
+// RUN: cp %S/Inputs/helper_decls_test* %T/used-helper-decls/
+// RUN: clang-move -names="a::Class6" -new_cc=%T/used-helper-decls/new_helper_decls_test.cpp -new_header=%T/used-helper-decls/new_helper_decls_test.h -old_cc=%T/used-helper-decls/helper_decls_test.cpp -old_header=../used-helper-decls/helper_decls_test.h %T/used-helper-decls/helper_decls_test.cpp -- -std=c++11
+// RUN: FileCheck -input-file=%T/used-helper-decls/new_helper_decls_test.cpp -check-prefix=CHECK-NEW-CLASS6-CPP %s
+// RUN: FileCheck -input-file=%T/used-helper-decls/helper_decls_test.cpp -check-prefix=CHECK-OLD-CLASS6-CPP %s
+
+// CHECK-NEW-CLASS6-CPP: #include "{{.*}}new_helper_decls_test.h"
+// CHECK-NEW-CLASS6-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS6-CPP-NEXT: namespace {
+// CHECK-NEW-CLASS6-CPP-NEXT: class HelperC3 {
+// CHECK-NEW-CLASS6-CPP-NEXT: public:
+// CHECK-NEW-CLASS6-CPP-NEXT: static int I;
+// CHECK-NEW-CLASS6-CPP-NEXT: };
+// CHECK-NEW-CLASS6-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS6-CPP-NEXT: int HelperC3::I = 0;
+// CHECK-NEW-CLASS6-CPP-NEXT: } // namespace
+// CHECK-NEW-CLASS6-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS6-CPP-NEXT: namespace a {
+// CHECK-NEW-CLASS6-CPP-NEXT: static const int K4 = HelperC3::I;
+// CHECK-NEW-CLASS6-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-CLASS6-CPP-NEXT: void Class6::f() { K4; }
+// CHECK-NEW-CLASS6-CPP-NEXT: } // namespace a
+
+// CHECK-OLD-CLASS6-CPP-NOT: class HelperC3 {
+// CHECK-OLD-CLASS6-CPP-NOT: int HelperC3::I = 0;
+// CHECK-OLD-CLASS6-CPP-NOT: static const int K4 = HelperC3::I;
+
+
+// RUN: cp %S/Inputs/helper_decls_test* %T/used-helper-decls/
+// RUN: clang-move -names="a::Fun1" -new_cc=%T/used-helper-decls/new_helper_decls_test.cpp -new_header=%T/used-helper-decls/new_helper_decls_test.h -old_cc=%T/used-helper-decls/helper_decls_test.cpp -old_header=../used-helper-decls/helper_decls_test.h %T/used-helper-decls/helper_decls_test.cpp -- -std=c++11
+// RUN: FileCheck -input-file=%T/used-helper-decls/new_helper_decls_test.cpp -check-prefix=CHECK-NEW-FUN1-CPP %s
+// RUN: FileCheck -input-file=%T/used-helper-decls/helper_decls_test.cpp -check-prefix=CHECK-OLD-FUN1-CPP %s
+
+// CHECK-NEW-FUN1-CPP: #include "{{.*}}new_helper_decls_test.h"
+// CHECK-NEW-FUN1-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-FUN1-CPP-NEXT: namespace a {
+// CHECK-NEW-FUN1-CPP-NEXT: static void HelperFun5() {}
+// CHECK-NEW-FUN1-CPP-SAME: {{[[:space:]]}}
+// CHECK-NEW-FUN1-CPP-NEXT: void Fun1() { HelperFun5(); }
+// CHECK-NEW-FUN1-CPP-NEXT: } // namespace a
+
+// CHECK-OLD-FUN1-CPP-NOT: static void HelperFun5() {}
+// CHECK-OLD-FUN1-CPP-NOT: void Fun1() { HelperFun5(); }
+
+
+// RUN: cp %S/Inputs/helper_decls_test* %T/used-helper-decls/
+// RUN: clang-move -names="a::Fun2" -new_cc=%T/used-helper-decls/new_helper_decls_test.cpp -new_header=%T/used-helper-decls/new_helper_decls_test.h -old_cc=%T/used-helper-decls/helper_decls_test.cpp -old_header=../used-helper-decls/helper_decls_test.h %T/used-helper-decls/helper_decls_test.cpp -- -std=c++11
+// RUN: FileCheck -input-file=%T/used-helper-decls/new_helper_decls_test.cpp -check-prefix=CHECK-NEW-FUN2-CPP %s
+// RUN: FileCheck -input-file=%T/used-helper-decls/new_helper_decls_test.h -check-prefix=CHECK-NEW-FUN2-H %s
+// RUN: FileCheck -input-file=%T/used-helper-decls/helper_decls_test.h -check-prefix=CHECK-OLD-FUN2-H %s
+
+// CHECK-NEW-FUN2-H: namespace a {
+// CHECK-NEW-FUN2-H-NEXT: inline void Fun2() {}
+// CHECK-NEW-FUN2-H-NEXT: } // namespace a
+
+// CHECK-NEW-FUN2-CPP: #include "{{.*}}new_helper_decls_test.h"
+// CHECK-NEW-FUN2-CPP-SAME: {{[[:space:]]}}
+
+// CHECK-OLD-FUN2-H-NOT: inline void Fun2() {}
Index: test/clang-move/move-multiple-classes.cpp
===================================================================
--- test/clang-move/move-multiple-classes.cpp
+++ test/clang-move/move-multiple-classes.cpp
@@ -25,17 +25,16 @@
// CHECK-OLD-TEST-CPP: namespace {
// CHECK-OLD-TEST-CPP: using a::Move1;
// CHECK-OLD-TEST-CPP: using namespace a;
-// CHECK-OLD-TEST-CPP: static int k = 0;
-// CHECK-OLD-TEST-CPP: } // anonymous namespace
+// CHECK-OLD-TEST-CPP: } // namespace
// CHECK-OLD-TEST-CPP: namespace b {
// CHECK-OLD-TEST-CPP: using a::Move1;
// CHECK-OLD-TEST-CPP: using namespace a;
// CHECK-OLD-TEST-CPP: using T = a::Move1;
// CHECK-OLD-TEST-CPP: } // namespace b
// CHECK-OLD-TEST-CPP: namespace c {
// CHECK-OLD-TEST-CPP: int NoMove::f() {
// CHECK-OLD-TEST-CPP: static int F = 0;
-// CHECK-OLD-TEST-CPP: return 0;
+// CHECK-OLD-TEST-CPP: return g;
// CHECK-OLD-TEST-CPP: }
// CHECK-OLD-TEST-CPP: } // namespace c
@@ -85,7 +84,7 @@
// CHECK-NEW-TEST-CPP: using a::Move1;
// CHECK-NEW-TEST-CPP: using namespace a;
// CHECK-NEW-TEST-CPP: static int k = 0;
-// CHECK-NEW-TEST-CPP: } // anonymous namespace
+// CHECK-NEW-TEST-CPP: } // namespace
// CHECK-NEW-TEST-CPP: namespace b {
// CHECK-NEW-TEST-CPP: using a::Move1;
// CHECK-NEW-TEST-CPP: using namespace a;
@@ -98,8 +97,8 @@
// CHECK-NEW-TEST-CPP: using namespace b;
// CHECK-NEW-TEST-CPP: return 0;
// CHECK-NEW-TEST-CPP: }
-// CHECK-NEW-TEST-CPP: int Move4::f() { return 0; }
+// CHECK-NEW-TEST-CPP: int Move4::f() { return k; }
// CHECK-NEW-TEST-CPP: int EnclosingMove5::a = 1;
-// CHECK-NEW-TEST-CPP: int EnclosingMove5::Nested::f() { return 0; }
+// CHECK-NEW-TEST-CPP: int EnclosingMove5::Nested::f() { return g; }
// CHECK-NEW-TEST-CPP: int EnclosingMove5::Nested::b = 1;
// CHECK-NEW-TEST-CPP: } // namespace c
Index: test/clang-move/Inputs/multiple_class_test.cpp
===================================================================
--- test/clang-move/Inputs/multiple_class_test.cpp
+++ test/clang-move/Inputs/multiple_class_test.cpp
@@ -15,7 +15,7 @@
using a::Move1;
using namespace a;
static int k = 0;
-} // anonymous namespace
+} // namespace
namespace b {
using a::Move1;
@@ -34,19 +34,19 @@
}
int Move4::f() {
- return 0;
+ return k;
}
int EnclosingMove5::a = 1;
int EnclosingMove5::Nested::f() {
- return 0;
+ return g;
}
int EnclosingMove5::Nested::b = 1;
int NoMove::f() {
static int F = 0;
- return 0;
+ return g;
}
} // namespace c
Index: test/clang-move/Inputs/helper_decls_test.h
===================================================================
--- /dev/null
+++ test/clang-move/Inputs/helper_decls_test.h
@@ -0,0 +1,30 @@
+namespace a {
+class Class1 {
+ void f();
+};
+
+class Class2 {
+ void f();
+};
+
+class Class3 {
+ void f();
+};
+
+class Class4 {
+ void f();
+};
+
+class Class5 {
+ void f();
+};
+
+class Class6 {
+ void f();
+};
+
+void Fun1();
+
+inline void Fun2() {}
+
+} // namespace a
Index: test/clang-move/Inputs/helper_decls_test.cpp
===================================================================
--- /dev/null
+++ test/clang-move/Inputs/helper_decls_test.cpp
@@ -0,0 +1,62 @@
+#include "helper_decls_test.h"
+
+namespace {
+class HelperC1 {
+public:
+ static int I;
+};
+
+int HelperC1::I = 0;
+
+class HelperC2 {};
+
+class HelperC3 {
+ public:
+ static int I;
+};
+
+int HelperC3::I = 0;
+
+void HelperFun1() {}
+
+void HelperFun2() { HelperFun1(); }
+
+const int K1 = 1;
+} // namespace
+
+static const int K2 = 2;
+static void HelperFun3() { K2; }
+
+namespace a {
+
+static const int K3 = 3;
+static const int K4 = HelperC3::I;
+
+static void HelperFun4() {}
+
+void Class1::f() { HelperFun2(); }
+
+void Class2::f() {
+ HelperFun1();
+ HelperFun3();
+}
+
+void Class3::f() { HelperC1::I; }
+
+void Class4::f() { HelperC2 c2; }
+
+void Class5::f() {
+ K1;
+ K2;
+ K3;
+ HelperFun4();
+}
+
+void Class6::f() {
+ K4;
+}
+
+static void HelperFun5() {}
+
+void Fun1() { HelperFun5(); }
+} // namespace a
Index: clang-move/UsedHelperDeclFinder.h
===================================================================
--- /dev/null
+++ clang-move/UsedHelperDeclFinder.h
@@ -0,0 +1,119 @@
+//===-- UsedHelperDeclFinder.h - AST-based call graph for helper decls ----===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_MOVE_USED_HELPER_DECL_FINDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_MOVE_USED_HELPER_DECL_FINDER_H
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Analysis/CallGraph.h"
+#include "llvm/ADT/DenseSet.h"
+#include <memory>
+#include <vector>
+
+namespace clang {
+namespace move {
+
+// Customized call graph for helper declarations in a single translation unit
+// (e.g. old.cc). We don't reuse CallGraph in clang/Analysis because the
+// CallGraph there only supports function declarations.
+//
+// Helper declarations include following types:
+// * function/variable/class definitions in an anonymous namespace.
+// * static function/variable definitions in a global/named namespace.
+//
+// The call graph is a directed graph. Each node in the graph represents a
+// helper declaration in old.cc or a non-moved/moved declaration (e.g. class,
+// function) in old.h, which means each node is associated with a Decl.
+//
+// To construct the graph, we use AST matcher to find interested Decls (usually
+// a pair of Caller and Callee), and add an edge from the Caller node to the
+// Callee node.
+//
+// Specially, for a class declaration, it might have multiple Decls for its
+// member methods/variables definitions, we only use a single node to present
+// this class, and this node is associate with the class declarations
+// (CXXRecordDecl in the implementation).
+//
+// Note that for a function where its declaration is in old.h and its
+// definition is in old.cc, the representing graph node for this function is
+// associated with the FunctionDecl in old.h, because there are two
+// different FunctionDecl pointers in the AST implementation, we want to make it
+// consistent.
+//
+// The graph has 3 types of edges:
+// 1. moved_decl => helper_decl
+// 2. non_moved_decl => helper_decl
+// 3. helper_decl => helper_decl
+class HelperDeclCallGraph {
+public:
+ HelperDeclCallGraph() = default;
+ ~HelperDeclCallGraph() = default;
+
+ // Add a directed edge from the caller node to the callee node.
+ // A new node will be created if the node for Caller/Callee isn't existed.
+ //
+ // Note that, all methods/variables declarations of a class are represented by
+ // a single node in the graph. The corresponding Decl of this node is the
+ // class declaration.
+ void addNodeForDecl(const Decl *Caller, const Decl *Callee);
+ CallGraphNode *getNode(const Decl *D) const;
+
+ // Get all reachable nodes in the graph from the given declaration D's node,
+ // including D.
+ llvm::DenseSet<const CallGraphNode *> getReachableNodes(const Decl *D) const;
+
+ // Dump the call graph for debug purpose.
+ void dump() const;
+
+private:
+ void print(raw_ostream &OS) const;
+ // Lookup a node for the given declaration D. If not found, insert a new
+ // node into the graph.
+ CallGraphNode *getOrInsertNode(Decl *D);
+
+ typedef llvm::DenseMap<const Decl *, std::unique_ptr<CallGraphNode>>
+ DeclMapTy;
+
+ // DeclMap owns all CallGraphNodes.
+ DeclMapTy DeclMap;
+};
+
+// A builder helps to construct a call graph of helper declarations.
+class HelperDeclCGBuilder : public ast_matchers::MatchFinder::MatchCallback {
+public:
+ HelperDeclCGBuilder() : CG(new HelperDeclCallGraph) {}
+ void run(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ const HelperDeclCallGraph *getCallGraph() const { return CG.get(); }
+
+private:
+ std::unique_ptr<HelperDeclCallGraph> CG;
+};
+
+class UsedHelperDeclFinder {
+public:
+ UsedHelperDeclFinder(const HelperDeclCallGraph *CG) : CG(CG) {}
+
+ typedef llvm::DenseSet<const Decl *> HelperDeclsSet;
+
+ // Return a set of all helper declarations which are used/referenced by the
+ // given Decls (The Decls can contain class method/variable declarations).
+ HelperDeclsSet
+ getUsedHelperDecls(const std::vector<const NamedDecl *> &Decls) const;
+
+ // Check whether the given declaration D is in the HelperDeclsSet.
+ static bool isUsed(const Decl *D, const HelperDeclsSet &UsedDecls);
+
+private:
+ const HelperDeclCallGraph *CG; // Not owned.
+};
+
+} // namespace move
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_MOVE_USED_HELPER_DECL_FINDER_H
Index: clang-move/UsedHelperDeclFinder.cpp
===================================================================
--- /dev/null
+++ clang-move/UsedHelperDeclFinder.cpp
@@ -0,0 +1,152 @@
+//===-- UsedHelperDeclFinder.cpp - AST-based call graph for helper decls --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UsedHelperDeclFinder.h"
+#include "ClangMove.h"
+#include "clang/AST/Decl.h"
+#include <vector>
+
+namespace clang {
+namespace move {
+namespace {
+// For a given Declaration (e.g. CXXMethodDecl, FuncionDecl), find out its
+// outmost eclosing class declaration or function declaration if exists.
+// Because we consider that all method declarations in a class are represented
+// by a single node which belongs to that class.
+const Decl *getOutmostEnclosingClassOrFunDecl(const Decl *D) {
+ const auto *DC = D->getDeclContext();
+ const Decl *Result = D;
+ while (DC) {
+ if (const auto *RD = dyn_cast<CXXRecordDecl>(DC))
+ Result = RD;
+ else if (const auto *FD = dyn_cast<FunctionDecl>(DC)) {
+ Result = FD;
+ if (const auto *RD = dyn_cast<CXXRecordDecl>(FD->getParent()))
+ Result = RD;
+ }
+ DC = DC->getParent();
+ }
+ return Result;
+}
+} // namespace
+
+void HelperDeclCallGraph::print(raw_ostream &OS) const {
+ OS << " --- Call graph Dump --- \n";
+ for (auto I = DeclMap.begin(); I != DeclMap.end(); ++I) {
+ const CallGraphNode *N = (I->second).get();
+
+ OS << " Declarations: ";
+ N->print(OS);
+
+ OS << " calls: ";
+ for (auto CI = N->begin(), CE = N->end(); CI != CE; ++CI) {
+ (*CI)->print(OS);
+ OS << " ";
+ }
+ OS << '\n';
+ }
+ OS.flush();
+}
+
+void HelperDeclCallGraph::addNodeForDecl(const Decl *Caller,
+ const Decl *Callee) {
+ assert(Caller);
+ assert(Callee);
+
+ // Allocate a new node, mark it as root, and process it's calls.
+ CallGraphNode *CallerNode = getOrInsertNode(const_cast<Decl *>(Caller));
+ CallGraphNode *CalleeNode = getOrInsertNode(const_cast<Decl *>(Callee));
+ CallerNode->addCallee(CalleeNode);
+}
+
+void HelperDeclCallGraph::dump() const { print(llvm::dbgs()); }
+
+CallGraphNode *HelperDeclCallGraph::getOrInsertNode(Decl *F) {
+ std::unique_ptr<CallGraphNode> &Node = DeclMap[F];
+ if (Node)
+ return Node.get();
+
+ Node = llvm::make_unique<CallGraphNode>(F);
+ return Node.get();
+}
+
+CallGraphNode *HelperDeclCallGraph::getNode(const Decl *D) const {
+ auto I = DeclMap.find(D);
+ return I == DeclMap.end() ? nullptr : I->second.get();
+}
+
+llvm::DenseSet<const CallGraphNode *>
+HelperDeclCallGraph::getReachableNodes(const Decl *Root) const {
+ const auto *RootNode = getNode(Root);
+ if (!RootNode)
+ return {};
+ llvm::DenseSet<const CallGraphNode *> ConnectedNodes;
+ std::function<void(const CallGraphNode *)> VisitNode =
+ [&](const CallGraphNode *Node) {
+ if (ConnectedNodes.count(Node))
+ return;
+ ConnectedNodes.insert(Node);
+ for (auto It = Node->begin(), End = Node->end(); It != End; ++It)
+ VisitNode(*It);
+ };
+
+ VisitNode(RootNode);
+ return ConnectedNodes;
+}
+
+UsedHelperDeclFinder::HelperDeclsSet UsedHelperDeclFinder::getUsedHelperDecls(
+ const std::vector<const NamedDecl *> &Decls) const {
+ llvm::DenseSet<const CallGraphNode *> Nodes;
+ for (const auto &RD : Decls) {
+ llvm::DenseSet<const CallGraphNode *> Result =
+ CG->getReachableNodes(getOutmostEnclosingClassOrFunDecl(RD));
+ Nodes.insert(Result.begin(), Result.end());
+ }
+ HelperDeclsSet Results;
+ for (const auto *Node : Nodes)
+ Results.insert(Node->getDecl());
+ return Results;
+}
+
+bool UsedHelperDeclFinder::isUsed(const Decl *D, const HelperDeclsSet &Decls) {
+ assert(D);
+ const Decl *UplevelDecl = getOutmostEnclosingClassOrFunDecl(D);
+ return Decls.find(UplevelDecl) != Decls.end();
+}
+
+void HelperDeclCGBuilder::run(
+ const ast_matchers::MatchFinder::MatchResult &Result) {
+ if (const clang::DeclRefExpr *FR =
+ Result.Nodes.getNodeAs<clang::DeclRefExpr>("func_ref")) {
+ const auto *DC = Result.Nodes.getNodeAs<clang::Decl>("dc");
+ if (!DC)
+ return;
+ // Try to get the its previous declaration, normally the declartion without
+ // definition.
+ const auto *T = DC->getPreviousDecl();
+ if (!T)
+ T = DC;
+ CG->addNodeForDecl(getOutmostEnclosingClassOrFunDecl(T),
+ getOutmostEnclosingClassOrFunDecl(FR->getDecl()));
+
+ } else if (const auto *T =
+ Result.Nodes.getNodeAs<CXXRecordDecl>("used_class")) {
+ const auto *D = getOutmostEnclosingClassOrFunDecl(
+ Result.Nodes.getNodeAs<clang::Decl>("dc"));
+ // In general, caller and callee are not the same Decls.For static class
+ // member definitions like "int CLASS::static_var = 1;", its DC is a VarDel
+ // whose outmost enclosing declaration is the "CLASS" CXXRecord Declaration.
+ if (T == D)
+ return;
+ CG->addNodeForDecl(D, T);
+ }
+}
+
+} // namespace move
+} // namespace clang
Index: clang-move/ClangMove.h
===================================================================
--- clang-move/ClangMove.h
+++ clang-move/ClangMove.h
@@ -10,6 +10,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_MOVE_CLANGMOVE_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_MOVE_CLANGMOVE_H
+#include "UsedHelperDeclFinder.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Frontend/FrontendAction.h"
#include "clang/Tooling/Core/Replacement.h"
@@ -88,11 +89,18 @@
};
// This tool is used to move class/function definitions from the given source
-// files (old.h/cc) to new files (new.h/cc). When moving a class, all its
-// members are also moved. In addition, all helper functions (anonymous
-// namespace declarations, static declarations, using declarations) in old.cc
-// and forward class declarations in old.h are copied to the new files.
-// The goal of this tool is to make the new files as compliable as possible.
+// files (old.h/cc) to new files (new.h/cc).
+// The goal of this tool is to make the new/old files as compilable as possible.
+//
+// When moving a symbol,all used helper declarations (e.g. static
+// functions/variables definitions in global/named namespace,
+// functions/variables/classes definitions in anonymous namespace) used by the
+// moved symbol in old.cc are moved to the new.cc. In addition, all
+// using-declarations in old.cc are also moved to new.cc; forward class
+// declarations in old.h are also moved to new.h.
+//
+// The remaining helper declarations which are unused by non-moved symbols in
+// old.cc will be removed.
//
// Note: When all declarations in old header are being moved, all code in
// old.h/cc will be moved, which means old.h/cc are empty. This ignores symbols
@@ -148,15 +156,21 @@
// Stores all MatchCallbacks created by this tool.
std::vector<std::unique_ptr<ast_matchers::MatchFinder::MatchCallback>>
MatchCallbacks;
- // All declarations (the class decl being moved, forward decls) that need to
- // be moved/copy to the new files, saving in an AST-visited order.
+ // Store all potential declarations (decls being moved, forward decls) that
+ // might need to move to new.h/cc. It includes all helper declarations
+ // (include unused ones) by default. The unused ones will be filtered out in
+ // the last stage. Saving in an AST-visited order.
std::vector<const NamedDecl *> MovedDecls;
// The declarations that needs to be removed in old.cc/h.
std::vector<const NamedDecl *> RemovedDecls;
// The #includes in old_header.h.
std::vector<std::string> HeaderIncludes;
// The #includes in old_cc.cc.
std::vector<std::string> CCIncludes;
+ // Records all helper declarations (function/variable/class definitions in
+ // anonymous namespaces, static function/variable definitions in global/named
+ // namespaces) in old.cc. saving in an AST-visited order.
+ std::vector<const NamedDecl *> HelperDeclarations;
// The unmoved named declarations in old header.
llvm::SmallPtrSet<const NamedDecl*, 8> UnremovedDeclsInOldHeader;
/// The source range for the written file name in #include (i.e. "old.h" for
@@ -170,6 +184,8 @@
ClangMoveContext *const Context;
/// A reporter to report all declarations from old header. It is not owned.
DeclarationReporter *const Reporter;
+ /// Builder for helper declarations call graph.
+ HelperDeclCGBuilder CGBuilder;
};
class ClangMoveAction : public clang::ASTFrontendAction {
Index: clang-move/ClangMove.cpp
===================================================================
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -8,6 +8,7 @@
//===----------------------------------------------------------------------===//
#include "ClangMove.h"
+#include "UsedHelperDeclFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Format/Format.h"
@@ -455,24 +456,17 @@
//============================================================================
// Matchers for old cc
//============================================================================
- auto InOldCCNamedOrGlobalNamespace =
- allOf(hasParent(decl(anyOf(namespaceDecl(unless(isAnonymous())),
- translationUnitDecl()))),
- InOldCC);
- // Matching using decls/type alias decls which are in named namespace or
- // global namespace. Those in classes, functions and anonymous namespaces are
- // covered in other matchers.
+ auto InOldCCNamespace = allOf(
+ hasParent(decl(anyOf(namespaceDecl(), translationUnitDecl()))), InOldCC);
+ // Matching using decls/type alias decls which are in named/anonymous/global
+ // namespace. Those in classes, functions are covered in other matchers.
Finder->addMatcher(
- namedDecl(anyOf(usingDecl(InOldCCNamedOrGlobalNamespace),
- usingDirectiveDecl(InOldCCNamedOrGlobalNamespace),
- typeAliasDecl( InOldCCNamedOrGlobalNamespace)))
+ namedDecl(anyOf(usingDecl(InOldCCNamespace),
+ usingDirectiveDecl(InOldCCNamespace),
+ typeAliasDecl(InOldCCNamespace)))
.bind("using_decl"),
this);
- // Match anonymous namespace decl in old cc.
- Finder->addMatcher(namespaceDecl(isAnonymous(), InOldCC).bind("anonymous_ns"),
- this);
-
// Match static functions/variable definitions which are defined in named
// namespaces.
Optional<ast_matchers::internal::Matcher<NamedDecl>> HasAnySymbolNames;
@@ -489,13 +483,36 @@
}
auto InMovedClass =
hasOutermostEnclosingClass(cxxRecordDecl(*HasAnySymbolNames));
- auto IsOldCCStaticDefinition =
- allOf(isDefinition(), unless(InMovedClass), InOldCCNamedOrGlobalNamespace,
- isStaticStorageClass());
- Finder->addMatcher(namedDecl(anyOf(functionDecl(IsOldCCStaticDefinition),
- varDecl(IsOldCCStaticDefinition)))
- .bind("static_decls"),
- this);
+
+ // Matchers for helper declarations in old.cc.
+ auto IsOldCCHelperDefinition = allOf(
+ isDefinition(), unless(InMovedClass), InOldCC,
+ anyOf(isStaticStorageClass(), hasParent(namespaceDecl(isAnonymous()))));
+ auto HelperFuncOrVar = namedDecl(anyOf(functionDecl(IsOldCCHelperDefinition),
+ varDecl(IsOldCCHelperDefinition)));
+ auto HelperClasses =
+ cxxRecordDecl(isDefinition(), unless(InMovedClass), InOldCC,
+ hasParent(namespaceDecl(isAnonymous())));
+ // Save all helper declarations in old.cc.
+ Finder->addMatcher(
+ namedDecl(anyOf(HelperFuncOrVar, HelperClasses)).bind("helper_decls"),
+ this);
+
+ // Construct an AST-based call graph of helper declarations in old.cc.
+ // In the following matcheres, "dc" is a caller while "helper_decls" and
+ // "used_class" is a callee, so a new edge starting from caller to callee will
+ // be add in the graph.
+ //
+ // Find helper function/variable usages.
+ Finder->addMatcher(
+ declRefExpr(to(HelperFuncOrVar), hasAncestor(decl().bind("dc")))
+ .bind("func_ref"),
+ &CGBuilder);
+ // Find helper class usages.
+ Finder->addMatcher(
+ typeLoc(loc(recordType(hasDeclaration(HelperClasses.bind("used_class")))),
+ hasAncestor(decl().bind("dc"))),
+ &CGBuilder);
//============================================================================
// Matchers for old files, including old.h/old.cc
@@ -543,12 +560,13 @@
else
MovedDecls.push_back(FWD);
}
- } else if (const auto *ANS =
- Result.Nodes.getNodeAs<clang::NamespaceDecl>("anonymous_ns")) {
- MovedDecls.push_back(ANS);
} else if (const auto *ND =
Result.Nodes.getNodeAs<clang::NamedDecl>("static_decls")) {
MovedDecls.push_back(ND);
+ } else if (const auto *ND =
+ Result.Nodes.getNodeAs<clang::NamedDecl>("helper_decls")) {
+ MovedDecls.push_back(ND);
+ HelperDeclarations.push_back(ND);
} else if (const auto *UD =
Result.Nodes.getNodeAs<clang::NamedDecl>("using_decl")) {
MovedDecls.push_back(UD);
@@ -567,9 +585,6 @@
SmallVector<char, 128> HeaderWithSearchPath;
llvm::sys::path::append(HeaderWithSearchPath, SearchPath, IncludeHeader);
std::string AbsoluteOldHeader = makeAbsolutePath(Context->Spec.OldHeader);
- // FIXME: Add old.h to the new.cc/h when the new target has dependencies on
- // old.h/c. For instance, when moved class uses another class defined in
- // old.h, the old.h should be added in new.h.
if (AbsoluteOldHeader ==
MakeAbsolutePath(SM, llvm::StringRef(HeaderWithSearchPath.data(),
HeaderWithSearchPath.size()))) {
@@ -591,6 +606,30 @@
void ClangMoveTool::removeDeclsInOldFiles() {
if (RemovedDecls.empty()) return;
+
+ // If old_header is not specified (only move declarations from old.cc), remain
+ // all the helper function declarations in old.cc as UnremovedDeclsInOldHeader
+ // is empty in this case, there is no way to verify unused/used helper
+ // declarations.
+ if (!Context->Spec.OldHeader.empty()) {
+ std::vector<const NamedDecl *> UnremovedDecls;
+ for (const auto *D : UnremovedDeclsInOldHeader)
+ UnremovedDecls.push_back(D);
+
+ UsedHelperDeclFinder HelperDeclFinder(CGBuilder.getCallGraph());
+ // Find all helper declarations in old.cc which are used in unremoved
+ // declarations.
+ UsedHelperDeclFinder::HelperDeclsSet HelpersUsedInOldCC =
+ HelperDeclFinder.getUsedHelperDecls(UnremovedDecls);
+
+ // We remove the helper declarations which are not used in the old.cc after
+ // moving the given declarations.
+ for (const auto *D : HelperDeclarations) {
+ if (!UsedHelperDeclFinder::isUsed(D, HelpersUsedInOldCC))
+ RemovedDecls.push_back(D);
+ }
+ }
+
for (const auto *RemovedDecl : RemovedDecls) {
const auto &SM = RemovedDecl->getASTContext().getSourceManager();
auto Range = getFullRange(RemovedDecl);
@@ -650,6 +689,21 @@
NewCCDecls.push_back(MovedDecl);
}
+ UsedHelperDeclFinder HelperDeclFinder(CGBuilder.getCallGraph());
+ llvm::DenseSet<const Decl *> HelpersUsedInNewCC =
+ HelperDeclFinder.getUsedHelperDecls(RemovedDecls);
+ std::vector<const NamedDecl *> RealNewCCDecls;
+ for (const auto &D : NewCCDecls) {
+ // Filter out unused helper declarations.
+ // We only move the used helper declarations (including transversely used
+ // helpers) and the given symbols being moved.
+ if (llvm::is_contained(HelperDeclarations, D) &&
+ !UsedHelperDeclFinder::isUsed(D, HelpersUsedInNewCC))
+ continue;
+
+ RealNewCCDecls.push_back(D);
+ }
+
if (!Context->Spec.NewHeader.empty()) {
std::string OldHeaderInclude =
Context->Spec.NewDependOnOld
@@ -662,7 +716,8 @@
}
if (!Context->Spec.NewCC.empty())
Context->FileToReplacements[Context->Spec.NewCC] =
- createInsertedReplacements(CCIncludes, NewCCDecls, Context->Spec.NewCC);
+ createInsertedReplacements(CCIncludes, RealNewCCDecls,
+ Context->Spec.NewCC);
}
// Move all contents from OldFile to NewFile.
Index: clang-move/CMakeLists.txt
===================================================================
--- clang-move/CMakeLists.txt
+++ clang-move/CMakeLists.txt
@@ -4,8 +4,10 @@
add_clang_library(clangMove
ClangMove.cpp
+ UsedHelperDeclFinder.cpp
LINK_LIBS
+ clangAnalysis
clangAST
clangASTMatchers
clangBasic
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
