[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
Mon Jul 22 02:38:01 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/3] [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/3] 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
>From 43b51e07cbd5d575a82f345913cf663b988907c6 Mon Sep 17 00:00:00 2001
From: Clement Courbet <courbet at google.com>
Date: Mon, 22 Jul 2024 08:58:52 +0000
Subject: [PATCH 3/3] only disable on cast-to-lvalue-ref. Add more tests.
---
.../MissingStdForwardCheck.cpp | 17 ++++--
.../missing-std-forward-casts.cpp | 50 ++++++++++++++++-
.../cppcoreguidelines/missing-std-forward.cpp | 55 ++++++++++++++++++-
3 files changed, 115 insertions(+), 7 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
index 9490e4873be84..3529f82acf9c2 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
@@ -25,9 +25,10 @@ AST_MATCHER_P(QualType, possiblyPackExpansionOf,
return InnerMatcher.matches(Node.getNonPackExpansionType(), Finder, Builder);
}
-AST_MATCHER(ParmVarDecl, isTemplateTypeParameter) {
+AST_MATCHER_P(ParmVarDecl, isForwardingReferenceType,
+ ast_matchers::internal::Matcher<RValueReferenceType>, InnerMatcher) {
ast_matchers::internal::Matcher<QualType> Inner = possiblyPackExpansionOf(
- qualType(rValueReferenceType(),
+ qualType(rValueReferenceType(InnerMatcher),
references(templateTypeParmType(
hasDeclaration(templateTypeParmDecl()))),
unless(references(qualType(isConstQualified())))));
@@ -130,14 +131,22 @@ void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) {
hasAncestor(expr(hasUnevaluatedContext())))));
auto StaticCast = Options.get("IgnoreStaticCasts", false)
- ? cxxStaticCastExpr(hasSourceExpression(
+ ? cxxStaticCastExpr(hasDestinationType(
+ lValueReferenceType(
+ pointee(
+ type(equalsBoundNode("qtype"))
+ )
+ )
+ ),
+ hasSourceExpression(
declRefExpr(to(equalsBoundNode("param")))))
: cxxStaticCastExpr(unless(anything()));
Finder->addMatcher(
parmVarDecl(
parmVarDecl().bind("param"), hasIdentifier(),
- unless(hasAttr(attr::Kind::Unused)), isTemplateTypeParameter(),
+ unless(hasAttr(attr::Kind::Unused)),
+ isForwardingReferenceType(pointee(type().bind("qtype"))),
hasAncestor(functionDecl().bind("func")),
hasAncestor(functionDecl(
isDefinition(), equalsBoundNode("func"), ToParam,
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
index d3c8ef79ab8b8..6e1d1756103de 100644
--- 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
@@ -15,15 +15,63 @@ template <typename T> using remove_reference_t = typename remove_reference<T>::t
template <typename T> constexpr T &&forward(remove_reference_t<T> &t) noexcept;
template <typename T> constexpr T &&forward(remove_reference_t<T> &&t) noexcept;
+template<typename T> using add_lvalue_reference_t = __add_lvalue_reference(T);
+
} // namespace std
// NOLINTEND
namespace in_static_cast {
template<typename T>
-void static_cast_to_lvalue_ref(T&& t) {
+void to_lvalue_ref(T&& t) {
static_cast<T&>(t);
}
+template<typename T>
+void to_const_lvalue_ref(T&& t) {
+ static_cast<const T&>(t);
+}
+
+template<typename T>
+void to_rvalue_ref(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ static_cast<T&&>(t);
+}
+
+template<typename T>
+void to_value(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ static_cast<T>(t);
+}
+
+template<typename T>
+void to_const_float_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<float&>(t);
+}
+
+template<typename T>
+void to_float(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ static_cast<float>(t);
+}
+
+template<typename T>
+void to_dependent(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ static_cast<std::add_lvalue_reference_t<T>>(t);
+}
+
+template<typename... T>
+void to_float_expanded(T&&... t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ (static_cast<float>(t), ...);
+}
+
+template<typename... T>
+void to_lvalue_ref_expanded(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 93b9b456f6077..70bcbfd473ff4 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
@@ -13,6 +13,8 @@ template <typename T> constexpr T &&forward(remove_reference_t<T> &t) noexcept;
template <typename T> constexpr T &&forward(remove_reference_t<T> &&t) noexcept;
template <typename T> constexpr remove_reference_t<T> &&move(T &&x);
+template<typename T> using add_lvalue_reference_t = __add_lvalue_reference(T);
+
} // namespace std
// NOLINTEND
@@ -215,9 +217,58 @@ void unused_argument3(F&& _) {}
namespace in_static_cast {
template<typename 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]
+void to_lvalue_ref(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
static_cast<T&>(t);
}
+template<typename T>
+void to_const_lvalue_ref(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ static_cast<const T&>(t);
+}
+
+template<typename T>
+void to_rvalue_ref(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ static_cast<T&&>(t);
+}
+
+template<typename T>
+void to_value(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ static_cast<T>(t);
+}
+
+template<typename T>
+void to_const_float_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<float&>(t);
+}
+
+template<typename T>
+void to_float(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ static_cast<float>(t);
+}
+
+template<typename T>
+void to_dependent(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ static_cast<std::add_lvalue_reference_t<T>>(t);
+}
+
+template<typename... T>
+void to_float_expanded(T&&... t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ (static_cast<float>(t), ...);
+}
+
+template<typename... T>
+void to_lvalue_ref_expanded(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<S&>(t), ...);
+}
+
} // namespace in_static_cast
+
More information about the cfe-commits
mailing list