[clang-tools-extra] r212924 - Suggest automated replacements of C-style casts with C++ casts.

Alexander Kornienko alexfh at google.com
Mon Jul 14 00:37:07 PDT 2014


Author: alexfh
Date: Mon Jul 14 02:37:05 2014
New Revision: 212924

URL: http://llvm.org/viewvc/llvm-project?rev=212924&view=rev
Log:
Suggest automated replacements of C-style casts with C++ casts.

Summary:
This patch implements a subset of possible replacements of C-style
casts with const_cast/static_cast/reinterpret_cast. This should cover a large
portion of cases in real code. Handling of a few more cases may be implemented
eventually.

Reviewers: sbenza, djasper

Reviewed By: djasper

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D4478

Modified:
    clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp
    clang-tools-extra/trunk/test/clang-tidy/avoid-c-style-casts.cpp

Modified: clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp?rev=212924&r1=212923&r2=212924&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp Mon Jul 14 02:37:05 2014
@@ -8,8 +8,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "AvoidCStyleCastsCheck.h"
+#include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
@@ -24,27 +26,107 @@ AvoidCStyleCastsCheck::registerMatchers(
           // Filter out (EnumType)IntegerLiteral construct, which is generated
           // for non-type template arguments of enum types.
           // FIXME: Remove this once this is fixed in the AST.
-          unless(hasParent(substNonTypeTemplateParmExpr()))).bind("cast"),
+          unless(hasParent(substNonTypeTemplateParmExpr())),
+          // Avoid matches in template instantiations.
+          unless(hasAncestor(decl(
+              anyOf(recordDecl(ast_matchers::isTemplateInstantiation()),
+                    functionDecl(ast_matchers::isTemplateInstantiation()))))))
+          .bind("cast"),
       this);
 }
 
+bool needsConstCast(QualType SourceType, QualType DestType) {
+  SourceType = SourceType.getNonReferenceType();
+  DestType = DestType.getNonReferenceType();
+  while (SourceType->isPointerType() && DestType->isPointerType()) {
+    SourceType = SourceType->getPointeeType();
+    DestType = DestType->getPointeeType();
+    if (SourceType.isConstQualified() && !DestType.isConstQualified())
+      return true;
+  }
+  return false;
+}
+
+bool pointedTypesAreEqual(QualType SourceType, QualType DestType) {
+  SourceType = SourceType.getNonReferenceType();
+  DestType = DestType.getNonReferenceType();
+  while (SourceType->isPointerType() && DestType->isPointerType()) {
+    SourceType = SourceType->getPointeeType();
+    DestType = DestType->getPointeeType();
+  }
+  return SourceType.getUnqualifiedType() == DestType.getUnqualifiedType();
+}
 void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *CastExpr = Result.Nodes.getNodeAs<CStyleCastExpr>("cast");
 
-  // Ignore casts in macros for now.
-  if (CastExpr->getLocStart().isMacroID())
-    return;
-
   // Casting to void is an idiomatic way to mute "unused variable" and similar
   // warnings.
   if (CastExpr->getTypeAsWritten()->isVoidType())
     return;
 
-  diag(CastExpr->getLocStart(), "C-style casts are discouraged. Use "
-                                "static_cast/const_cast/reinterpret_cast "
-                                "instead.");
-  // FIXME: Suggest appropriate C++ cast. See [expr.cast] for cast notation
-  // semantics.
+  QualType SourceType =
+      CastExpr->getSubExprAsWritten()->getType().getCanonicalType();
+  QualType DestType = CastExpr->getTypeAsWritten().getCanonicalType();
+
+  auto ParenRange = CharSourceRange::getTokenRange(CastExpr->getLParenLoc(),
+                                                   CastExpr->getRParenLoc());
+  if (SourceType == DestType) {
+    diag(CastExpr->getLocStart(), "Redundant cast to the same type.")
+        << FixItHint::CreateRemoval(ParenRange);
+    return;
+  }
+
+  std::string DestTypeString = CastExpr->getTypeAsWritten().getAsString();
+
+  auto diag_builder =
+      diag(CastExpr->getLocStart(), "C-style casts are discouraged. %0");
+
+  auto ReplaceWithCast = [&](StringRef CastType) {
+    diag_builder << ("Use " + CastType + ".").str();
+    if (ParenRange.getBegin().isMacroID() || ParenRange.getEnd().isMacroID())
+      return;
+    diag_builder << FixItHint::CreateReplacement(
+                        ParenRange,
+                        (CastType + "<" + DestTypeString + ">(").str())
+                 << FixItHint::CreateInsertion(
+                        Lexer::getLocForEndOfToken(
+                            CastExpr->getSubExprAsWritten()->getLocEnd(), 0,
+                            *Result.SourceManager,
+                            Result.Context->getLangOpts()),
+                        ")");
+  };
+  // Suggest appropriate C++ cast. See [expr.cast] for cast notation semantics.
+  switch (CastExpr->getCastKind()) {
+  case CK_NoOp:
+    if (needsConstCast(SourceType, DestType) &&
+        pointedTypesAreEqual(SourceType, DestType)) {
+      ReplaceWithCast("const_cast");
+      return;
+    }
+    if (DestType->isReferenceType() &&
+        (SourceType.getNonReferenceType() ==
+             DestType.getNonReferenceType().withConst() ||
+         SourceType.getNonReferenceType() == DestType.getNonReferenceType())) {
+      ReplaceWithCast("const_cast");
+      return;
+    }
+    if (SourceType->isBuiltinType() && DestType->isBuiltinType()) {
+      ReplaceWithCast("static_cast");
+      return;
+    }
+    break;
+  case CK_BitCast:
+    // FIXME: Suggest const_cast<...>(reinterpret_cast<...>(...)) replacement.
+    if (!needsConstCast(SourceType, DestType)) {
+      ReplaceWithCast("reinterpret_cast");
+      return;
+    }
+    break;
+  default:
+    break;
+  }
+
+  diag_builder << "Use static_cast/const_cast/reinterpret_cast.";
 }
 
 } // namespace readability

