[clang-tools-extra] [clang-tidy] Fix handling of functional cast in google-readability-casting (PR #71650)

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Sun Jan 21 03:28:24 PST 2024


https://github.com/PiotrZSL updated https://github.com/llvm/llvm-project/pull/71650

>From b2da54fc89e99d3056ee80d47b8be2874916e02f Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Wed, 8 Nov 2023 09:38:39 +0000
Subject: [PATCH 1/3] [clang-tidy] Fix handling of functional cast in
 google-readability-casting

Fix issue with constructor call being interpreted
as functional cast and considered for a replacement
with static cast or being removed as redundant.
---
 .../clang-tidy/google/AvoidCStyleCastsCheck.cpp     | 13 ++++++++-----
 .../clang-tidy/google/AvoidCStyleCastsCheck.h       |  3 +++
 clang-tools-extra/docs/ReleaseNotes.rst             |  4 ++++
 .../checkers/google/readability-casting.cpp         | 13 +++----------
 4 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
index 714ac0ee54dafb3..3afb93ac7f7dd15 100644
--- a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -18,19 +18,22 @@ namespace clang::tidy::google::readability {
 
 void AvoidCStyleCastsCheck::registerMatchers(
     ast_matchers::MatchFinder *Finder) {
+
   Finder->addMatcher(
       cStyleCastExpr(
           // Filter out (EnumType)IntegerLiteral construct, which is generated
           // for non-type template arguments of enum types.
           // FIXME: Remove this once this is fixed in the AST.
-          unless(hasParent(substNonTypeTemplateParmExpr())),
-          // Avoid matches in template instantiations.
-          unless(isInTemplateInstantiation()))
+          unless(hasParent(substNonTypeTemplateParmExpr())))
           .bind("cast"),
       this);
+
   Finder->addMatcher(
-      cxxFunctionalCastExpr(unless(hasDescendant(cxxConstructExpr())),
-                            unless(hasDescendant(initListExpr())))
+      cxxFunctionalCastExpr(
+          hasDestinationType(hasCanonicalType(anyOf(
+              builtinType(), references(qualType()), pointsTo(qualType())))),
+          unless(
+              hasSourceExpression(anyOf(cxxConstructExpr(), initListExpr()))))
           .bind("cast"),
       this);
 }
diff --git a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.h b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.h
index 485640d230280ee..4267b896b6992c6 100644
--- a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.h
+++ b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.h
@@ -31,6 +31,9 @@ class AvoidCStyleCastsCheck : public ClangTidyCheck {
       : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace clang::tidy::google::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index fe8c7175d554c7b..f02f4bba8916bb6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -290,6 +290,10 @@ Changes in existing checks
   to ignore unused parameters when they are marked as unused and parameters of
   deleted functions and constructors.
 
+- Improved :doc:`google-readability-casting
+  <clang-tidy/checks/google/readability-casting>` check to ignore constructor
+  calls disguised as functional casts.
+
 - Improved :doc:`llvm-namespace-comment
   <clang-tidy/checks/llvm/namespace-comment>` check to provide fixes for
   ``inline`` namespaces in the same format as :program:`clang-format`.
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 e25463cf451b741..da49c3943d73222 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
@@ -322,17 +322,10 @@ void conversions() {
 }
 
 template <class T>
-T functional_cast_template_used_by_class(float i) {
+T functional_cast_template(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);
 };
@@ -356,8 +349,8 @@ void functional_casts() {
   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);
+  functional_cast_template<S2>(x);
+  functional_cast_template<int>(x);
 
   // New expressions are not functional casts
   auto w = new int(x);

>From bdc890dad78c78703e52d82d5d0ce956e3659319 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Wed, 8 Nov 2023 10:33:43 +0000
Subject: [PATCH 2/3] Add test

---
 .../test/clang-tidy/checkers/google/readability-casting.cpp     | 2 ++
 1 file changed, 2 insertions(+)

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 da49c3943d73222..fdc71167cd82b07 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
@@ -359,4 +359,6 @@ void functional_casts() {
   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
+
+  throw S2(5.0f);
 }

>From dc9548b7094e400838ac6249626fba73b9d499a2 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Sun, 21 Jan 2024 11:28:10 +0000
Subject: [PATCH 3/3] Remove empty line

---
 clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
index 3afb93ac7f7dd15..50d47a88a731ec8 100644
--- a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -18,7 +18,6 @@ namespace clang::tidy::google::readability {
 
 void AvoidCStyleCastsCheck::registerMatchers(
     ast_matchers::MatchFinder *Finder) {
-
   Finder->addMatcher(
       cStyleCastExpr(
           // Filter out (EnumType)IntegerLiteral construct, which is generated



More information about the cfe-commits mailing list