Index: clang-tidy/google/AvoidCStyleCastsCheck.cpp =================================================================== --- clang-tidy/google/AvoidCStyleCastsCheck.cpp +++ clang-tidy/google/AvoidCStyleCastsCheck.cpp @@ -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 @@ // 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("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().isFileID() && ParenRange.getEnd().isFileID()) { + 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 Index: test/clang-tidy/avoid-c-style-casts.cpp =================================================================== --- test/clang-tidy/avoid-c-style-casts.cpp +++ test/clang-tidy/avoid-c-style-casts.cpp @@ -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(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((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(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(*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(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(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(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(b); int b4 = b; @@ -17,8 +59,49 @@ return (void)g(); } -// CHECK-NOT: warning: +template +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 +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().f(1, 42); + TemplateStruct().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 -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 {};