[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

Author: carlosgalvezp

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,

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
 ^^^^^^^^^^^^^^

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
+}