[clang-move] Ignore using decls which are defined in macros.

[hokein]

Summary

Also ignore helpers which are defined in macro. Currently clang-move doesn't
handle macro well enough, especiall for complex macros. This patch will ignore
declarations in macros to make the behavior of clang-move more correct.

[ioeric]

Lg.

test/clang-move/no-move-macro-helpers.cpp

1 ↗

(On Diff #84570)

Would be nice to add a test case where both A and f1 are moved.

19 ↗

(On Diff #84570)

Check that this is not deleted in old cc.

[hokein]

Add more tests.

clang-tools-extra/trunk/clang-move/ClangMove.cpp

 
 namespace clang {
 namespace move {
 namespace {
 
 // FIXME: Move to ASTMatchers.
 AST_MATCHER(VarDecl, isStaticDataMember) { return Node.isStaticDataMember(); }
 
+AST_MATCHER(NamedDecl, notInMacro) { return !Node.getLocation().isMacroID(); }
+
 AST_MATCHER_P(Decl, hasOutermostEnclosingClass,
               ast_matchers::internal::Matcher<Decl>, InnerMatcher) {
   const auto *Context = Node.getDeclContext();
   if (!Context)
     return false;
   while (const auto *NextContext = Context->getParent()) {
     if (isa<NamespaceDecl>(NextContext) ||
         isa<TranslationUnitDecl>(NextContext))
void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
   //============================================================================
   // Matchers for old cc
   //============================================================================
   auto IsOldCCTopLevelDecl = allOf(
       hasParent(decl(anyOf(namespaceDecl(), translationUnitDecl()))), InOldCC);
   // Matching using decls/type alias decls which are in named/anonymous/global
   // namespace, these decls are always copied to new.h/cc. Those in classes,
   // functions are covered in other matchers.
-  Finder->addMatcher(
-      namedDecl(anyOf(usingDecl(IsOldCCTopLevelDecl),
+  Finder->addMatcher(namedDecl(anyOf(usingDecl(IsOldCCTopLevelDecl),
                                      usingDirectiveDecl(IsOldCCTopLevelDecl),
-                      typeAliasDecl(IsOldCCTopLevelDecl)))
+                                     typeAliasDecl(IsOldCCTopLevelDecl)),
+                               notInMacro())
                          .bind("using_decl"),
                      this);
 
   // Match static functions/variable definitions which are defined in named
   // namespaces.
   Optional<ast_matchers::internal::Matcher<NamedDecl>> HasAnySymbolNames;
   for (StringRef SymbolName : Context->Spec.Names) {
     llvm::StringRef GlobalSymbolName = SymbolName.trim().ltrim(':');
     const auto HasName = hasName(("::" + GlobalSymbolName).str());
     HasAnySymbolNames =
void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
 
   // 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));
   // Match helper classes separately with helper functions/variables since we
   // want to reuse these matchers in finding helpers usage below.
-  auto HelperFuncOrVar = namedDecl(anyOf(functionDecl(IsOldCCHelperDefinition),
-                                         varDecl(IsOldCCHelperDefinition)));
-  auto HelperClasses = cxxRecordDecl(DefinitionInOldCC, InAnonymousNS);
+  auto HelperFuncOrVar =
+      namedDecl(notInMacro(), anyOf(functionDecl(IsOldCCHelperDefinition),
+  auto HelperClasses =
+      cxxRecordDecl(notInMacro(), DefinitionInOldCC, InAnonymousNS);
   // 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

clang-tools-extra/trunk/test/clang-move/Inputs/macro_helper_test.h

+class A {};
+void f1();

clang-tools-extra/trunk/test/clang-move/Inputs/macro_helper_test.cpp

+#include "macro_helper_test.h"
+
+#define DEFINE(name) \
+  namespace ns { \
+  static const bool t1 = false; \
+  bool t2_##name = t1; \
+  bool t3_##name = t1; \
+  } \
+  using ns::t2_##name;
+
+DEFINE(test)
+
+void f1() {}

clang-tools-extra/trunk/test/clang-move/no-move-macro-helpers.cpp

+// RUN: mkdir -p %T/no-move-macro-helper
+// RUN: cp %S/Inputs/macro_helper_test.h  %T/no-move-macro-helper/macro_helper_test.h
+// RUN: cp %S/Inputs/macro_helper_test.cpp %T/no-move-macro-helper/macro_helper_test.cpp
+// RUN: cd %T/no-move-macro-helper
+//
+// -----------------------------------------------------------------------------
+// Test no moving helpers in macro.
+// -----------------------------------------------------------------------------
+// RUN: clang-move -names="A" -new_cc=%T/no-move-macro-helper/new_test.cpp -new_header=%T/no-move-macro-helper/new_test.h -old_cc=%T/no-move-macro-helper/macro_helper_test.cpp -old_header=%T/no-move-macro-helper/macro_helper_test.h %T/no-move-macro-helper/macro_helper_test.cpp -- -std=c++11
+// RUN: FileCheck -input-file=%T/no-move-macro-helper/new_test.h -check-prefix=CHECK-NEW-TEST-CASE1-H %s
+// RUN: FileCheck -input-file=%T/no-move-macro-helper/new_test.cpp -check-prefix=CHECK-NEW-TEST-CASE1-CPP %s
+// RUN: FileCheck -input-file=%T/no-move-macro-helper/macro_helper_test.h -check-prefix=CHECK-OLD-TEST-CASE1-H %s
+// RUN: FileCheck -input-file=%T/no-move-macro-helper/macro_helper_test.cpp -check-prefix=CHECK-OLD-TEST-CASE1-CPP %s
+
+// CHECK-NEW-TEST-CASE1-H: class A {};
+
+// CHECK-OLD-TEST-CASE1-H-NOT: class A {};
+
+// CHECK-OLD-TEST-CASE1-CPP: DEFINE(test)
+
+// CHECK-NEW-TEST-CASE1-CPP-NOT: DEFINE(test)
+
+
+// -----------------------------------------------------------------------------
+// Test moving all.
+// -----------------------------------------------------------------------------
+// RUN: cp %S/Inputs/macro_helper_test.h  %T/no-move-macro-helper/macro_helper_test.h
+// RUN: cp %S/Inputs/macro_helper_test.cpp %T/no-move-macro-helper/macro_helper_test.cpp
+// RUN: clang-move -names="A, f1" -new_cc=%T/no-move-macro-helper/new_test.cpp -new_header=%T/no-move-macro-helper/new_test.h -old_cc=%T/no-move-macro-helper/macro_helper_test.cpp -old_header=%T/no-move-macro-helper/macro_helper_test.h %T/no-move-macro-helper/macro_helper_test.cpp -- -std=c++11
+//
+// RUN: FileCheck -input-file=%T/no-move-macro-helper/new_test.h -check-prefix=CHECK-NEW-TEST-CASE2-H %s
+// RUN: FileCheck -input-file=%T/no-move-macro-helper/new_test.cpp -check-prefix=CHECK-NEW-TEST-CASE2-CPP %s
+// RUN: FileCheck -input-file=%T/no-move-macro-helper/macro_helper_test.h -allow-empty -check-prefix=CHECK-EMPTY %s
+// RUN: FileCheck -input-file=%T/no-move-macro-helper/macro_helper_test.cpp -allow-empty -check-prefix=CHECK-EMPTY %s
+
+// CHECK-NEW-TEST-CASE2-H: class A {};
+// CHECK-NEW-TEST-CASE2-H-NEXT:void f1();
+
+
+// CHECK-NEW-TEST-CASE2-CPP: DEFINE(test)
+// CHECK-NEW-TEST-CASE2-CPP: void f1() {}
+
+// CHECK-EMPTY: {{^}}{{$}}