[clang-move] Move using-decl in old cc.

[hokein]

Summary

Another fix is to move the whole anonymous namespace declaration
completely instead of moving fun/var declarations only.

[ioeric]

clang-move/ClangMove.cpp

356 ↗

(On Diff #75120)

Is UsingDirectiveDecl, i.e. using namespace decl, included in this case?

366 ↗

(On Diff #75120)

Seems that filters for functionDecl and varDecl are the same? Maybe apply them on namedDecl?

test/clang-move/Inputs/multiple_class_test.cpp

9 ↗

(On Diff #75120)

Can you also add tests where using decls are in classes/functions?

[hokein]

Address comments.

• Cover type alias declarations.
• Fix an issue of moving function-scope static definitions in non-moved classes.

[hokein]

clang-move/ClangMove.cpp

356 ↗

(On Diff #75120)

Also covered type alias declarations.

366 ↗

(On Diff #75120)

I tried it before, but some ast matchers are not available on nameDecl. I simplified the code a little bit by pulling out these filters.

[ioeric]

Lg

clang-move/ClangMove.cpp

372 ↗

(On Diff #75139)

s/named spaces/named namespaces/ ?

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

Finder->addMatcher(
       this);
 
   // Match static member variable definition of the moved class.
   Finder->addMatcher(varDecl(InMovedClass, InOldCC, isDefinition(),
                              isStaticDataMember())
                          .bind("class_static_var_decl"),
                      this);
 
-  auto inAnonymousNamespace = hasParent(namespaceDecl(isAnonymous()));
-  // Match functions/variables definitions which are defined in anonymous
-  // namespace in old cc.
+  auto InOldCCNamedNamespace =
+      allOf(hasParent(namespaceDecl(unless(isAnonymous()))), InOldCC);
+  // Matching using decls/type alias decls which are in named namespace. Those
+  // in classes, functions and anonymous namespaces are covered in other
+  // matchers.
   Finder->addMatcher(
-      namedDecl(anyOf(functionDecl(isDefinition()), varDecl(isDefinition())),
-                inAnonymousNamespace)
-          .bind("decls_in_anonymous_ns"),
+      namedDecl(
+          anyOf(usingDecl(InOldCCNamedNamespace),
+                usingDirectiveDecl(InOldCC, InOldCCNamedNamespace),
+                typeAliasDecl(InOldCC, InOldCCNamedNamespace)))
+          .bind("using_decl"),
       this);
 
-  // Match static functions/variabale definitions in old cc.
+  // 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.
+  auto IsOldCCStaticDefinition =
+      allOf(isDefinition(), unless(InMovedClass), InOldCCNamedNamespace,
+            isStaticStorageClass());
   Finder->addMatcher(
-      namedDecl(anyOf(functionDecl(isDefinition(), unless(InMovedClass),
-                                   isStaticStorageClass(), InOldCC),
-                      varDecl(isDefinition(), unless(InMovedClass),
-                              isStaticStorageClass(), InOldCC)))
+      namedDecl(anyOf(functionDecl(IsOldCCStaticDefinition),
+                      varDecl(IsOldCCStaticDefinition)))
           .bind("static_decls"),
       this);
 
   // Match forward declarations in old header.
   Finder->addMatcher(
       cxxRecordDecl(unless(anyOf(isImplicit(), isDefinition())), InOldHeader)
           .bind("fwd_decl"),
       this);
RemovedDecls.push_back(MovedDecls.back());
                  Result.Nodes.getNodeAs<clang::CXXRecordDecl>("moved_class")) {
     MovedDecls.emplace_back(class_decl, &Result.Context->getSourceManager());
     RemovedDecls.push_back(MovedDecls.back());
   } else if (const auto *FWD =
                  Result.Nodes.getNodeAs<clang::CXXRecordDecl>("fwd_decl")) {
     // Skip all forwad declarations which appear after moved class declaration.
     if (RemovedDecls.empty())
       MovedDecls.emplace_back(FWD, &Result.Context->getSourceManager());
-  } else if (const auto *FD = Result.Nodes.getNodeAs<clang::NamedDecl>(
-                 "decls_in_anonymous_ns")) {
-    MovedDecls.emplace_back(FD, &Result.Context->getSourceManager());
+  } else if (const auto *ANS = Result.Nodes.getNodeAs<clang::NamespaceDecl>(
+                 "anonymous_ns")) {
+    MovedDecls.emplace_back(ANS, &Result.Context->getSourceManager());
   } else if (const auto *ND =
                  Result.Nodes.getNodeAs<clang::NamedDecl>("static_decls")) {
     MovedDecls.emplace_back(ND, &Result.Context->getSourceManager());
+  } else if (const auto *UD =
+                 Result.Nodes.getNodeAs<clang::NamedDecl>("using_decl")) {
+    MovedDecls.emplace_back(UD, &Result.Context->getSourceManager());
   }
 }
 
 void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader,
                                 bool IsAngled,
                                 llvm::StringRef SearchPath,
                                 llvm::StringRef FileName,
                                 const SourceManager& SM) {

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

 #include "multiple_class_test.h"
 
 namespace a {
 int Move1::f() {
   return 0;
 }
 } // namespace a
 
+namespace {
+using a::Move1;
+using namespace a;
+static int k = 0;
+} // anonymous namespace
+
 namespace b {
+using a::Move1;
+using namespace a;
+using T = a::Move1;
 int Move2::f() {
   return 0;
 }
 } // namespace b
 
 namespace c {
 int Move3::f() {
+  using a::Move1;
+  using namespace b;
   return 0;
 }
 
 int Move4::f() {
   return 0;
 }
 
 int EnclosingMove5::a = 1;
 
 int EnclosingMove5::Nested::f() {
   return 0;
 }
 
 int EnclosingMove5::Nested::b = 1;
 
 int NoMove::f() {
+  static int F = 0;
   return 0;
 }
 } // namespace c

clang-tools-extra/trunk/test/clang-move/move-multiple-classes.cpp

 // CHECK-OLD-TEST-H: namespace c {
 // CHECK-OLD-TEST-H: class NoMove {
 // CHECK-OLD-TEST-H: public:
 // CHECK-OLD-TEST-H:   int f();
 // CHECK-OLD-TEST-H: };
 // CHECK-OLD-TEST-H: } // namespace c
 
 // CHECK-OLD-TEST-CPP: #include "{{.*}}multiple_class_test.h"
+// 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 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: }
 // CHECK-OLD-TEST-CPP: } // namespace c
 
 // CHECK-NEW-TEST-H: #ifndef {{.*}}NEW_MULTIPLE_CLASS_TEST_H
 // CHECK-NEW-TEST-H: #define {{.*}}NEW_MULTIPLE_CLASS_TEST_H
 // CHECK-NEW-TEST-H: namespace a {
 // CHECK-NEW-TEST-H: class Move1 {

 // CHECK-NEW-TEST-H: };
 // CHECK-NEW-TEST-H: } // namespace c
 // CHECK-NEW-TEST-H: #endif // {{.*}}NEW_MULTIPLE_CLASS_TEST_H
 
 // CHECK-NEW-TEST-CPP: #include "{{.*}}new_multiple_class_test.h"
 // CHECK-NEW-TEST-CPP: namespace a {
 // CHECK-NEW-TEST-CPP: int Move1::f() { return 0; }
 // CHECK-NEW-TEST-CPP: } // namespace a
+// CHECK-NEW-TEST-CPP: namespace {
+// 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 b {
+// CHECK-NEW-TEST-CPP: using a::Move1;
+// CHECK-NEW-TEST-CPP: using namespace a;
+// CHECK-NEW-TEST-CPP: using T = a::Move1;
 // CHECK-NEW-TEST-CPP: int Move2::f() { return 0; }
 // CHECK-NEW-TEST-CPP: } // namespace b
 // CHECK-NEW-TEST-CPP: namespace c {
-// CHECK-NEW-TEST-CPP: int Move3::f() { return 0; }
+// CHECK-NEW-TEST-CPP: int Move3::f() {
+// CHECK-NEW-TEST-CPP:   using a::Move1;
+// 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 EnclosingMove5::a = 1;
 // CHECK-NEW-TEST-CPP: int EnclosingMove5::Nested::f() { return 0; }
 // CHECK-NEW-TEST-CPP: int EnclosingMove5::Nested::b = 1;
 // CHECK-NEW-TEST-CPP: } // namespace c