carlosgalvezp updated this revision to Diff 390191.
carlosgalvezp added a comment.

Add extra test for template functions that use the functional cast. It should 
only warn when T is not a class type.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114427/new/

https://reviews.llvm.org/D114427

Files:
  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

Index: clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp
@@ -143,11 +143,10 @@
   // 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,43 @@
   // 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) {}
+};
+
+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);
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -133,9 +133,11 @@
 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 ``google-readability-casting`` to diagnose and fix functional casts, to achieve feature
+  parity with the corresponding ``cpplint.py`` check.
 
 Removed checks
 ^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -31,6 +31,11 @@
           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 @@
   return T1.getUnqualifiedType() == T2.getUnqualifiedType();
 }
 
+static clang::CharSourceRange getReplaceRange(const CStyleCastExpr *CastExpr) {
+  return CharSourceRange::getCharRange(
+      CastExpr->getLParenLoc(), CastExpr->getSubExprAsWritten()->getBeginLoc());
+}
+
+static clang::CharSourceRange
+getReplaceRange(const CXXFunctionalCastExpr *CastExpr) {
+  return CharSourceRange::getCharRange(CastExpr->getBeginLoc(),
+                                       CastExpr->getLParenLoc());
+}
+
+static StringRef getDestTypeString(const SourceManager &SM,
+                                   const LangOptions &LangOpts,
+                                   const CStyleCastExpr *CastExpr) {
+  return Lexer::getSourceText(
+      CharSourceRange::getTokenRange(
+          CastExpr->getLParenLoc().getLocWithOffset(1),
+          CastExpr->getRParenLoc().getLocWithOffset(-1)),
+      SM, LangOpts);
+}
+
+static StringRef getDestTypeString(const SourceManager &SM,
+                                   const LangOptions &LangOpts,
+                                   const CXXFunctionalCastExpr *CastExpr) {
+  return Lexer::getSourceText(
+      CharSourceRange::getTokenRange(
+          CastExpr->getBeginLoc(),
+          CastExpr->getLParenLoc().getLocWithOffset(-1)),
+      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,10 @@
   const QualType SourceType = SourceTypeAsWritten.getCanonicalType();
   const QualType DestType = DestTypeAsWritten.getCanonicalType();
 
-  auto ReplaceRange = CharSourceRange::getCharRange(
-      CastExpr->getLParenLoc(), CastExpr->getSubExprAsWritten()->getBeginLoc());
+  CharSourceRange ReplaceRange =
+      isa<CStyleCastExpr>(CastExpr)
+          ? getReplaceRange(dyn_cast<const CStyleCastExpr>(CastExpr))
+          : getReplaceRange(dyn_cast<const CXXFunctionalCastExpr>(CastExpr));
 
   bool FnToFnCast =
       IsFunction(SourceTypeAsWritten) && IsFunction(DestTypeAsWritten);
@@ -125,17 +163,18 @@
   // 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());
+      isa<CStyleCastExpr>(CastExpr)
+          ? getDestTypeString(SM, getLangOpts(),
+                              dyn_cast<const CStyleCastExpr>(CastExpr))
+          : getDestTypeString(SM, getLangOpts(),
+                              dyn_cast<const CXXFunctionalCastExpr>(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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to