[change-namespace] do not fix calls to overloaded operator functions.

[ioeric]

Summary

Also make sure one function reference is only processed once.

[ioeric]

• minor fix.

[hokein]

LGTM.

unittests/change-namespace/ChangeNamespaceTests.cpp

578 ↗

(On Diff #82092)

Maybe also add this case in the test although it is not fixed yet.

592 ↗

(On Diff #82092)

missing a \n, the same below.

[ioeric]

• Address comments

clang-tools-extra/trunk/change-namespace/ChangeNamespace.h

private:
   // specifiers.
   llvm::SmallPtrSet<const UsingDecl *, 8> UsingDecls;
   // Records all using namespace declarations, which can be used to shorten
   // namespace specifiers.
   llvm::SmallPtrSet<const UsingDirectiveDecl *, 8> UsingNamespaceDecls;
   // TypeLocs of CXXCtorInitializer. Types of CXXCtorInitializers do not need to
   // be fixed.
   llvm::SmallVector<TypeLoc, 8> BaseCtorInitializerTypeLocs;
+  // Since a DeclRefExpr for a function call can be matched twice (one as
+  // CallExpr and one as DeclRefExpr), we record all DeclRefExpr's that have
+  // been processed so that we don't handle them twice.
+  llvm::SmallPtrSet<const clang::DeclRefExpr*, 16> ProcessedFuncRefs;
 };
 
 } // namespace change_namespace
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CHANGE_NAMESPACE_CHANGENAMESPACE_H

clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp

if (const auto *Using = Result.Nodes.getNodeAs<UsingDecl>("using")) {
     if (Var->getCanonicalDecl()->isStaticDataMember())
       return;
     const auto *Context = Result.Nodes.getNodeAs<Decl>("dc");
     assert(Context && "Empty decl context.");
     fixDeclRefExpr(Result, Context->getDeclContext(),
                    llvm::cast<NamedDecl>(Var), VarRef);
   } else if (const auto *FuncRef =
                  Result.Nodes.getNodeAs<DeclRefExpr>("func_ref")) {
+    // If this reference has been processed as a function call, we do not
+    // process it again.
+    if (ProcessedFuncRefs.count(FuncRef))
+      return;
+    ProcessedFuncRefs.insert(FuncRef);
     const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func_decl");
     assert(Func);
     const auto *Context = Result.Nodes.getNodeAs<Decl>("dc");
     assert(Context && "Empty decl context.");
     fixDeclRefExpr(Result, Context->getDeclContext(),
                    llvm::cast<NamedDecl>(Func), FuncRef);
   } else {
     const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call");
     assert(Call != nullptr && "Expecting callback for CallExpr.");
+    const auto *CalleeFuncRef =
+        llvm::cast<DeclRefExpr>(Call->getCallee()->IgnoreImplicit());
+    ProcessedFuncRefs.insert(CalleeFuncRef);
     const FunctionDecl *Func = Call->getDirectCallee();
     assert(Func != nullptr);
+    // FIXME: ignore overloaded operators. This would miss cases where operators
+    // are called by qualified names (i.e. "ns::operator <"). Ignore such
+    // cases for now.
+    if (Func->isOverloadedOperator())
+      return;
     // Ignore out-of-line static methods since they will be handled by nested
     // name specifiers.
     if (Func->getCanonicalDecl()->getStorageClass() ==
             StorageClass::SC_Static &&
         Func->isOutOfLine())
       return;
     const auto *Context = Result.Nodes.getNodeAs<Decl>("dc");
     assert(Context && "Empty decl context.");

clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp

std::string Expected =
       "namespace y {\n"
       "void f() { ::na::a_f(); ::na::static_f(); ::na::A::f(); }\n"
       "void g() { f(); ::na::A::g(); }\n"
       "}  // namespace y\n"
       "}  // namespace x\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, FixOverloadedOperatorFunctionNameSpecifiers) {
+  std::string Code =
+      "namespace na {\n"
+      "class A {\n"
+      "public:\n"
+      "  int x;\n"
+      "  bool operator==(const A &RHS) const { return x == RHS.x; }\n"
+      "};\n"
+      "bool operator<(const A &LHS, const A &RHS) { return LHS.x == RHS.x; }\n"
+      "namespace nb {\n"
+      "bool f() {\n"
+      "  A x, y;\n"
+      "  auto f = operator<;\n"
+      "  return (x == y) && (x < y) && (operator<(x, y));\n"
+      "}\n"
+      "}  // namespace nb\n"
+      "}  // namespace na\n";
+  std::string Expected =
+      "namespace na {\n"
+      "class A {\n"
+      "public:\n"
+      "  int x;\n"
+      "  bool operator==(const A &RHS) const { return x == RHS.x; }\n"
+      "};\n"
+      "bool operator<(const A &LHS, const A &RHS) { return LHS.x == RHS.x; }\n"
+      "\n"
+      "}  // namespace na\n"
+      "namespace x {\n"
+      "namespace y {\n"
+      "bool f() {\n"
+      "  ::na::A x, y;\n"
+      "  auto f = ::na::operator<;\n"
+      // FIXME: function calls to overloaded operators are not fixed now even if
+      // they are referenced by qualified names.
+      "  return (x == y) && (x < y) && (operator<(x,y));\n"
+      "}\n"
+      "}  // namespace y\n"
+      "}  // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
 TEST_F(ChangeNamespaceTest, FixNonCallingFunctionReferences) {
   std::string Code = "namespace na {\n"
                      "class A {\n"
                      "public:\n"
                      "  static void f() {}\n"
                      "};\n"
                      "void a_f() {}\n"
                      "static void s_f() {}\n"