This revision was automatically updated to reflect the committed changes. Closed by commit rG5902bb9584d6: [clang-tidy] Implement cppcoreguidelines F.19 (authored by ccotter, committed by PiotrZSL).
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst clang-tools-extra/docs/clang-tidy/checks/list.rst clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp @@ -0,0 +1,150 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-missing-std-forward %t -- -- -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; +template <typename T> constexpr remove_reference_t<T> &&move(T &&x); + +} // namespace std +// NOLINTEND + +struct S { + S(); + S(const S&); + S(S&&) noexcept; + S& operator=(const S&); + S& operator=(S&&) noexcept; +}; + +template <class... Ts> +void consumes_all(Ts&&...); + +namespace positive_cases { + +template <class T> +void does_not_forward(T&& t) { + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + T other = t; +} + +template <class T> +void does_not_forward_invoked(T&& t) { + // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + T other = t(); +} + +template <class T> +void forwards_pairwise(T&& t) { + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + auto first = std::forward<T>(t.first); + auto second = std::forward<T>(t.second); +} + +template <class... Ts> +void does_not_forward_pack(Ts&&... ts) { + // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + consumes_all(ts...); +} + +template <class T> +class AClass { + + template <class U> + AClass(U&& u) : data(u) {} + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + + template <class U> + AClass& operator=(U&& u) { } + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + + template <class U> + void mixed_params(T&& t, U&& u) { + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + T other1 = std::move(t); + U other2 = std::move(u); + } + + T data; +}; + +template <class T> +void does_not_forward_in_evaluated_code(T&& t) { + // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + using result_t = decltype(std::forward<T>(t)); + unsigned len = sizeof(std::forward<T>(t)); + T other = t; +} + +template <class T> +void lambda_value_capture(T&& t) { + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + [=]() { T other = std::forward<T>(t); }; +} + +template <class T> +void lambda_value_reference(T&& t) { + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + [&]() { T other = std::forward<T>(t); }; +} + +} // namespace positive_cases + +namespace negative_cases { + +template <class T> +void just_a_decl(T&&t); + +template <class T> +void does_forward(T&& t) { + T other = std::forward<T>(t); +} + +template <class... Ts> +void does_forward_pack(Ts&&... ts) { + consumes_all(std::forward<Ts>(ts)...); +} + +void pass_by_value(S s) { + S other = std::move(s); +} + +void lvalue_ref(S& s) { + S other = std::move(s); +} + +void rvalue_ref(S&& s) { + S other = std::move(s); +} + +template <class T> +void templated_rvalue_ref(std::remove_reference_t<T>&& t) { + T other = std::move(t); +} + +template <class T> +class AClass { + + template <class U> + AClass(U&& u) : data(std::forward<U>(u)) {} + + template <class U> + AClass& operator=(U&& u) { + data = std::forward<U>(u); + } + + void rvalue_ref(T&& t) { + T other = std::move(t); + } + + T data; +}; + +} // namespace negative_cases Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -191,6 +191,7 @@ `cppcoreguidelines-interfaces-global-init <cppcoreguidelines/interfaces-global-init.html>`_, `cppcoreguidelines-macro-usage <cppcoreguidelines/macro-usage.html>`_, `cppcoreguidelines-misleading-capture-default-by-value <cppcoreguidelines/misleading-capture-default-by-value.html>`_, "Yes" + `cppcoreguidelines-missing-std-forward <cppcoreguidelines/missing-std-forward.html>`_, `cppcoreguidelines-narrowing-conversions <cppcoreguidelines/narrowing-conversions.html>`_, `cppcoreguidelines-no-malloc <cppcoreguidelines/no-malloc.html>`_, `cppcoreguidelines-owning-memory <cppcoreguidelines/owning-memory.html>`_, Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst @@ -0,0 +1,39 @@ +.. title:: clang-tidy - cppcoreguidelines-missing-std-forward + +cppcoreguidelines-missing-std-forward +===================================== + +Warns when a forwarding reference parameter is not forwarded inside the +function body. + +Example: + +.. code-block:: c++ + + template <class T> + void wrapper(T&& t) { + impl(std::forward<T>(t), 1, 2); // Correct + } + + template <class T> + void wrapper2(T&& t) { + impl(t, 1, 2); // Oops - should use std::forward<T>(t) + } + + template <class T> + void wrapper3(T&& t) { + impl(std::move(t), 1, 2); // Also buggy - should use std::forward<T>(t) + } + + template <class F> + void wrapper_function(F&& f) { + std::forward<F>(f)(1, 2); // Correct + } + + template <class F> + void wrapper_function2(F&& f) { + f(1, 2); // Incorrect - may not invoke the desired qualified function operator + } + +This check implements +`CppCoreGuideline F.19 <http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-forward>`_. Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -138,6 +138,12 @@ Warns when lambda specify a by-value capture default and capture ``this``. +- New :doc:`cppcoreguidelines-missing-std-forward + <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check. + + Warns when a forwarding reference parameter is not forwarded within the + function body. + - New :doc:`cppcoreguidelines-rvalue-reference-param-not-moved <clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved>` check. Index: clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h @@ -0,0 +1,38 @@ +//===--- MissingStdForwardCheck.h - clang-tidy ------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MISSINGSTDFORWARDCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MISSINGSTDFORWARDCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::cppcoreguidelines { + +/// Warns when a function accepting a forwarding reference does anything besides +/// forwarding (with std::forward) the parameter in the body of the function. +/// This check implement CppCoreGuideline F.19. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/missing-std-forward.html +class MissingStdForwardCheck : public ClangTidyCheck { +public: + MissingStdForwardCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } +}; + +} // namespace clang::tidy::cppcoreguidelines + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_MISSINGSTDFORWARDCHECK_H Index: clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp @@ -0,0 +1,90 @@ +//===--- MissingStdForwardCheck.cpp - clang-tidy --------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "MissingStdForwardCheck.h" +#include "../utils/Matchers.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/ExprConcepts.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::cppcoreguidelines { + +namespace { + +using matchers::hasUnevaluatedContext; + +AST_MATCHER_P(QualType, possiblyPackExpansionOf, + ast_matchers::internal::Matcher<QualType>, InnerMatcher) { + return InnerMatcher.matches(Node.getNonPackExpansionType(), Finder, Builder); +} + +AST_MATCHER(ParmVarDecl, isTemplateTypeParameter) { + ast_matchers::internal::Matcher<QualType> Inner = possiblyPackExpansionOf( + qualType(rValueReferenceType(), + references(templateTypeParmType( + hasDeclaration(templateTypeParmDecl()))), + unless(references(qualType(isConstQualified()))))); + if (!Inner.matches(Node.getType(), Finder, Builder)) + return false; + + const auto *Function = dyn_cast<FunctionDecl>(Node.getDeclContext()); + if (!Function) + return false; + + const FunctionTemplateDecl *FuncTemplate = + Function->getDescribedFunctionTemplate(); + if (!FuncTemplate) + return false; + + QualType ParamType = + Node.getType().getNonPackExpansionType()->getPointeeType(); + const auto *TemplateType = ParamType->getAs<TemplateTypeParmType>(); + if (!TemplateType) + return false; + + return TemplateType->getDepth() == + FuncTemplate->getTemplateParameters()->getDepth(); +} + +} // namespace + +void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) { + auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param"))); + + auto ForwardCallMatcher = callExpr( + forCallable(equalsBoundNode("func")), argumentCountIs(1), + callee(unresolvedLookupExpr(hasAnyDeclaration( + namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))), + hasArgument(0, declRefExpr(to(equalsBoundNode("param"))).bind("ref")), + unless(anyOf(hasAncestor(typeLoc()), + hasAncestor(expr(hasUnevaluatedContext()))))); + + Finder->addMatcher( + parmVarDecl(parmVarDecl().bind("param"), isTemplateTypeParameter(), + hasAncestor(functionDecl().bind("func")), + hasAncestor(functionDecl( + isDefinition(), equalsBoundNode("func"), ToParam, + unless(hasDescendant(std::move(ForwardCallMatcher)))))), + this); +} + +void MissingStdForwardCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param"); + + if (!Param) + return; + + diag(Param->getLocation(), + "forwarding reference parameter %0 is never forwarded " + "inside the function body") + << Param; +} + +} // namespace clang::tidy::cppcoreguidelines Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -25,6 +25,7 @@ #include "InterfacesGlobalInitCheck.h" #include "MacroUsageCheck.h" #include "MisleadingCaptureDefaultByValueCheck.h" +#include "MissingStdForwardCheck.h" #include "NarrowingConversionsCheck.h" #include "NoMallocCheck.h" #include "OwningMemoryCheck.h" @@ -77,6 +78,8 @@ "cppcoreguidelines-macro-usage"); CheckFactories.registerCheck<MisleadingCaptureDefaultByValueCheck>( "cppcoreguidelines-misleading-capture-default-by-value"); + CheckFactories.registerCheck<MissingStdForwardCheck>( + "cppcoreguidelines-missing-std-forward"); CheckFactories.registerCheck<NarrowingConversionsCheck>( "cppcoreguidelines-narrowing-conversions"); CheckFactories.registerCheck<NoMallocCheck>("cppcoreguidelines-no-malloc"); Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -15,6 +15,7 @@ InterfacesGlobalInitCheck.cpp MacroUsageCheck.cpp MisleadingCaptureDefaultByValueCheck.cpp + MissingStdForwardCheck.cpp NarrowingConversionsCheck.cpp NoMallocCheck.cpp OwningMemoryCheck.cpp
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits