[clang-tools-extra] 5902bb9 - [clang-tidy] Implement cppcoreguidelines F.19
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sat May 6 03:50:11 PDT 2023
Author: Chris Cotter
Date: 2023-05-06T10:49:23Z
New Revision: 5902bb9584d686789a80005b35de1b2c2e2ce0b4
URL: https://github.com/llvm/llvm-project/commit/5902bb9584d686789a80005b35de1b2c2e2ce0b4
DIFF: https://github.com/llvm/llvm-project/commit/5902bb9584d686789a80005b35de1b2c2e2ce0b4.diff
LOG: [clang-tidy] Implement cppcoreguidelines F.19
Warns when a function accepting a forwarding reference does anything besides
forwarding (with std::forward) the parameter in the body of the function.
Reviewed By: PiotrZSL
Differential Revision: https://reviews.llvm.org/D146921
Added:
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
Modified:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
index 700070d6783aa..b63f28fa3ddcd 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -15,6 +15,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule
InterfacesGlobalInitCheck.cpp
MacroUsageCheck.cpp
MisleadingCaptureDefaultByValueCheck.cpp
+ MissingStdForwardCheck.cpp
NarrowingConversionsCheck.cpp
NoMallocCheck.cpp
OwningMemoryCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
index c0f6a07b57eb5..264d771f44491 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ b/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 @@ class CppCoreGuidelinesModule : public ClangTidyModule {
"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");
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
new file mode 100644
index 0000000000000..0b85ea19735ee
--- /dev/null
+++ b/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
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h
new file mode 100644
index 0000000000000..5995ad588855e
--- /dev/null
+++ b/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
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 98f18cc539faf..6b2c36975b80c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -138,6 +138,12 @@ New checks
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.
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
new file mode 100644
index 0000000000000..dbc25db2f3a50
--- /dev/null
+++ b/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>`_.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 6a58bb932e9ec..055e6ae661f09 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -191,6 +191,7 @@ Clang-Tidy Checks
`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>`_,
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
new file mode 100644
index 0000000000000..b9720db272e40
--- /dev/null
+++ b/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
More information about the cfe-commits
mailing list