Author: alexfh Date: Thu Mar 2 09:27:34 2017 New Revision: 296753 URL: http://llvm.org/viewvc/llvm-project?rev=296753&view=rev Log: [clang-tidy] Fix a few more issues in google-readability-casting
* suggest static_cast instead of reinterpret_cast for casts from void* * top-level const doesn't need a const_cast * don't emit a separate "possibly redundant cast" warning, instead suggest static_cast (in C++ only) and add a little hint to consider removing the cast Modified: clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp clang-tools-extra/trunk/test/clang-tidy/google-readability-casting.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=296753&r1=296752&r2=296753&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp Thu Mar 2 09:27:34 2017 @@ -35,19 +35,16 @@ void AvoidCStyleCastsCheck::registerMatc } static bool needsConstCast(QualType SourceType, QualType DestType) { - for (;;) { + while ((SourceType->isPointerType() && DestType->isPointerType()) || + (SourceType->isReferenceType() && DestType->isReferenceType())) { + SourceType = SourceType->getPointeeType(); + DestType = DestType->getPointeeType(); if (SourceType.isConstQualified() && !DestType.isConstQualified()) { return (SourceType->isPointerType() == DestType->isPointerType()) && (SourceType->isReferenceType() == DestType->isReferenceType()); } - if ((SourceType->isPointerType() && DestType->isPointerType()) || - (SourceType->isReferenceType() && DestType->isReferenceType())) { - SourceType = SourceType->getPointeeType(); - DestType = DestType->getPointeeType(); - } else { - return false; - } } + return false; } static bool pointedUnqualifiedTypesAreEqual(QualType T1, QualType T2) { @@ -61,7 +58,6 @@ static bool pointedUnqualifiedTypesAreEq void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) { const auto *CastExpr = Result.Nodes.getNodeAs<CStyleCastExpr>("cast"); - auto ParenRange = CharSourceRange::getTokenRange(CastExpr->getLParenLoc(), CastExpr->getRParenLoc()); // Ignore casts in macros. @@ -81,8 +77,10 @@ void AvoidCStyleCastsCheck::check(const const QualType DestTypeAsWritten = CastExpr->getTypeAsWritten(); const QualType SourceTypeAsWritten = CastExpr->getSubExprAsWritten()->getType(); - const QualType SourceType = SourceTypeAsWritten.getCanonicalType(); - const QualType DestType = DestTypeAsWritten.getCanonicalType(); + const QualType SourceType = + SourceTypeAsWritten.getCanonicalType().getUnqualifiedType(); + const QualType DestType = + DestTypeAsWritten.getCanonicalType().getUnqualifiedType(); bool FnToFnCast = isFunction(SourceTypeAsWritten) && isFunction(DestTypeAsWritten); @@ -97,11 +95,6 @@ void AvoidCStyleCastsCheck::check(const << FixItHint::CreateRemoval(ParenRange); return; } - if (SourceType == DestType) { - diag(CastExpr->getLocStart(), - "possibly redundant cast between typedefs of the same type"); - return; - } } // The rest of this check is only relevant to C++. @@ -115,9 +108,11 @@ void AvoidCStyleCastsCheck::check(const // compiled as C++. if (getCurrentMainFile().endswith(".c")) return; + + SourceManager &SM = *Result.SourceManager; + // Ignore code in .c files #included in other files (which shouldn't be done, // but people still do this for test and other purposes). - SourceManager &SM = *Result.SourceManager; if (SM.getFilename(SM.getSpellingLoc(CastExpr->getLocStart())).endswith(".c")) return; @@ -145,21 +140,7 @@ void AvoidCStyleCastsCheck::check(const }; auto ReplaceWithNamedCast = [&](StringRef CastType) { Diag << CastType; - std::string CastText = (CastType + "<" + DestTypeString + ">").str(); - ReplaceWithCast(std::move(CastText)); - }; - auto ReplaceWithCtorCall = [&]() { - std::string CastText; - if (!DestTypeAsWritten.hasQualifiers() && - DestTypeAsWritten->isRecordType() && - !DestTypeAsWritten->isElaboratedTypeSpecifier()) { - Diag << "constructor call syntax"; - CastText = DestTypeString.str(); // FIXME: Validate DestTypeString, maybe. - } else { - Diag << "static_cast"; - CastText = ("static_cast<" + DestTypeString + ">").str(); - } - ReplaceWithCast(std::move(CastText)); + ReplaceWithCast((CastType + "<" + DestTypeString + ">").str()); }; // Suggest appropriate C++ cast. See [expr.cast] for cast notation semantics. @@ -168,13 +149,26 @@ void AvoidCStyleCastsCheck::check(const ReplaceWithNamedCast("static_cast"); return; case CK_ConstructorConversion: - ReplaceWithCtorCall(); + if (!DestTypeAsWritten.hasQualifiers() && + DestTypeAsWritten->isRecordType() && + !DestTypeAsWritten->isElaboratedTypeSpecifier()) { + Diag << "constructor call syntax"; + // FIXME: Validate DestTypeString, maybe. + ReplaceWithCast(DestTypeString.str()); + } else { + ReplaceWithNamedCast("static_cast"); + } return; case CK_NoOp: if (FnToFnCast) { ReplaceWithNamedCast("static_cast"); return; } + if (SourceType == DestType) { + Diag << "static_cast (if needed, the cast may be redundant)"; + ReplaceWithCast(("static_cast<" + DestTypeString + ">").str()); + return; + } if (needsConstCast(SourceType, DestType) && pointedUnqualifiedTypesAreEqual(SourceType, DestType)) { ReplaceWithNamedCast("const_cast"); @@ -204,7 +198,10 @@ void AvoidCStyleCastsCheck::check(const case CK_BitCast: // FIXME: Suggest const_cast<...>(reinterpret_cast<...>(...)) replacement. if (!needsConstCast(SourceType, DestType)) { - ReplaceWithNamedCast("reinterpret_cast"); + if (SourceType->isVoidPointerType()) + ReplaceWithNamedCast("static_cast"); + else + ReplaceWithNamedCast("reinterpret_cast"); return; } break; Modified: clang-tools-extra/trunk/test/clang-tidy/google-readability-casting.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-casting.cpp?rev=296753&r1=296752&r2=296753&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/google-readability-casting.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/google-readability-casting.cpp Thu Mar 2 09:27:34 2017 @@ -15,14 +15,14 @@ void f(int a, double b, const char *cpc, typedef const char *Typedef2; Typedef1 t1; (Typedef2)t1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: possibly redundant cast between typedefs of the same type [google-readability-casting] - // CHECK-FIXES: {{^}} (Typedef2)t1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C-style casts are discouraged; use static_cast (if needed, the cast may be redundant) [google-readability-casting] + // CHECK-FIXES: {{^}} static_cast<Typedef2>(t1); (const char*)t1; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: possibly redundant cast {{.*}} - // CHECK-FIXES: {{^}} (const char*)t1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: {{.*}}; use static_cast (if needed + // CHECK-FIXES: {{^}} static_cast<const char*>(t1); (Typedef1)cpc; - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: possibly redundant cast {{.*}} - // CHECK-FIXES: {{^}} (Typedef1)cpc; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: {{.*}}; use static_cast (if needed + // CHECK-FIXES: {{^}} static_cast<Typedef1>(cpc); (Typedef1)t1; // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant cast to the same type // CHECK-FIXES: {{^}} t1; @@ -30,64 +30,79 @@ void f(int a, double b, const char *cpc, char *pc = (char*)cpc; // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: C-style casts are discouraged; use const_cast [google-readability-casting] // CHECK-FIXES: char *pc = const_cast<char*>(cpc); + typedef char Char; + Char *pChar = (Char*)pc; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}}; use static_cast (if needed + // CHECK-FIXES: {{^}} Char *pChar = static_cast<Char*>(pc); + + (Char)*cpc; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: {{.*}}; use static_cast (if needed + // CHECK-FIXES: {{^}} static_cast<Char>(*cpc); + + (char)*pChar; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: {{.*}}; use static_cast (if needed + // CHECK-FIXES: {{^}} static_cast<char>(*pChar); + + (const char*)cpv; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: {{.*}}; use static_cast [ + // CHECK-FIXES: static_cast<const char*>(cpv); char *pc2 = (char*)(cpc + 33); - // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}; use const_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}; 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: {{.*}}; use const_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: {{.*}}; use const_cast [ // CHECK-FIXES: char &rc = const_cast<char&>(crc); char &rc2 = (char&)*cpc; - // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}; use const_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}; 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: {{.*}}; use const_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: {{.*}}; use const_cast [ // CHECK-FIXES: char ****ppppc = const_cast<char****>(ppcpcpc); char ***pppc = (char***)*(ppcpcpc); - // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: {{.*}}; use const_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: {{.*}}; use const_cast [ // CHECK-FIXES: char ***pppc = const_cast<char***>(*(ppcpcpc)); char ***pppc2 = (char***)(*ppcpcpc); - // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: {{.*}}; use const_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: {{.*}}; use const_cast [ // CHECK-FIXES: char ***pppc2 = const_cast<char***>(*ppcpcpc); char *pc5 = (char*)(const char*)(cpv); - // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}; use const_cast {{.*}} - // CHECK-MESSAGES: :[[@LINE-2]]:22: warning: {{.*}}; use reinterpret_cast {{.*}} - // CHECK-FIXES: char *pc5 = const_cast<char*>(reinterpret_cast<const char*>(cpv)); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}; use const_cast [ + // CHECK-MESSAGES: :[[@LINE-2]]:22: warning: {{.*}}; use static_cast [ + // CHECK-FIXES: char *pc5 = const_cast<char*>(static_cast<const char*>(cpv)); int b1 = (int)b; - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{.*}}; use static_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{.*}}; use static_cast [ // CHECK-FIXES: int b1 = static_cast<int>(b); Y *pB = (Y*)pX; - // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}; use static_cast/const_cast/reinterpret_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}; use static_cast/const_cast/reinterpret_cast [ Y &rB = (Y&)*pX; - // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}; use static_cast/const_cast/reinterpret_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}; use static_cast/const_cast/reinterpret_cast [ const char *pc3 = (const char*)cpv; - // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}}; use reinterpret_cast {{.*}} - // CHECK-FIXES: const char *pc3 = reinterpret_cast<const char*>(cpv); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}}; use static_cast [ + // CHECK-FIXES: const char *pc3 = static_cast<const char*>(cpv); char *pc4 = (char*)cpv; - // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}; use static_cast/const_cast/reinterpret_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}}; use static_cast/const_cast/reinterpret_cast [ // CHECK-FIXES: char *pc4 = (char*)cpv; b1 = (int)Enum1; - // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: {{.*}}; use static_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: {{.*}}; use static_cast [ // CHECK-FIXES: b1 = static_cast<int>(Enum1); Enum e = (Enum)b1; - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{.*}}; use static_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{.*}}; use static_cast [ // CHECK-FIXES: Enum e = static_cast<Enum>(b1); - // CHECK-MESSAGES-NOT: warning: int b2 = int(b); int b3 = static_cast<double>(b); int b4 = b; @@ -99,7 +114,7 @@ void f(int a, double b, const char *cpc, template <typename T> void template_function(T t, int n) { int i = (int)t; - // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}; use static_cast/const_cast/reinterpret_cast {{.*}} + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}; 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits