[clang-move] Handle helpers with forward declarations.

Reviewers: ioeric

Reviewed By: ioeric

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D28801

llvm-svn: 292215
diff --git a/clang-tools-extra/clang-move/ClangMove.cpp b/clang-tools-extra/clang-move/ClangMove.cpp
index e1f20b7..f43f278 100644
--- a/clang-tools-extra/clang-move/ClangMove.cpp
+++ b/clang-tools-extra/clang-move/ClangMove.cpp
@@ -553,16 +553,22 @@
 
   // Matchers for helper declarations in old.cc.
   auto InAnonymousNS = hasParent(namespaceDecl(isAnonymous()));
-  auto DefinitionInOldCC = allOf(isDefinition(), unless(InMovedClass), InOldCC);
-  auto IsOldCCHelperDefinition =
-      allOf(DefinitionInOldCC, anyOf(isStaticStorageClass(), InAnonymousNS));
+  auto NotInMovedClass= allOf(unless(InMovedClass), InOldCC);
+  auto IsOldCCHelper =
+      allOf(NotInMovedClass, anyOf(isStaticStorageClass(), InAnonymousNS));
   // Match helper classes separately with helper functions/variables since we
   // want to reuse these matchers in finding helpers usage below.
-  auto HelperFuncOrVar =
-      namedDecl(notInMacro(), anyOf(functionDecl(IsOldCCHelperDefinition),
-                                    varDecl(IsOldCCHelperDefinition)));
+  //
+  // There could be forward declarations usage for helpers, especially for
+  // classes and functions. We need include these forward declarations.
+  //
+  // Forward declarations for variable helpers will be excluded as these
+  // declarations (with "extern") are not supposed in cpp file.
+   auto HelperFuncOrVar =
+      namedDecl(notInMacro(), anyOf(functionDecl(IsOldCCHelper),
+                                    varDecl(isDefinition(), IsOldCCHelper)));
   auto HelperClasses =
-      cxxRecordDecl(notInMacro(), DefinitionInOldCC, InAnonymousNS);
+      cxxRecordDecl(notInMacro(), NotInMovedClass, InAnonymousNS);
   // Save all helper declarations in old.cc.
   Finder->addMatcher(
       namedDecl(anyOf(HelperFuncOrVar, HelperClasses)).bind("helper_decls"),
@@ -650,6 +656,8 @@
                  Result.Nodes.getNodeAs<clang::NamedDecl>("helper_decls")) {
     MovedDecls.push_back(ND);
     HelperDeclarations.push_back(ND);
+    DEBUG(llvm::dbgs() << "Add helper : "
+                       << ND->getNameAsString() << " (" << ND << ")\n");
   } else if (const auto *UD =
                  Result.Nodes.getNodeAs<clang::NamedDecl>("using_decl")) {
     MovedDecls.push_back(UD);
@@ -703,9 +711,12 @@
     // We remove the helper declarations which are not used in the old.cc after
     // moving the given declarations.
     for (const auto *D : HelperDeclarations) {
-      if (!UsedDecls.count(HelperDeclRGBuilder::getOutmostClassOrFunDecl(D))) {
+      DEBUG(llvm::dbgs() << "Check helper is used: "
+                         << D->getNameAsString() << " (" << D << ")\n");
+      if (!UsedDecls.count(HelperDeclRGBuilder::getOutmostClassOrFunDecl(
+              D->getCanonicalDecl()))) {
         DEBUG(llvm::dbgs() << "Helper removed in old.cc: "
-                           << D->getNameAsString() << " " << D << "\n");
+                           << D->getNameAsString() << " (" << D << ")\n");
         RemovedDecls.push_back(D);
       }
     }
@@ -781,7 +792,8 @@
   // given symbols being moved.
   for (const auto *D : NewCCDecls) {
     if (llvm::is_contained(HelperDeclarations, D) &&
-        !UsedDecls.count(HelperDeclRGBuilder::getOutmostClassOrFunDecl(D)))
+        !UsedDecls.count(HelperDeclRGBuilder::getOutmostClassOrFunDecl(
+            D->getCanonicalDecl())))
       continue;
 
     DEBUG(llvm::dbgs() << "Helper used in new.cc: " << D->getNameAsString()