ccotter updated this revision to Diff 508450.
ccotter added a comment.
- add tests, simplify expr, handle unevaluated exprs
- formatting
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/ForwardingReferenceParamNotForwardedCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-forwarding-reference-param-not-forwarded %t
+
+// 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-forwarding-reference-param-not-forwarded]
+ 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-forwarding-reference-param-not-forwarded]
+ 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-forwarding-reference-param-not-forwarded]
+ 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-forwarding-reference-param-not-forwarded]
+ 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-forwarding-reference-param-not-forwarded]
+
+ 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-forwarding-reference-param-not-forwarded]
+
+ 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-forwarding-reference-param-not-forwarded]
+ 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-forwarding-reference-param-not-forwarded]
+ using result_t = decltype(std::forward<T>(t));
+ unsigned len = sizeof(std::forward<T>(t));
+ T other = 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);
+}
+
+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
@@ -186,6 +186,7 @@
`cppcoreguidelines-avoid-goto <cppcoreguidelines/avoid-goto.html>`_,
`cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines/avoid-non-const-global-variables.html>`_,
`cppcoreguidelines-avoid-reference-coroutine-parameters <cppcoreguidelines/avoid-reference-coroutine-parameters.html>`_,
+ `cppcoreguidelines-forwarding-reference-param-not-forwarded <cppcoreguidelines/forwarding-reference-param-not-forwarded.html>`_, "Yes"
`cppcoreguidelines-init-variables <cppcoreguidelines/init-variables.html>`_, "Yes"
`cppcoreguidelines-interfaces-global-init <cppcoreguidelines/interfaces-global-init.html>`_,
`cppcoreguidelines-macro-usage <cppcoreguidelines/macro-usage.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - cppcoreguidelines-forwarding-reference-param-not-forwarded
+
+cppcoreguidelines-forwarding-reference-param-not-forwarded
+==========================================================
+
+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
+ }
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -121,6 +121,13 @@
use-after-free errors and suggests avoiding captures or ensuring the lambda
closure object has a guaranteed lifetime.
+- New :doc:`cppcoreguidelines-forwarding-reference-param-not-forwarded
+ <clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded>`
+ 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/ForwardingReferenceParamNotForwardedCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h
@@ -0,0 +1,36 @@
+//===--- ForwardingReferenceParamNotForwardedCheck.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_FORWARDINGREFERENCEPARAMNOTFORWARDEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_FORWARDINGREFERENCEPARAMNOTFORWARDEDCHECK_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/forwarding-reference-param-not-forwarded.html
+class ForwardingReferenceParamNotForwardedCheck : public ClangTidyCheck {
+public:
+ ForwardingReferenceParamNotForwardedCheck(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;
+ }
+};
+
+} // namespace clang::tidy::cppcoreguidelines
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_FORWARDINGREFERENCEPARAMNOTFORWARDEDCHECK_H
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp
@@ -0,0 +1,105 @@
+//===--- ForwardingReferenceParamNotForwardedCheck.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 "ForwardingReferenceParamNotForwardedCheck.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 {
+
+AST_MATCHER(Expr, hasUnevaluatedContext) {
+ if (isa<CXXNoexceptExpr>(Node) || isa<RequiresExpr>(Node))
+ return true;
+ if (const auto *UnaryExpr = dyn_cast<UnaryExprOrTypeTraitExpr>(&Node)) {
+ switch (UnaryExpr->getKind()) {
+ case UETT_SizeOf:
+ case UETT_AlignOf:
+ return true;
+ default:
+ return false;
+ }
+ }
+ if (const auto *TypeIDExpr = dyn_cast<CXXTypeidExpr>(&Node))
+ return !TypeIDExpr->isPotentiallyEvaluated();
+ return false;
+}
+
+AST_MATCHER_P(QualType, possiblyPackExpansionOf,
+ ast_matchers::internal::Matcher<QualType>, InnerMatcher) {
+ return InnerMatcher.matches(Node.getNonPackExpansionType(), Finder, Builder);
+}
+
+AST_MATCHER(ParmVarDecl, isTemplateTypeOfFunction) {
+ 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 ForwardingReferenceParamNotForwardedCheck::registerMatchers(
+ MatchFinder *Finder) {
+ auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param")));
+
+ StatementMatcher ForwardCallMatcher = callExpr(
+ 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"), isTemplateTypeOfFunction(),
+ hasAncestor(functionDecl(isDefinition(), ToParam,
+ unless(hasDescendant(ForwardCallMatcher))))),
+ this);
+}
+
+void ForwardingReferenceParamNotForwardedCheck::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
@@ -21,6 +21,7 @@
#include "AvoidGotoCheck.h"
#include "AvoidNonConstGlobalVariablesCheck.h"
#include "AvoidReferenceCoroutineParametersCheck.h"
+#include "ForwardingReferenceParamNotForwardedCheck.h"
#include "InitVariablesCheck.h"
#include "InterfacesGlobalInitCheck.h"
#include "MacroUsageCheck.h"
@@ -70,6 +71,8 @@
"cppcoreguidelines-avoid-reference-coroutine-parameters");
CheckFactories.registerCheck<modernize::UseOverrideCheck>(
"cppcoreguidelines-explicit-virtual-functions");
+ CheckFactories.registerCheck<ForwardingReferenceParamNotForwardedCheck>(
+ "cppcoreguidelines-forwarding-reference-param-not-forwarded");
CheckFactories.registerCheck<InitVariablesCheck>(
"cppcoreguidelines-init-variables");
CheckFactories.registerCheck<InterfacesGlobalInitCheck>(
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
@@ -12,6 +12,7 @@
AvoidNonConstGlobalVariablesCheck.cpp
AvoidReferenceCoroutineParametersCheck.cpp
CppCoreGuidelinesTidyModule.cpp
+ ForwardingReferenceParamNotForwardedCheck.cpp
InitVariablesCheck.cpp
InterfacesGlobalInitCheck.cpp
MacroUsageCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
