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

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL290176: [change-namespace] do not fix calls to overloaded 
operator functions. (authored by ioeric).

Changed prior to commit:
  https://reviews.llvm.org/D27982?vs=82104=82105#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27982

Files:
  clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
  clang-tools-extra/trunk/change-namespace/ChangeNamespace.h
  clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp


Index: 
clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -575,6 +575,47 @@
   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 ) const { return x == RHS.x; }\n"
+  "};\n"
+  "bool operator<(const A , const A ) { 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 ) const { return x == RHS.x; }\n"
+  "};\n"
+  "bool operator<(const A , const A ) { 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"
Index: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
===
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
@@ -481,6 +481,11 @@
llvm::cast(Var), VarRef);
   } else if (const auto *FuncRef =
  Result.Nodes.getNodeAs("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("func_decl");
 assert(Func);
 const auto *Context = Result.Nodes.getNodeAs("dc");
@@ -490,8 +495,16 @@
   } else {
 const auto *Call = Result.Nodes.getNodeAs("call");
 assert(Call != nullptr && "Expecting callback for CallExpr.");
+const auto *CalleeFuncRef =
+llvm::cast(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() ==
Index: clang-tools-extra/trunk/change-namespace/ChangeNamespace.h
===
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.h
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.h
@@ -157,6 +157,10 @@
   // TypeLocs of CXXCtorInitializer. Types of CXXCtorInitializers do not need 
to
   // be fixed.
   llvm::SmallVector 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 ProcessedFuncRefs;
 };
 
 } // namespace change_namespace


Index: clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
+++ 

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

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 82104.
ioeric marked 2 inline comments as done.
ioeric added a comment.

- Address comments


https://reviews.llvm.org/D27982

Files:
  change-namespace/ChangeNamespace.cpp
  change-namespace/ChangeNamespace.h
  unittests/change-namespace/ChangeNamespaceTests.cpp


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -575,6 +575,47 @@
   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 ) const { return x == RHS.x; }\n"
+  "};\n"
+  "bool operator<(const A , const A ) { 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 ) const { return x == RHS.x; }\n"
+  "};\n"
+  "bool operator<(const A , const A ) { 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"
Index: change-namespace/ChangeNamespace.h
===
--- change-namespace/ChangeNamespace.h
+++ change-namespace/ChangeNamespace.h
@@ -157,6 +157,10 @@
   // TypeLocs of CXXCtorInitializer. Types of CXXCtorInitializers do not need 
to
   // be fixed.
   llvm::SmallVector 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 ProcessedFuncRefs;
 };
 
 } // namespace change_namespace
Index: change-namespace/ChangeNamespace.cpp
===
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -481,6 +481,11 @@
llvm::cast(Var), VarRef);
   } else if (const auto *FuncRef =
  Result.Nodes.getNodeAs("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("func_decl");
 assert(Func);
 const auto *Context = Result.Nodes.getNodeAs("dc");
@@ -490,8 +495,16 @@
   } else {
 const auto *Call = Result.Nodes.getNodeAs("call");
 assert(Call != nullptr && "Expecting callback for CallExpr.");
+const auto *CalleeFuncRef =
+llvm::cast(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() ==


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -575,6 +575,47 @@
   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 ) const { return x == RHS.x; }\n"
+  "};\n"
+  "bool operator<(const A , const A ) { return LHS.x == RHS.x; }\n"
+  "namespace nb {\n"
+  "bool f() {\n"
+  "  A x, y;\n"
+   

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

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: unittests/change-namespace/ChangeNamespaceTests.cpp:578
 
+// FIXME: function calls to overloaded operators are not fixed now even if they
+// are referenced by qualified names.

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



Comment at: unittests/change-namespace/ChangeNamespaceTests.cpp:592
+  "  A x, y;\n"
+  "  auto f = operator<;"
+  "  return (x == y) && (x < y) && (operator<(x, y));"

missing a `\n`, the same below.


https://reviews.llvm.org/D27982



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 82098.
ioeric added a comment.

- minor fix.


https://reviews.llvm.org/D27982

Files:
  change-namespace/ChangeNamespace.cpp
  change-namespace/ChangeNamespace.h
  unittests/change-namespace/ChangeNamespaceTests.cpp


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -575,6 +575,47 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+// FIXME: function calls to overloaded operators are not fixed now even if they
+// are referenced by qualified names.
+TEST_F(ChangeNamespaceTest, FixOverloadedOperatorFunctionNameSpecifiers) {
+  std::string Code =
+  "namespace na {\n"
+  "class A {\n"
+  "public:\n"
+  "  int x;\n"
+  "  bool operator==(const A ) const { return x == RHS.x; }\n"
+  "};\n"
+  "bool operator<(const A , const A ) { return LHS.x == RHS.x; }\n"
+  "namespace nb {\n"
+  "bool f() {\n"
+  "  A x, y;\n"
+  "  auto f = operator<;"
+  "  return (x == y) && (x < y) && (operator<(x, y));"
+  "}\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 ) const { return x == RHS.x; }\n"
+  "};\n"
+  "bool operator<(const A , const A ) { 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<;"
+  "  return (x == y) && (x < y) && (operator<(x,y));"
+  "}\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"
Index: change-namespace/ChangeNamespace.h
===
--- change-namespace/ChangeNamespace.h
+++ change-namespace/ChangeNamespace.h
@@ -157,6 +157,10 @@
   // TypeLocs of CXXCtorInitializer. Types of CXXCtorInitializers do not need 
to
   // be fixed.
   llvm::SmallVector 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 ProcessedFuncRefs;
 };
 
 } // namespace change_namespace
Index: change-namespace/ChangeNamespace.cpp
===
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -481,6 +481,11 @@
llvm::cast(Var), VarRef);
   } else if (const auto *FuncRef =
  Result.Nodes.getNodeAs("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("func_decl");
 assert(Func);
 const auto *Context = Result.Nodes.getNodeAs("dc");
@@ -490,8 +495,16 @@
   } else {
 const auto *Call = Result.Nodes.getNodeAs("call");
 assert(Call != nullptr && "Expecting callback for CallExpr.");
+const auto *CalleeFuncRef =
+llvm::cast(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() ==


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -575,6 +575,47 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+// FIXME: function calls to overloaded operators are not fixed now even if they
+// are referenced by qualified names.
+TEST_F(ChangeNamespaceTest, FixOverloadedOperatorFunctionNameSpecifiers) {
+  std::string Code =
+  "namespace na {\n"
+  "class A {\n"
+  "public:\n"
+  "  int x;\n"
+  "  bool operator==(const A ) const { return x == RHS.x; }\n"
+  "};\n"
+  "bool operator<(const A , const A ) { return LHS.x == RHS.x; }\n"
+  "namespace nb 

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

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: hokein.
ioeric added a subscriber: cfe-commits.

Also make sure one function reference is only processed once.


https://reviews.llvm.org/D27982

Files:
  change-namespace/ChangeNamespace.cpp
  change-namespace/ChangeNamespace.h
  unittests/change-namespace/ChangeNamespaceTests.cpp


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -575,6 +575,47 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+// FIXME: function calls to overloaded operators are not fixed now even if they
+// are referenced by qualified names.
+TEST_F(ChangeNamespaceTest, FixOverloadedOperatorFunctionNameSpecifiers) {
+  std::string Code =
+  "namespace na {\n"
+  "class A {\n"
+  "public:\n"
+  "  int x;\n"
+  "  bool operator==(const A ) const { return x == RHS.x; }\n"
+  "};\n"
+  "bool operator<(const A , const A ) { return LHS.x == RHS.x; }\n"
+  "namespace nb {\n"
+  "bool f() {\n"
+  "  A x, y;\n"
+  "  auto f = operator<;"
+  "  return (x == y) && (x < y) && (operator<(x, y));"
+  "}\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 ) const { return x == RHS.x; }\n"
+  "};\n"
+  "bool operator<(const A , const A ) { 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<;"
+  "  return (x == y) && (x < y) && (operator<(x,y));"
+  "}\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"
Index: change-namespace/ChangeNamespace.h
===
--- change-namespace/ChangeNamespace.h
+++ change-namespace/ChangeNamespace.h
@@ -157,6 +157,10 @@
   // TypeLocs of CXXCtorInitializer. Types of CXXCtorInitializers do not need 
to
   // be fixed.
   llvm::SmallVector 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 ProcessedFuncRefs;
 };
 
 } // namespace change_namespace
Index: change-namespace/ChangeNamespace.cpp
===
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -481,6 +481,10 @@
llvm::cast(Var), VarRef);
   } else if (const auto *FuncRef =
  Result.Nodes.getNodeAs("func_ref")) {
+// If this reference has been processed as a function call, we do not
+// process it again.
+if (ProcessedFuncRefs.count(FuncRef))
+  return;
 const auto *Func = Result.Nodes.getNodeAs("func_decl");
 assert(Func);
 const auto *Context = Result.Nodes.getNodeAs("dc");
@@ -490,8 +494,16 @@
   } else {
 const auto *Call = Result.Nodes.getNodeAs("call");
 assert(Call != nullptr && "Expecting callback for CallExpr.");
+const auto *CalleeFuncRef =
+llvm::cast(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() ==


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -575,6 +575,47 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+// FIXME: function calls to overloaded operators are not fixed now even if they
+// are referenced by qualified names.
+TEST_F(ChangeNamespaceTest, FixOverloadedOperatorFunctionNameSpecifiers) {
+  std::string Code =
+  "namespace na {\n"
+  "class A {\n"
+  "public:\n"
+  "  int x;\n"
+  "  bool operator==(const A ) const { return x == RHS.x; }\n"
+  "};\n"
+  "bool operator<(const A , const A ) {