[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 llvm-svn: 296753
This commit is contained in:
Alexander Kornienko
parent
e5027cfbcc
commit
ce482d9269
|
|
@ -35,19 +35,16 @@ void AvoidCStyleCastsCheck::registerMatchers(
|
|||
}
|
||||
|
||||
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 pointedUnqualifiedTypesAreEqual(QualType T1, QualType T2) {
|
|||
|
||||
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 MatchFinder::MatchResult &Result) {
|
|||
|
||||
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 MatchFinder::MatchResult &Result) {
|
|||
<< 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 MatchFinder::MatchResult &Result) {
|
|||
// 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 MatchFinder::MatchResult &Result) {
|
|||
};
|
||||
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 MatchFinder::MatchResult &Result) {
|
|||
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 MatchFinder::MatchResult &Result) {
|
|||
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;
|
||||
|
|
|
|||
|
|
@ -15,14 +15,14 @@ void f(int a, double b, const char *cpc, const void *cpv, X *pX) {
|
|||
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, const void *cpv, X *pX) {
|
|||
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, const void *cpv, X *pX) {
|
|||
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
|
||||
|
|
|
|||
Loading…
Reference in New Issue