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 <cour...@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 <cour...@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 <cour...@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 + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits