PiotrZSL updated this revision to Diff 503357.
PiotrZSL added a comment.
Rebase due to broken baseline
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144347/new/
https://reviews.llvm.org/D144347
Files:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp
clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.h
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst
clang-tools-extra/test/clang-tidy/checkers/readability/forward-usage.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability/forward-usage.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/forward-usage.cpp
@@ -0,0 +1,257 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s readability-forward-usage %t -- -- -fno-delayed-template-parsing
+
+namespace std {
+
+struct false_type {
+ static constexpr bool value = false;
+};
+
+struct true_type {
+ static constexpr bool value = true;
+};
+
+template <class T>
+struct is_lvalue_reference : false_type {};
+
+template <class T>
+struct is_lvalue_reference<T&> : true_type {};
+
+template <class T>
+struct remove_reference {
+ using type = T;
+};
+
+template <class T>
+struct remove_reference<T&> {
+ using type = T;
+};
+
+template <class T>
+struct remove_reference<T&&> {
+ using type = T;
+};
+
+template <class T>
+using remove_reference_t = typename remove_reference<T>::type;
+
+template <class T>
+constexpr T&& forward(typename std::remove_reference<T>::type& t) noexcept {
+ return static_cast<T&&>(t);
+}
+
+template <class T>
+constexpr T&& forward(typename std::remove_reference<T>::type&& t) noexcept {
+ static_assert(!std::is_lvalue_reference<T>::value, "Can't forward an rvalue as an lvalue.");
+ return static_cast<T&&>(t);
+}
+
+template <class T>
+constexpr typename std::remove_reference<T>::type&& move(T&& t) noexcept {
+ return static_cast<typename std::remove_reference<T>::type&&>(t);
+}
+
+}
+
+struct TestType {
+ int value = 0U;
+};
+
+struct TestTypeEx : TestType {
+};
+
+template<typename ...Args>
+void test(Args&&...) {}
+
+
+template<typename T>
+void testCorrectForward(T&& value) {
+ test(std::forward<T>(value));
+}
+
+template<typename ...Args>
+void testForwardVariadicTemplate(Args&& ...args) {
+ test(std::forward<Args>(args)...);
+}
+
+void callAll() {
+ TestType type1;
+ const TestType type2;
+ testCorrectForward(type1);
+ testCorrectForward(type2);
+ testCorrectForward(std::move(type1));
+ testCorrectForward(std::move(type2));
+ testForwardVariadicTemplate(type1, TestType(), std::move(type2));
+}
+
+void testExplicit(TestType& value) {
+ test(std::forward<TestType>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(value));{{$}}
+}
+
+void testExplicit2(const TestType& value) {
+ test(std::forward<const TestType>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(value));{{$}}
+}
+
+void testExplicit3(TestType value) {
+ test(std::forward<TestType>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(value));{{$}}
+}
+
+void testExplicit4(TestType&& value) {
+ test(std::forward<TestType>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(value));{{$}}
+}
+
+void testExplicit5(const TestType&& value) {
+ test(std::forward<const TestType>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(value));{{$}}
+}
+
+void testExplicit6(TestType&& value) {
+ test(std::forward<TestType&>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: remove redundant use of 'std::forward' as it is unnecessary in this context [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(value);{{$}}
+}
+
+void testExplicit7(TestType value) {
+ test(std::forward<TestType&>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: remove redundant use of 'std::forward' as it is unnecessary in this context [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(value);{{$}}
+}
+
+void testExplicit8(TestType value) {
+ test(std::forward<TestType&&>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(value));{{$}}
+}
+
+void testExplicit9(TestTypeEx value) {
+ test(std::forward<TestType>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using 'std::forward' for type conversions from 'TestTypeEx' to 'TestType' is not recommended here, use 'static_cast' instead [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(static_cast<TestType &&>(value));{{$}}
+}
+
+void testExplicit10(TestTypeEx value) {
+ test(std::forward<TestType&>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using 'std::forward' for type conversions from 'TestTypeEx' to 'TestType &' is not recommended here, use 'static_cast' instead [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(static_cast<TestType&>(value));{{$}}
+}
+
+void testExplicit11(TestTypeEx value) {
+ test(std::forward<TestType&&>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using 'std::forward' for type conversions from 'TestTypeEx' to 'TestType &&' is not recommended here, use 'static_cast' instead [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(static_cast<TestType&&>(value));{{$}}
+}
+
+void testExplicit12(TestType value) {
+ test(std::forward<TestType>(std::move(value)));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: remove redundant use of 'std::forward' as it is unnecessary in this context [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(value));{{$}}
+}
+
+void testExplicit13() {
+ test(std::forward<TestType>(TestType()));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: remove redundant use of 'std::forward' as it is unnecessary in this context [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(TestType());{{$}}
+}
+
+TestType getTestType();
+
+void testExplicit14() {
+ test(std::forward<TestType>(getTestType()));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: remove redundant use of 'std::forward' as it is unnecessary in this context [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(getTestType());{{$}}
+}
+
+#define FORWARD(x,y) std::forward<x>(y)
+#define FORWARD_FUNC std::forward
+#define TEMPLATE_ARG(x) <x>
+
+void testMacro(TestType value) {
+ test(FORWARD(TestType, value));
+ test(FORWARD_FUNC<TestType>(value));
+ test(std::forward TEMPLATE_ARG(TestType)(value));
+}
+
+template<typename T>
+struct Wrapper {};
+
+template<typename T>
+struct WrapperEx : Wrapper<T> {};
+
+template<typename T>
+struct TemplateClass
+{
+ void testTemplate(Wrapper<T> value) {
+ test(std::forward<Wrapper<T>>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(value));{{$}}
+ }
+};
+
+template<typename T>
+void testPartialTemplate(Wrapper<T> value) {
+ test(std::forward<Wrapper<T>>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(value));{{$}}
+
+ using Type = Wrapper<T>;
+ test(std::forward<Type>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(value));{{$}}
+
+}
+
+template<typename T>
+void testPartialTemplate(WrapperEx<T> value) {
+ test(std::forward<Wrapper<T>>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using 'std::forward' for type conversions from 'WrapperEx<T>' to 'Wrapper<T>' is not recommended here, use 'static_cast' instead [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(static_cast<Wrapper<T> &&>(value));{{$}}
+}
+
+void testWrapper() {
+ TemplateClass<TestType>().testTemplate(Wrapper<TestType>());
+ testPartialTemplate(Wrapper<TestType>());
+}
+
+template<template <typename Type> class TemplateType>
+void testTemplateTemplate(TemplateType<TestType> value) {
+ test(std::forward<TemplateType<TestType>>(value));
+
+ using Type = TemplateType<TestType>;
+ test(std::forward<Type>(value));
+}
+
+template<int V>
+struct WrapperInt {};
+
+template<int V>
+WrapperInt<V>& getValueInt();
+
+template<int V>
+WrapperInt<V>&& getValueInt2();
+
+template<int ...Args>
+void CheckArgs() {
+ test(std::forward<WrapperInt<Args>&>(getValueInt<Args>())...);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: remove redundant use of 'std::forward' as it is unnecessary in this context [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(getValueInt<Args>()...);{{$}}
+
+ test(std::forward<WrapperInt<Args>>(getValueInt<Args>())...);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(getValueInt<Args>())...);{{$}}
+
+ test(std::forward<WrapperInt<Args>>(getValueInt2<Args>())...);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(getValueInt2<Args>())...);{{$}}
+
+ test(std::forward<WrapperInt<Args>>(WrapperInt<Args>())...);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^ }}test(std::move(WrapperInt<Args>())...);{{$}}
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst
@@ -0,0 +1,120 @@
+.. title:: clang-tidy - readability-forward-usage
+
+readability-forward-usage
+=========================
+
+Suggests removing or replacing ``std::forward`` with ``std::move`` or
+``static_cast`` in cases where the template argument type is invariant and
+the call always produces the same result.
+
+Recognized applications
+-----------------------
+
+Check can detect different types of usages, as follows:
+
+Casting between different types
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The purpose of ``std::forward`` is to forward the value category of an argument,
+while preserving its constness and reference qualifiers. It is not intended to
+be used for type conversions, as that is outside the scope of its intended use.
+
+Using ``std::forward`` to convert between two types is a misuse because it can
+result in unexpected behavior, such as object slicing. Both the input and output
+types should be cv-equivalent, meaning that they represent the same fundamental
+type after removing all ``const``, ``volatile``, and reference qualifiers.
+
+Check will suggest usage of ``static_cast`` in this situation.
+
+.. code-block:: c++
+
+ struct Base {};
+ struct Derived : Base {};
+
+ void func(Base& base) {
+ doSomething(std::forward<Derived&>(base)); // Incorrect usage
+ doSomething(static_cast<Derived&>(base)); // Suggested usage
+ }
+
+
+Misusing ``std::forward`` as ``std::move``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+While it is technically possible to use ``std::forward`` to convert an `lvalue`
+to an `rvalue` when the resulting type is fixed, this is a misapplication
+because it can result in code that is difficult to read and understand.
+
+The C++ standard provides a better alternative in ``std::move``, which is
+intended specifically for converting an `lvalue` to an `rvalue`, and clearly
+communicates the intention of the developer.
+
+Check will suggest usage of ``std::move`` in this situation.
+
+.. code-block:: c++
+
+ struct Object {};
+
+ void func(Object& obj) {
+ doSomething(std::forward<Object&&>(obj)); // Incorrect usage
+ doSomething(std::move(obj)); // Suggested usage
+ }
+
+
+Redundant use of ``std::forward``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+When using ``std::forward`` to convert from an `lvalue` to another `lvalue`,
+or from an `rvalue` to another `rvalue`, the usage is redundant and should be
+avoided. In such cases, the ``std::forward`` call is unnecessary since the
+types are already equivalent, and it does not provide any additional value in
+preserving the value category of the argument.
+
+It is generally recommended to simply remove the ``std::forward`` call in
+these cases, as it does not have any effect on the result of the expression.
+
+Check will suggest removal of ``std::forward`` in this situation.
+
+.. code-block:: c++
+
+ struct Object {};
+
+ Object createObject();
+
+ void func(Object&& obj) {
+ doSomething(std::forward<Object&&>(createObject())); // Incorrect usage
+ doSomething(std::forward<Object>(createObject())); // Incorrect usage
+ doSomething(createObject()); // Suggested usage
+ }
+
+ void func(Object& obj) {
+ doSomething(std::forward<Object&>(obj)); // Incorrect usage
+ doSomething(obj); // Suggested usage
+ }
+
+
+Options
+-------
+
+.. option:: DisableTypeMismatchSuggestion
+
+ Disables suggestions for type mismatch situations. When this option is
+ enabled, the check treats situations with different types as if they were
+ cv-equal and won't suggest using ``static_cast``, but will suggest using
+ ``std::move`` or removing the call.
+ This option has a default value of `false`.
+
+.. option:: DisableRemoveSuggestion
+
+ Disables suggestions to remove redundant ``std::forward`` calls.
+ This option has a default value of `false`.
+
+.. option:: DisableMoveSuggestion
+
+ Disables suggestions to use ``std::move`` instead of ``std::forward`` for
+ `lvalue` to `rvalue` conversions.
+ This option has a default value of `false`.
+
+.. option:: IgnoreDependentExpresions
+
+ Ignore dependent expressions during analysis (like any templates dependent
+ code). This option has a default value of `false`.
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
@@ -338,6 +338,7 @@
`readability-delete-null-pointer <readability/delete-null-pointer.html>`_, "Yes"
`readability-duplicate-include <readability/duplicate-include.html>`_, "Yes"
`readability-else-after-return <readability/else-after-return.html>`_, "Yes"
+ `readability-forward-usage <readability/forward-usage.html>`_, "Yes"
`readability-function-cognitive-complexity <readability/function-cognitive-complexity.html>`_,
`readability-function-size <readability/function-size.html>`_,
`readability-identifier-length <readability/identifier-length.html>`_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,13 @@
Checks that all implicit and explicit inline functions in header files are
tagged with the ``LIBC_INLINE`` macro.
+- New :doc:`readability-forward-usage
+ <clang-tidy/checks/readability/forward-usage>` check.
+
+ Suggests removing or replacing ``std::forward`` with ``std::move`` or
+ ``static_cast`` in cases where the template argument type is invariant and
+ the call always produces the same result.
+
New check aliases
^^^^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -19,6 +19,7 @@
#include "DeleteNullPointerCheck.h"
#include "DuplicateIncludeCheck.h"
#include "ElseAfterReturnCheck.h"
+#include "ForwardUsageCheck.h"
#include "FunctionCognitiveComplexityCheck.h"
#include "FunctionSizeCheck.h"
#include "IdentifierLengthCheck.h"
@@ -78,6 +79,8 @@
"readability-duplicate-include");
CheckFactories.registerCheck<ElseAfterReturnCheck>(
"readability-else-after-return");
+ CheckFactories.registerCheck<ForwardUsageCheck>(
+ "readability-forward-usage");
CheckFactories.registerCheck<FunctionCognitiveComplexityCheck>(
"readability-function-cognitive-complexity");
CheckFactories.registerCheck<FunctionSizeCheck>(
Index: clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.h
@@ -0,0 +1,39 @@
+//===--- ForwardUsageCheck.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_READABILITY_FORWARDUSAGECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FORWARDUSAGECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::readability {
+
+/// Suggests removing or replacing std::forward with std::move or static_cast
+/// in cases where the template argument type is invariant and the call always
+/// produces the same result.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/forward-usage.html
+class ForwardUsageCheck : public ClangTidyCheck {
+public:
+ ForwardUsageCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+
+private:
+ const bool DisableTypeMismatchSuggestion;
+ const bool DisableRemoveSuggestion;
+ const bool DisableMoveSuggestion;
+ const bool IgnoreDependentExpresions;
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FORWARDUSAGECHECK_H
Index: clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp
@@ -0,0 +1,192 @@
+//===--- ForwardUsageCheck.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 "ForwardUsageCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+
+AST_MATCHER_P(UnresolvedLookupExpr, hasOnyTemplateArgumentLoc,
+ ast_matchers::internal::Matcher<TemplateArgumentLoc>,
+ InnerMatcher) {
+ return Node.getNumTemplateArgs() == 1U &&
+ InnerMatcher.matches(Node.getTemplateArgs()[0], Finder, Builder);
+}
+
+AST_MATCHER(QualType, isFixedType) {
+ QualType CleanType = Node.getCanonicalType().getNonReferenceType();
+
+ if (CleanType->containsErrors())
+ return false;
+
+ if (CleanType->getDependence() == TypeDependenceScope::None)
+ return true;
+
+ const auto *TemplateStruct = CleanType->getAs<TemplateSpecializationType>();
+ if (TemplateStruct) {
+ return TemplateStruct->getTemplateName().getDependence() ==
+ TemplateNameDependence::None;
+ }
+
+ return false;
+}
+
+inline SourceRange getAnglesLoc(const Expr *MatchedCallee) {
+ if (const auto *DeclRefExprCallee =
+ llvm::dyn_cast_or_null<DeclRefExpr>(MatchedCallee))
+ return SourceRange(DeclRefExprCallee->getLAngleLoc(),
+ DeclRefExprCallee->getRAngleLoc());
+
+ if (const auto *UnresolvedLookupExprCallee =
+ llvm::dyn_cast_or_null<UnresolvedLookupExpr>(MatchedCallee))
+ return SourceRange(UnresolvedLookupExprCallee->getLAngleLoc(),
+ UnresolvedLookupExprCallee->getRAngleLoc());
+ return {};
+}
+
+} // namespace
+
+inline bool isDeducedType(const QualType &Type) {
+ return !Type->isSpecificBuiltinType(BuiltinType::Dependent);
+}
+
+inline bool hasXValueArgument(const CallExpr &Call) {
+ return Call.getArg(0U)->isXValue();
+}
+
+ForwardUsageCheck::ForwardUsageCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ DisableTypeMismatchSuggestion(
+ Options.get("DisableTypeMismatchSuggestion", false)),
+ DisableRemoveSuggestion(Options.get("DisableRemoveSuggestion", false)),
+ DisableMoveSuggestion(Options.get("DisableMoveSuggestion", false)),
+ IgnoreDependentExpresions(
+ Options.get("IgnoreDependentExpresions", false)) {}
+
+void ForwardUsageCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "DisableTypeMismatchSuggestion",
+ DisableTypeMismatchSuggestion);
+ Options.store(Opts, "DisableRemoveSuggestion", DisableRemoveSuggestion);
+ Options.store(Opts, "DisableMoveSuggestion", DisableMoveSuggestion);
+ Options.store(Opts, "IgnoreDependentExpresions", IgnoreDependentExpresions);
+}
+
+bool ForwardUsageCheck::isLanguageVersionSupported(
+ const LangOptions &LangOpts) const {
+ return LangOpts.CPlusPlus11;
+}
+
+void ForwardUsageCheck::registerMatchers(MatchFinder *Finder) {
+ const auto templateArgumentType =
+ templateArgumentLoc(hasTypeLoc(loc(qualType(isFixedType()).bind("T"))));
+
+ Finder->addMatcher(
+ traverse(
+ TK_IgnoreUnlessSpelledInSource,
+ callExpr(
+ unless(isExpansionInSystemHeader()), argumentCountIs(1U),
+ IgnoreDependentExpresions
+ ? expr(unless(isInstantiationDependent()))
+ : expr(),
+
+ unless(hasAncestor(functionDecl(isImplicit()))),
+
+ callee(
+ expr(anyOf(declRefExpr(hasDeclaration(functionDecl(
+ hasName("::std::forward"))),
+ hasTemplateArgumentLoc(
+ 0U, templateArgumentLoc(
+ templateArgumentType))),
+ unresolvedLookupExpr(
+ hasAnyDeclaration(
+ namedDecl(hasName("::std::forward"))),
+ hasOnyTemplateArgumentLoc(templateArgumentLoc(
+ templateArgumentType)))))
+ .bind("callee")),
+ hasArgument(0U, expr(hasType(qualType().bind("arg")))))
+ .bind("call")),
+ this);
+}
+
+void ForwardUsageCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedCallee = Result.Nodes.getNodeAs<Expr>("callee");
+ if (MatchedCallee->getBeginLoc().isMacroID() ||
+ MatchedCallee->getEndLoc().isMacroID() ||
+ !utils::rangeCanBeFixed(MatchedCallee->getSourceRange(),
+ Result.SourceManager))
+ return;
+
+ const auto *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("call");
+
+ const auto *MatchedTemplateType = Result.Nodes.getNodeAs<QualType>("T");
+ const auto *MatchedArgumentType = Result.Nodes.getNodeAs<QualType>("arg");
+
+ if (!DisableTypeMismatchSuggestion &&
+ MatchedTemplateType->getCanonicalType().getNonReferenceType() !=
+ MatchedArgumentType->getCanonicalType().getNonReferenceType() &&
+ isDeducedType(*MatchedArgumentType)) {
+
+ auto Diagnostic =
+ diag(MatchedExpr->getBeginLoc(),
+ "using 'std::forward' for type conversions from %0 to %1 is not "
+ "recommended here, use 'static_cast' instead");
+ Diagnostic << *MatchedArgumentType << *MatchedTemplateType;
+
+ const SourceRange AngleSourceRange = getAnglesLoc(MatchedCallee);
+ if (AngleSourceRange.isInvalid())
+ return;
+
+ if (!MatchedTemplateType->getCanonicalType()->isReferenceType()) {
+ Diagnostic << FixItHint::CreateReplacement(
+ SourceRange(AngleSourceRange.getEnd(), AngleSourceRange.getEnd()),
+ " &&>");
+ }
+
+ Diagnostic << FixItHint::CreateReplacement(
+ SourceRange(MatchedCallee->getBeginLoc(), AngleSourceRange.getBegin()),
+ "static_cast<");
+ return;
+ }
+
+ const bool LValueReferenceTemplateType =
+ (*MatchedTemplateType)->isLValueReferenceType();
+ const bool XValueArgument =
+ hasXValueArgument(*MatchedExpr) && isDeducedType(*MatchedArgumentType);
+
+ if (!LValueReferenceTemplateType && !XValueArgument) {
+ if (DisableMoveSuggestion)
+ return;
+
+ diag(MatchedExpr->getBeginLoc(),
+ "use 'std::move' instead of 'std::forward' here, as it more "
+ "accurately reflects the intended purpose")
+ << FixItHint::CreateReplacement(MatchedCallee->getSourceRange(),
+ "std::move");
+ return;
+ }
+
+ if (DisableRemoveSuggestion)
+ return;
+
+ const SourceLocation BeforeArgBegin =
+ MatchedExpr->getArg(0U)->getBeginLoc().getLocWithOffset(-1);
+ diag(MatchedExpr->getBeginLoc(), "remove redundant use of 'std::forward' as "
+ "it is unnecessary in this context")
+ << FixItHint::CreateRemoval(SourceRange(MatchedExpr->getRParenLoc(),
+ MatchedExpr->getRParenLoc()))
+ << FixItHint::CreateRemoval(
+ SourceRange(MatchedExpr->getBeginLoc(), BeforeArgBegin));
+}
+
+} // namespace clang::tidy::readability
Index: clang-tools-extra/clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -14,6 +14,7 @@
DeleteNullPointerCheck.cpp
DuplicateIncludeCheck.cpp
ElseAfterReturnCheck.cpp
+ ForwardUsageCheck.cpp
FunctionCognitiveComplexityCheck.cpp
FunctionSizeCheck.cpp
IdentifierLengthCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
