[clang-tools-extra] [clang-tidy][cppcoreguidelines-missing-std-forward] Do not warn when the parameter is used in a `static_cast`. (PR #99477)

Clement Courbet via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 19 01:39:45 PDT 2024


https://github.com/legrosbuffle updated https://github.com/llvm/llvm-project/pull/99477

>From b423b26cba90288912b3377c39ab4207c9fc95dc Mon Sep 17 00:00:00 2001
From: Clement Courbet <courbet at google.com>
Date: Thu, 18 Jul 2024 11:47:56 +0000
Subject: [PATCH 1/2] [clang-tidy][cppcoreguidelines-missing-std-forward] Do
 not warn when the parameter is used in a `static_cast`.

This provides a way to inform the check that we're intending to use an
the forwarding reference as a specific reference category

Fixes #99474.
---
 .../MissingStdForwardCheck.cpp                  |  8 ++++++--
 clang-tools-extra/docs/ReleaseNotes.rst         |  5 +++++
 .../cppcoreguidelines/missing-std-forward.rst   |  3 +++
 .../cppcoreguidelines/missing-std-forward.cpp   | 17 +++++++++++++++++
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
index bbb35228ce47f..726e9fffc628b 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
@@ -129,6 +129,9 @@ void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) {
       unless(anyOf(hasAncestor(typeLoc()),
                    hasAncestor(expr(hasUnevaluatedContext())))));
 
+  auto StaticCast = cxxStaticCastExpr(
+      hasSourceExpression(declRefExpr(to(equalsBoundNode("param")))));
+
   Finder->addMatcher(
       parmVarDecl(
           parmVarDecl().bind("param"), hasIdentifier(),
@@ -136,8 +139,9 @@ void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) {
           hasAncestor(functionDecl().bind("func")),
           hasAncestor(functionDecl(
               isDefinition(), equalsBoundNode("func"), ToParam,
-              unless(anyOf(isDeleted(),
-                           hasDescendant(std::move(ForwardCallMatcher))))))),
+              unless(anyOf(isDeleted(), hasDescendant(expr(
+                                            anyOf(std::move(ForwardCallMatcher),
+                                                  std::move(StaticCast))))))))),
       this);
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a23483e6df6d2..c3f41811fa7d7 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -518,6 +518,11 @@ Changes in existing checks
   usages of ``std::string_view::compare``. Added a `StringLikeClasses` option
   to detect usages of ``compare`` method in custom string-like classes.
 
+- Improved :doc:`cppcoreguidelines-missing-std-forward
+  <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check to allow
+  using ``static_cast<T&>`` to explicitly convey the intention of using a
+  forwarding reference as an lvalue reference.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
index 0c311b59a5d5a..12765b45088de 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
@@ -38,3 +38,6 @@ Example:
 This check implements `F.19
 <http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-forward>`_
 from the C++ Core Guidelines.
+
+Users who want to use the forwarding reference as an lvalue reference can convey
+the intention by using ``static_cast<T&>(t)``.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
index 8116db58c937d..519d2822948bb 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -211,3 +211,20 @@ template<typename F>
 void unused_argument3(F&& _) {}
 
 } // namespace unused_arguments
+
+namespace escape_hatch {
+
+template<typename T>
+void used_as_lvalue_on_purpose(T&& t) {
+  static_cast<T&>(t);
+  static_cast<const T&>(t);
+}
+
+template<typename T>
+void used_as_rvalue_on_purpose(T&& t) {
+  static_cast<const T&&>(t);
+  // Typically used as another spelling for `std::forward`.
+  static_cast<T&&>(t);
+}
+
+}

>From 64f957fead4d409e0835a6f032330f73983b975f Mon Sep 17 00:00:00 2001
From: Clement Courbet <courbet at google.com>
Date: Fri, 19 Jul 2024 08:20:30 +0000
Subject: [PATCH 2/2] Add a check option to ignore `static_cast`s.

---
 .../MissingStdForwardCheck.cpp                |  6 ++--
 clang-tools-extra/docs/ReleaseNotes.rst       |  8 ++---
 .../cppcoreguidelines/missing-std-forward.rst | 12 ++++++--
 .../missing-std-forward-casts.cpp             | 29 +++++++++++++++++++
 .../cppcoreguidelines/missing-std-forward.cpp | 15 +++-------
 5 files changed, 49 insertions(+), 21 deletions(-)
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward-casts.cpp

diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
index 726e9fffc628b..9490e4873be84 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
@@ -129,8 +129,10 @@ void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) {
       unless(anyOf(hasAncestor(typeLoc()),
                    hasAncestor(expr(hasUnevaluatedContext())))));
 
-  auto StaticCast = cxxStaticCastExpr(
-      hasSourceExpression(declRefExpr(to(equalsBoundNode("param")))));
+  auto StaticCast = Options.get("IgnoreStaticCasts", false)
+                        ? cxxStaticCastExpr(hasSourceExpression(
+                              declRefExpr(to(equalsBoundNode("param")))))
+                        : cxxStaticCastExpr(unless(anything()));
 
   Finder->addMatcher(
       parmVarDecl(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c3f41811fa7d7..b005af501904b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -320,7 +320,8 @@ Changes in existing checks
   <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check by no longer
   giving false positives for deleted functions, by fixing false negatives when only
   a few parameters are forwarded and by ignoring parameters without a name (unused
-  arguments).
+  arguments). Add an option to allow using ``static_cast<T&>`` to explicitly
+  convey the intention of using a forwarding reference as an lvalue reference.
 
 - Improved :doc:`cppcoreguidelines-owning-memory
   <clang-tidy/checks/cppcoreguidelines/owning-memory>` check to properly handle
@@ -518,11 +519,6 @@ Changes in existing checks
   usages of ``std::string_view::compare``. Added a `StringLikeClasses` option
   to detect usages of ``compare`` method in custom string-like classes.
 
-- Improved :doc:`cppcoreguidelines-missing-std-forward
-  <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check to allow
-  using ``static_cast<T&>`` to explicitly convey the intention of using a
-  forwarding reference as an lvalue reference.
-
 Removed checks
 ^^^^^^^^^^^^^^
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
index 12765b45088de..b1adc9eedc568 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
@@ -39,5 +39,13 @@ This check implements `F.19
 <http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-forward>`_
 from the C++ Core Guidelines.
 
-Users who want to use the forwarding reference as an lvalue reference can convey
-the intention by using ``static_cast<T&>(t)``.
+
+Options
+-------
+
+.. option:: IgnoreStaticCasts
+
+Boolean flag to allow users who want to use the forwarding reference as an
+lvalue reference to convey he intention by using ``static_cast<T&>(t)`` to
+disable warning. Default value is `false`.
+
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward-casts.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward-casts.cpp
new file mode 100644
index 0000000000000..d3c8ef79ab8b8
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward-casts.cpp
@@ -0,0 +1,29 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-missing-std-forward %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN:     {cppcoreguidelines-missing-std-forward.IgnoreStaticCasts: true }}" \
+// RUN: -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+
+template <typename T> struct remove_reference      { using type = T; };
+template <typename T> struct remove_reference<T&>  { using type = T; };
+template <typename T> struct remove_reference<T&&> { using type = T; };
+
+template <typename T> using remove_reference_t = typename remove_reference<T>::type;
+
+template <typename T> constexpr T &&forward(remove_reference_t<T> &t) noexcept;
+template <typename T> constexpr T &&forward(remove_reference_t<T> &&t) noexcept;
+
+} // namespace std
+// NOLINTEND
+
+namespace in_static_cast {
+
+template<typename T>
+void static_cast_to_lvalue_ref(T&& t) {
+  static_cast<T&>(t);
+}
+
+} // namespace in_static_cast
+
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
index 519d2822948bb..93b9b456f6077 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -212,19 +212,12 @@ void unused_argument3(F&& _) {}
 
 } // namespace unused_arguments
 
-namespace escape_hatch {
+namespace in_static_cast {
 
 template<typename T>
-void used_as_lvalue_on_purpose(T&& t) {
+void static_cast_to_lvalue_ref(T&& t) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
   static_cast<T&>(t);
-  static_cast<const T&>(t);
 }
 
-template<typename T>
-void used_as_rvalue_on_purpose(T&& t) {
-  static_cast<const T&&>(t);
-  // Typically used as another spelling for `std::forward`.
-  static_cast<T&&>(t);
-}
-
-}
+} // namespace in_static_cast



More information about the cfe-commits mailing list