[clang-move] Don't dump macro symbols.

[ioeric]

Lg. Thanks for the change!

clang-move/ClangMove.cpp

526

I'd probably relax the condition a bit; theoretically tools would be able to handle entire identifiers that are either spelled in macro or passed in by users. But it's probably rare. Might worth a FIXME though?

unittests/clang-move/ClangMoveTests.cpp

397

Could you add a test case where the identifier is composed of macro and user parameter? Something like #define Foo(x) void func_##x()?

[ioeric]

Ooops, forgot to stamp!

[hokein]

Add more tests.

[hokein]

clang-move/ClangMove.cpp

526

This mainly affects the dump behavior. Moving symbols spelled in macro is tricky, and we can't guarantee always doing right thing. Actually, clang-move supports it partially. For the test case of this patch, if you move the Foo (names=Foo), clang-move will move the expansion macro (DEFINE_Foo;), although Foo is not dumped.

clang-move/ClangMove.cpp

void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
   // Match all top-level named declarations (e.g. function, variable, enum) in
   // old header, exclude forward class declarations and namespace declarations.
   //
   // We consider declarations inside a class belongs to the class. So these
   // declarations will be ignored.
   auto AllDeclsInHeader = namedDecl(
       unless(ForwardClassDecls), unless(namespaceDecl()),
       unless(usingDirectiveDecl()), // using namespace decl.
+      notInMacro(),
       InOldHeader,
       hasParent(decl(anyOf(namespaceDecl(), translationUnitDecl()))),
       hasDeclContext(decl(anyOf(namespaceDecl(), translationUnitDecl()))));
   Finder->addMatcher(AllDeclsInHeader.bind("decls_in_header"), this);
 
   // Don't register other matchers when dumping all declarations in header.
   if (Context->DumpDeclarations)
     return;
for (const auto *Decl : UnremovedDeclsInOldHeader) {
                Kind == Decl::Kind::TypeAliasTemplate)
         Reporter->reportDeclaration(QualifiedName, "TypeAlias");
     }
     return;
   }
 
   if (RemovedDecls.empty())
     return;
-  // Ignore symbols that are not supported (e.g. typedef and enum) when
-  // checking if there is unremoved symbol in old header. This makes sure that
-  // we always move old files to new files when all symbols produced from
-  // dump_decls are moved.
+  // Ignore symbols that are not supported when checking if there is unremoved
+  // symbol in old header. This makes sure that we always move old files to new
+  // files when all symbols produced from dump_decls are moved.
   auto IsSupportedKind = [](const clang::NamedDecl *Decl) {
     switch (Decl->getKind()) {
     case Decl::Kind::Function:
     case Decl::Kind::FunctionTemplate:
     case Decl::Kind::ClassTemplate:
     case Decl::Kind::CXXRecord:
     case Decl::Kind::Enum:
     case Decl::Kind::Typedef:

unittests/clang-move/ClangMoveTests.cpp

for (const auto& Header : TestHeaders) {
     auto Results = runClangMoveOnCode(Spec, Header.c_str(), Code);
     EXPECT_EQ(ExpectedHeader, Results[Spec.NewHeader]);
     // The expected old header should not contain class A definition.
     std::string ExpectedOldHeader = Header.substr(0, Header.size() - 32);
     EXPECT_EQ(ExpectedOldHeader, Results[Spec.OldHeader]);
   }
 }
 
+TEST(ClangMove, IgnoreMacroSymbolsAndMoveAll) {
+  const char TestCode[] = "#include \"foo.h\"";
+  std::vector<std::string> TestHeaders = {
+    "#define DEFINE_Foo int Foo = 1;\nDEFINE_Foo;\nclass Bar {};\n",
+    "#define DEFINE(x) int var_##x = 1;\nDEFINE(foo);\nclass Bar {};\n",
+  };
+  move::MoveDefinitionSpec Spec;
+  Spec.Names.push_back("Bar");
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+
+  for (const auto& Header : TestHeaders) {
+    auto Results = runClangMoveOnCode(Spec, Header.c_str(), TestCode);
+    EXPECT_EQ("", Results[Spec.OldHeader]);
+    EXPECT_EQ(Header, Results[Spec.NewHeader]);
+  }
+}
+
 TEST(ClangMove, MacroInFunction) {
   const char TestHeader[] = "#define INT int\n"
                             "class A {\npublic:\n  int f();\n};\n"
                             "class B {};\n";
   const char TestCode[] = "#include \"foo.h\"\n"
                           "INT A::f() { return 0; }\n";
   const char ExpectedNewCode[] = "#include \"new_foo.h\"\n\n"
                                  "INT A::f() { return 0; }\n";
const char TestHeader[] = "template <typename T>\n"
                             "enum E1 { Green };\n"
                             "enum class E2 { Red };\n"
                             "typedef int Int2;\n"
                             "typedef A<double> A_d;"
                             "using Int = int;\n"
                             "extern int kGlobalInt;\n"
                             "extern const char* const kGlobalStr;\n"
                             "} // namespace b\n"
-                            "} // namespace a\n";
+                            "} // namespace a\n"
+                            "#define DEFINE_FOO class Foo {};\n"
+                            "DEFINE_FOO\n";
   const char TestCode[] = "#include \"foo.h\"\n";
   move::MoveDefinitionSpec Spec;
   Spec.Names.push_back("B");
   Spec.OldHeader = "foo.h";
   Spec.OldCC = "foo.cc";
   Spec.NewHeader = "new_foo.h";
   Spec.NewCC = "new_foo.cc";
   DeclarationReporter Reporter;