[clang-tools-extra] r295544 - [clang-tidy] google-readability-casting: Handle user-defined conversions
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Sat Feb 18 01:45:01 PST 2017
Author: alexfh
Date: Sat Feb 18 03:45:00 2017
New Revision: 295544
URL: http://llvm.org/viewvc/llvm-project?rev=295544&view=rev
Log:
[clang-tidy] google-readability-casting: Handle user-defined conversions
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=295544&r1=295543&r2=295544&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp Sat Feb 18 03:45:00 2017
@@ -35,25 +35,28 @@ void AvoidCStyleCastsCheck::registerMatc
}
static 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;
+ for (;;) {
+ 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 pointedTypesAreEqual(QualType SourceType, QualType DestType) {
- SourceType = SourceType.getNonReferenceType();
- DestType = DestType.getNonReferenceType();
- while (SourceType->isPointerType() && DestType->isPointerType()) {
- SourceType = SourceType->getPointeeType();
- DestType = DestType->getPointeeType();
+static bool pointedUnqualifiedTypesAreEqual(QualType T1, QualType T2) {
+ while ((T1->isPointerType() && T2->isPointerType()) ||
+ (T1->isReferenceType() && T2->isReferenceType())) {
+ T1 = T1->getPointeeType();
+ T2 = T2->getPointeeType();
}
- return SourceType.getUnqualifiedType() == DestType.getUnqualifiedType();
+ return T1.getUnqualifiedType() == T2.getUnqualifiedType();
}
void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
@@ -67,35 +70,38 @@ void AvoidCStyleCastsCheck::check(const
// Casting to void is an idiomatic way to mute "unused variable" and similar
// warnings.
- if (CastExpr->getTypeAsWritten()->isVoidType())
+ if (CastExpr->getCastKind() == CK_ToVoid)
return;
- QualType SourceType = CastExpr->getSubExprAsWritten()->getType();
- QualType DestType = CastExpr->getTypeAsWritten();
-
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 && !FnToFnCast) {
- diag(CastExpr->getLocStart(),
- "possibly redundant cast between typedefs of the same type");
- return;
+ const QualType DestTypeAsWritten = CastExpr->getTypeAsWritten();
+ const QualType SourceTypeAsWritten = CastExpr->getSubExprAsWritten()->getType();
+ const QualType SourceType = SourceTypeAsWritten.getCanonicalType();
+ const QualType DestType = DestTypeAsWritten.getCanonicalType();
+
+ bool FnToFnCast =
+ isFunction(SourceTypeAsWritten) && isFunction(DestTypeAsWritten);
+
+ if (CastExpr->getCastKind() == CK_NoOp && !FnToFnCast) {
+ // 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 (SourceTypeAsWritten == DestTypeAsWritten) {
+ diag(CastExpr->getLocStart(), "redundant cast to the same type")
+ << 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++.
@@ -126,11 +132,8 @@ void AvoidCStyleCastsCheck::check(const
auto Diag =
diag(CastExpr->getLocStart(), "C-style casts are discouraged; use %0");
- auto ReplaceWithCast = [&](StringRef CastType) {
- Diag << CastType;
-
+ auto ReplaceWithCast = [&](std::string CastText) {
const Expr *SubExpr = CastExpr->getSubExprAsWritten()->IgnoreImpCasts();
- std::string CastText = (CastType + "<" + DestTypeString + ">").str();
if (!isa<ParenExpr>(SubExpr)) {
CastText.push_back('(');
Diag << FixItHint::CreateInsertion(
@@ -140,28 +143,52 @@ void AvoidCStyleCastsCheck::check(const
}
Diag << FixItHint::CreateReplacement(ParenRange, CastText);
};
+ 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));
+ };
// Suggest appropriate C++ cast. See [expr.cast] for cast notation semantics.
switch (CastExpr->getCastKind()) {
case CK_FunctionToPointerDecay:
- ReplaceWithCast("static_cast");
+ ReplaceWithNamedCast("static_cast");
+ return;
+ case CK_ConstructorConversion:
+ ReplaceWithCtorCall();
return;
case CK_NoOp:
if (FnToFnCast) {
- ReplaceWithCast("static_cast");
+ ReplaceWithNamedCast("static_cast");
return;
}
if (needsConstCast(SourceType, DestType) &&
- pointedTypesAreEqual(SourceType, DestType)) {
- ReplaceWithCast("const_cast");
+ pointedUnqualifiedTypesAreEqual(SourceType, DestType)) {
+ ReplaceWithNamedCast("const_cast");
return;
}
- if (DestType->isReferenceType() &&
- (SourceType.getNonReferenceType() ==
- DestType.getNonReferenceType().withConst() ||
- SourceType.getNonReferenceType() == DestType.getNonReferenceType())) {
- ReplaceWithCast("const_cast");
- return;
+ if (DestType->isReferenceType()) {
+ QualType Dest = DestType.getNonReferenceType();
+ QualType Source = SourceType.getNonReferenceType();
+ if (Source == Dest.withConst() ||
+ SourceType.getNonReferenceType() == DestType.getNonReferenceType()) {
+ ReplaceWithNamedCast("const_cast");
+ return;
+ }
+ break;
}
// FALLTHROUGH
case clang::CK_IntegralCast:
@@ -170,14 +197,14 @@ void AvoidCStyleCastsCheck::check(const
// still retained.
if ((SourceType->isBuiltinType() || SourceType->isEnumeralType()) &&
(DestType->isBuiltinType() || DestType->isEnumeralType())) {
- ReplaceWithCast("static_cast");
+ ReplaceWithNamedCast("static_cast");
return;
}
break;
case CK_BitCast:
// FIXME: Suggest const_cast<...>(reinterpret_cast<...>(...)) replacement.
if (!needsConstCast(SourceType, DestType)) {
- ReplaceWithCast("reinterpret_cast");
+ 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=295544&r1=295543&r2=295544&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 Sat Feb 18 03:45:00 2017
@@ -222,3 +222,43 @@ void function_casts() {
FnPtrVoid correct2 = static_cast<void (*)()>(&overloaded_function);
FnRefInt correct3 = static_cast<void (&)(int)>(overloaded_function);
}
+
+struct S {
+ S(const char *);
+};
+struct ConvertibleToS {
+ operator S() const;
+};
+struct ConvertibleToSRef {
+ operator const S&() const;
+};
+
+void conversions() {
+ //auto s1 = (const S&)"";
+ // C HECK-MESSAGES: :[[@LINE-1]]:10: warning: C-style casts are discouraged; use static_cast [
+ // C HECK-FIXES: S s1 = static_cast<const S&>("");
+ auto s2 = (S)"";
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: C-style casts are discouraged; use constructor call syntax [
+ // CHECK-FIXES: auto s2 = S("");
+ auto s2a = (struct S)"";
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: C-style casts are discouraged; use static_cast [
+ // CHECK-FIXES: auto s2a = static_cast<struct S>("");
+ ConvertibleToS c;
+ auto s3 = (const S&)c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast [
+ // CHECK-FIXES: auto s3 = (const S&)c;
+ // FIXME: This should be a static_cast
+ // C HECK-FIXES: auto s3 = static_cast<const S&>(c);
+ auto s4 = (S)c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: C-style casts are discouraged; use constructor call syntax [
+ // CHECK-FIXES: auto s4 = S(c);
+ ConvertibleToSRef cr;
+ auto s5 = (const S&)cr;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast [
+ // CHECK-FIXES: auto s5 = (const S&)cr;
+ // FIXME: This should be a static_cast
+ // C HECK-FIXES: auto s5 = static_cast<const S&>(cr);
+ auto s6 = (S)cr;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: C-style casts are discouraged; use constructor call syntax [
+ // CHECK-FIXES: auto s6 = S(cr);
+}
More information about the cfe-commits
mailing list