[clang-tidy] Fix handling of function types in google-readability-casting

llvm-svn: 294751

2 files changed, 100 insertions, 7 deletions

diff --git a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
index 10dcb342fef..8dd2f82fbee 100644
--- a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -73,14 +73,26 @@ void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
   QualType SourceType = CastExpr->getSubExprAsWritten()->getType();
   QualType DestType = CastExpr->getTypeAsWritten();
 
-  if (SourceType == DestType) {
+  auto isFunction = [](QualType T) {
+    T = T.getCanonicalType().getNonReferenceType();
+    return T->isFunctionType() || T->isFunctionPointerType() ||
+           T->isMemberFunctionPointerType();
+  };
+
+  bool FnToFnCast = isFunction(SourceType) && isFunction(DestType);
+
+  // Function pointer/reference casts may be needed to resolve ambiguities in
+  // case of overloaded functions, so detection of redundant casts is trickier
+  // in this case. Don't emit "redundant cast" warnings for function
+  // pointer/reference types.
+  if (SourceType == DestType && !FnToFnCast) {
     diag(CastExpr->getLocStart(), "redundant cast to the same type")
         << FixItHint::CreateRemoval(ParenRange);
     return;
   }
   SourceType = SourceType.getCanonicalType();
   DestType = DestType.getCanonicalType();
-  if (SourceType == DestType) {
+  if (SourceType == DestType && !FnToFnCast) {
     diag(CastExpr->getLocStart(),
          "possibly redundant cast between typedefs of the same type");
     return;
@@ -111,27 +123,34 @@ void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
                                CastExpr->getRParenLoc().getLocWithOffset(-1)),
                            SM, getLangOpts());
 
-  auto diag_builder =
+  auto Diag =
       diag(CastExpr->getLocStart(), "C-style casts are discouraged; use %0");
 
   auto ReplaceWithCast = [&](StringRef CastType) {
-    diag_builder << CastType;
+    Diag << CastType;
 
     const Expr *SubExpr = CastExpr->getSubExprAsWritten()->IgnoreImpCasts();
     std::string CastText = (CastType + "<" + DestTypeString + ">").str();
     if (!isa<ParenExpr>(SubExpr)) {
       CastText.push_back('(');
-      diag_builder << FixItHint::CreateInsertion(
+      Diag << FixItHint::CreateInsertion(
           Lexer::getLocForEndOfToken(SubExpr->getLocEnd(), 0, SM,
                                      getLangOpts()),
           ")");
     }
-    diag_builder << FixItHint::CreateReplacement(ParenRange, CastText);
+    Diag << FixItHint::CreateReplacement(ParenRange, CastText);
   };
 
   // Suggest appropriate C++ cast. See [expr.cast] for cast notation semantics.
   switch (CastExpr->getCastKind()) {
+  case CK_FunctionToPointerDecay:
+    ReplaceWithCast("static_cast");
+    return;
   case CK_NoOp:
+    if (FnToFnCast) {
+      ReplaceWithCast("static_cast");
+      return;
+    }
     if (needsConstCast(SourceType, DestType) &&
         pointedTypesAreEqual(SourceType, DestType)) {
       ReplaceWithCast("const_cast");
@@ -166,7 +185,7 @@ void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
     break;
   }
 
-  diag_builder << "static_cast/const_cast/reinterpret_cast";
+  Diag << "static_cast/const_cast/reinterpret_cast";
 }
 
 } // namespace readability
diff --git a/clang-tools-extra/test/clang-tidy/google-readability-casting.cpp b/clang-tools-extra/test/clang-tidy/google-readability-casting.cpp
index 16b3453fab2..76c700564a1 100644
--- a/clang-tools-extra/test/clang-tidy/google-readability-casting.cpp
+++ b/clang-tools-extra/test/clang-tidy/google-readability-casting.cpp
@@ -148,3 +148,77 @@ struct A {
   static const E ee = e;
 };
 struct B : public A<E1> {};
+
+
+void overloaded_function();
+void overloaded_function(int);
+
+template<typename Fn>
+void g(Fn fn) {
+  fn();
+}
+
+void function_casts() {
+  typedef void (*FnPtrVoid)();
+  typedef void (&FnRefVoid)();
+  typedef void (&FnRefInt)(int);
+
+  g((void (*)())overloaded_function);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: g(static_cast<void (*)()>(overloaded_function));
+  g((void (*)())&overloaded_function);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: g(static_cast<void (*)()>(&overloaded_function));
+  g((void (&)())overloaded_function);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: g(static_cast<void (&)()>(overloaded_function));
+
+  g((FnPtrVoid)overloaded_function);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: g(static_cast<FnPtrVoid>(overloaded_function));
+  g((FnPtrVoid)&overloaded_function);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: g(static_cast<FnPtrVoid>(&overloaded_function));
+  g((FnRefVoid)overloaded_function);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: g(static_cast<FnRefVoid>(overloaded_function));
+
+  FnPtrVoid fn0 = (void (*)())&overloaded_function;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: FnPtrVoid fn0 = static_cast<void (*)()>(&overloaded_function);
+  FnPtrVoid fn1 = (void (*)())overloaded_function;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: FnPtrVoid fn1 = static_cast<void (*)()>(overloaded_function);
+  FnPtrVoid fn1a = (FnPtrVoid)overloaded_function;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: FnPtrVoid fn1a = static_cast<FnPtrVoid>(overloaded_function);
+  FnRefInt fn2 = (void (&)(int))overloaded_function;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: FnRefInt fn2 = static_cast<void (&)(int)>(overloaded_function);
+  auto fn3 = (void (*)())&overloaded_function;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: auto fn3 = static_cast<void (*)()>(&overloaded_function);
+  auto fn4 = (void (*)())overloaded_function;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: auto fn4 = static_cast<void (*)()>(overloaded_function);
+  auto fn5 = (void (&)(int))overloaded_function;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: auto fn5 = static_cast<void (&)(int)>(overloaded_function);
+
+  void (*fn6)() = (void (*)())&overloaded_function;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: void (*fn6)() = static_cast<void (*)()>(&overloaded_function);
+  void (*fn7)() = (void (*)())overloaded_function;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: void (*fn7)() = static_cast<void (*)()>(overloaded_function);
+  void (*fn8)() = (FnPtrVoid)overloaded_function;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: void (*fn8)() = static_cast<FnPtrVoid>(overloaded_function);
+  void (&fn9)(int) = (void (&)(int))overloaded_function;
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: C-style casts are discouraged; use static_cast [
+  // CHECK-FIXES: void (&fn9)(int) = static_cast<void (&)(int)>(overloaded_function);
+
+  void (*correct1)() = static_cast<void (*)()>(overloaded_function);
+  FnPtrVoid correct2 = static_cast<void (*)()>(&overloaded_function);
+  FnRefInt correct3 = static_cast<void (&)(int)>(overloaded_function);
+}