[clang-tools-extra] 5bbe501 - [clang-tidy] Warn on functional C-style casts
Carlos Galvez via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 29 23:31:52 PST 2021
Author: Carlos Galvez
Date: 2021-11-30T07:31:17Z
New Revision: 5bbe50148f3b515c170be22209395b72890f5b8c
URL: https://github.com/llvm/llvm-project/commit/5bbe50148f3b515c170be22209395b72890f5b8c
DIFF: https://github.com/llvm/llvm-project/commit/5bbe50148f3b515c170be22209395b72890f5b8c.diff
LOG: [clang-tidy] Warn on functional C-style casts
The google-readability-casting check is meant to be on par
with cpplint's readability/casting check, according to the
documentation. However it currently does not diagnose
functional casts, like:
float x = 1.5F;
int y = int(x);
This is detected by cpplint, however, and the guidelines
are clear that such a cast is only allowed when the type
is a class type (constructor call):
> You may use cast formats like `T(x)` only when `T` is a class type.
Therefore, update the clang-tidy check to check this
case.
Differential Revision: https://reviews.llvm.org/D114427
Added:
Modified:
clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
index a32603dc2cb0..82804a670959 100644
--- a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -31,6 +31,11 @@ void AvoidCStyleCastsCheck::registerMatchers(
unless(isInTemplateInstantiation()))
.bind("cast"),
this);
+ Finder->addMatcher(
+ cxxFunctionalCastExpr(unless(hasDescendant(cxxConstructExpr())),
+ unless(hasDescendant(initListExpr())))
+ .bind("cast"),
+ this);
}
static bool needsConstCast(QualType SourceType, QualType DestType) {
@@ -55,8 +60,39 @@ static bool pointedUnqualifiedTypesAreEqual(QualType T1, QualType T2) {
return T1.getUnqualifiedType() == T2.getUnqualifiedType();
}
+static clang::CharSourceRange getReplaceRange(const ExplicitCastExpr *Expr) {
+ if (const auto *CastExpr = dyn_cast<CStyleCastExpr>(Expr)) {
+ return CharSourceRange::getCharRange(
+ CastExpr->getLParenLoc(),
+ CastExpr->getSubExprAsWritten()->getBeginLoc());
+ } else if (const auto *CastExpr = dyn_cast<CXXFunctionalCastExpr>(Expr)) {
+ return CharSourceRange::getCharRange(CastExpr->getBeginLoc(),
+ CastExpr->getLParenLoc());
+ } else
+ llvm_unreachable("Unsupported CastExpr");
+}
+
+static StringRef getDestTypeString(const SourceManager &SM,
+ const LangOptions &LangOpts,
+ const ExplicitCastExpr *Expr) {
+ SourceLocation BeginLoc;
+ SourceLocation EndLoc;
+
+ if (const auto *CastExpr = dyn_cast<CStyleCastExpr>(Expr)) {
+ BeginLoc = CastExpr->getLParenLoc().getLocWithOffset(1);
+ EndLoc = CastExpr->getRParenLoc().getLocWithOffset(-1);
+ } else if (const auto *CastExpr = dyn_cast<CXXFunctionalCastExpr>(Expr)) {
+ BeginLoc = CastExpr->getBeginLoc();
+ EndLoc = CastExpr->getLParenLoc().getLocWithOffset(-1);
+ } else
+ llvm_unreachable("Unsupported CastExpr");
+
+ return Lexer::getSourceText(CharSourceRange::getTokenRange(BeginLoc, EndLoc),
+ SM, LangOpts);
+}
+
void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *CastExpr = Result.Nodes.getNodeAs<CStyleCastExpr>("cast");
+ const auto *CastExpr = Result.Nodes.getNodeAs<ExplicitCastExpr>("cast");
// Ignore casts in macros.
if (CastExpr->getExprLoc().isMacroID())
@@ -80,8 +116,7 @@ void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
const QualType SourceType = SourceTypeAsWritten.getCanonicalType();
const QualType DestType = DestTypeAsWritten.getCanonicalType();
- auto ReplaceRange = CharSourceRange::getCharRange(
- CastExpr->getLParenLoc(), CastExpr->getSubExprAsWritten()->getBeginLoc());
+ CharSourceRange ReplaceRange = getReplaceRange(CastExpr);
bool FnToFnCast =
IsFunction(SourceTypeAsWritten) && IsFunction(DestTypeAsWritten);
@@ -124,18 +159,14 @@ void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) {
// Leave type spelling exactly as it was (unlike
// getTypeAsWritten().getAsString() which would spell enum types 'enum X').
- StringRef DestTypeString =
- Lexer::getSourceText(CharSourceRange::getTokenRange(
- CastExpr->getLParenLoc().getLocWithOffset(1),
- CastExpr->getRParenLoc().getLocWithOffset(-1)),
- SM, getLangOpts());
+ StringRef DestTypeString = getDestTypeString(SM, getLangOpts(), CastExpr);
auto Diag =
diag(CastExpr->getBeginLoc(), "C-style casts are discouraged; use %0");
auto ReplaceWithCast = [&](std::string CastText) {
const Expr *SubExpr = CastExpr->getSubExprAsWritten()->IgnoreImpCasts();
- if (!isa<ParenExpr>(SubExpr)) {
+ if (!isa<ParenExpr>(SubExpr) && !isa<CXXFunctionalCastExpr>(CastExpr)) {
CastText.push_back('(');
Diag << FixItHint::CreateInsertion(
Lexer::getLocForEndOfToken(SubExpr->getEndLoc(), 0, SM,
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b73ea44a82bf..fa03d7eac99e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -133,9 +133,12 @@ New check aliases
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
-- Removed default setting `cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"`,
+- Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
to match the current state of the C++ Core Guidelines.
+- Updated :doc:`google-readability-casting
+ <clang-tidy/checks/google-readability-casting>` to diagnose and fix functional
+ casts, to achieve feature parity with the corresponding ``cpplint.py`` check.
Removed checks
^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp b/clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
index bddc28c2d81e..e25463cf451b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
@@ -143,11 +143,10 @@ void f(int a, double b, const char *cpc, const void *cpv, X *pX) {
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant cast to the same type
// CHECK-FIXES: {{^}} kZero;
- int b2 = int(b);
- int b3 = static_cast<double>(b);
- int b4 = b;
+ int b2 = static_cast<double>(b);
+ int b3 = b;
double aa = a;
- (void)b2;
+ (void)aa;
return (void)g();
}
@@ -321,3 +320,50 @@ void conversions() {
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: C-style casts are discouraged; use constructor call syntax [
// CHECK-FIXES: auto s6 = S(cr);
}
+
+template <class T>
+T functional_cast_template_used_by_class(float i) {
+ return T(i);
+}
+
+template <class T>
+T functional_cast_template_used_by_int(float i) {
+ return T(i);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C-style casts are discouraged; use static_cast
+ // CHECK-FIXES: return static_cast<T>(i);
+}
+
+struct S2 {
+ S2(float);
+};
+using T = S2;
+using U = S2 &;
+
+void functional_casts() {
+ float x = 1.5F;
+ auto y = int(x);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: C-style casts are discouraged; use static_cast
+ // CHECK-FIXES: auto y = static_cast<int>(x);
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wc++11-narrowing"
+ // This if fine, compiler will warn about implicit conversions with brace initialization
+ auto z = int{x};
+#pragma clang diagnostic pop
+
+ // Functional casts are allowed if S is of class type
+ const char *str = "foo";
+ auto s = S(str);
+
+ // Functional casts in template functions
+ functional_cast_template_used_by_class<S2>(x);
+ functional_cast_template_used_by_int<int>(x);
+
+ // New expressions are not functional casts
+ auto w = new int(x);
+
+ // Typedefs
+ S2 t = T(x); // OK, constructor call
+ S2 u = U(x); // NOK, it's a reinterpret_cast in disguise
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast
+}
More information about the cfe-commits
mailing list