Modified: clang-tools-extra/trunk/test/clang-tidy/avoid-c-style-casts.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/avoid-c-style-casts.cpp?rev=212924&r1=212923&r2=212924&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/avoid-c-style-casts.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/avoid-c-style-casts.cpp Mon Jul 14 02:37:05 2014
@@ -1,14 +1,56 @@
-// RUN: clang-tidy -checks=-*,google-readability-casting %s -- | FileCheck %s
-
-// CHECK-NOT: warning:
+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s google-readability-casting %t
+// REQUIRES: shell
 
 bool g() { return false; }
 
-void f(int a, double b) {
+struct X {};
+struct Y : public X {};
+
+void f(int a, double b, const char *cpc, const void *cpv, X *pX) {
+  const char *cpc2 = (const char*)cpc;
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: Redundant cast to the same type. [google-readability-casting]
+  // CHECK-FIXES: const char *cpc2 = cpc;
+
+  char *pc = (char*)cpc;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: C-style casts are discouraged. Use const_cast. {{.*}}
+  // CHECK-FIXES: char *pc = const_cast<char *>(cpc);
+
+  char *pc2 = (char*)(cpc + 33);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C-style casts are discouraged. Use const_cast. {{.*}}
+  // CHECK-FIXES: char *pc2 = const_cast<char *>((cpc + 33));
+
+  const char &crc = *cpc;
+  char &rc = (char&)crc;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: C-style casts are discouraged. Use const_cast. {{.*}}
+  // CHECK-FIXES: char &rc = const_cast<char &>(crc);
+
+  char &rc2 = (char&)*cpc;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C-style casts are discouraged. Use const_cast. {{.*}}
+  // CHECK-FIXES: char &rc2 = const_cast<char &>(*cpc);
+
+  char ** const* const* ppcpcpc;
+  char ****ppppc = (char****)ppcpcpc;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: C-style casts are discouraged. Use const_cast. {{.*}}
+  // CHECK-FIXES: char ****ppppc = const_cast<char ****>(ppcpcpc);
+
   int b1 = (int)b;
-  // CHECK: :[[@LINE-1]]:12: warning: C-style casts are discouraged. Use static_cast{{.*}}
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: C-style casts are discouraged. Use static_cast. [google-readability-casting]
+  // CHECK-FIXES: int b1 = static_cast<int>(b);
+
+  Y *pB = (Y*)pX;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: C-style casts are discouraged. Use static_cast/const_cast/reinterpret_cast. [google-readability-casting]
+  Y &rB = (Y&)*pX;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: C-style casts are discouraged. Use static_cast/const_cast/reinterpret_cast. [google-readability-casting]
 
-  // CHECK-NOT: warning:
+  const char *pc3 = (const char*)cpv;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: C-style casts are discouraged. Use reinterpret_cast. [google-readability-casting]
+  // CHECK-FIXES: const char *pc3 = reinterpret_cast<const char *>(cpv);
+
+  char *pc4 = (char*)cpv;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C-style casts are discouraged. Use static_cast/const_cast/reinterpret_cast. [google-readability-casting]
+  // CHECK-FIXES: char *pc4 = (char*)cpv;
+
+  // CHECK-MESSAGES-NOT: warning:
   int b2 = int(b);
   int b3 = static_cast<double>(b);
   int b4 = b;
@@ -17,8 +59,49 @@ void f(int a, double b) {
   return (void)g();
 }
 
-// CHECK-NOT: warning:
+template <typename T>
+void template_function(T t, int n) {
+  int i = (int)t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: C-style casts are discouraged. Use static_cast/const_cast/reinterpret_cast.
+  // CHECK-FIXES: int i = (int)t;
+  int j = (int)n;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Redundant cast to the same type.
+  // CHECK-FIXES: int j = n;
+}
+
+template <typename T>
+struct TemplateStruct {
+  void f(T t, int n) {
+    int k = (int)t;
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: C-style casts are discouraged. Use static_cast/const_cast/reinterpret_cast.
+    // CHECK-FIXES: int k = (int)t;
+    int l = (int)n;
+    // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: Redundant cast to the same type.
+    // CHECK-FIXES: int l = n;
+  }
+};
+
+void test_templates() {
+  template_function(1, 42);
+  template_function(1.0, 42);
+  TemplateStruct<int>().f(1, 42);
+  TemplateStruct<double>().f(1.0, 42);
+}
+
+#define CAST(type, value) (type)(value)
+void macros(double d) {
+  int i = CAST(int, d);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: C-style casts are discouraged. Use static_cast.
+  // CHECK-FIXES: #define CAST(type, value) (type)(value)
+  // CHECK-FIXES: int i = CAST(int, d);
+}
+
 enum E { E1 = 1 };
 template <E e>
-struct A { static const E ee = e; };
+struct A {
+  // Usage of template argument e = E1 is represented as (E)1 in the AST for
+  // some reason. We have a special treatment of this case to avoid warnings
+  // here.
+  static const E ee = e;
+};
 struct B : public A<E1> {};





More information about the cfe-commits mailing list