[clang-tidy] Add readability-forward-usage check

[PiotrZSL]

Summary

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.

[PiotrZSL]

Fix list.rst

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp

46

Excessive newline.

55

return {}; is shorter.

123

Usually pointers are used. Same in other similar places..

clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst

101

Excessive indentation. Same for other options below.

[PiotrZSL]

Does anyone know why on WINDOWS generated AST is different:

LINUX (when function is never called):
|-FunctionTemplateDecl 0x5577338b2580 <line:211:1, line:216:1> line:212:6 testPartialTemplate
| |-TemplateTypeParmDecl 0x5577338b2288 <line:211:10, col:19> col:19 referenced typename depth 0 index 0 T
| `-FunctionDecl 0x5577338b24d8 <line:212:1, line:216:1> line:212:6 testPartialTemplate 'void (WrapperEx<T>)'
|   |-ParmVarDecl 0x5577338b23e0 <col:26, col:39> col:39 referenced value 'WrapperEx<T>':'WrapperEx<T>'
|   `-CompoundStmt 0x5577338b28a0 <col:46, line:216:1>
|     `-CallExpr 0x5577338b2878 <line:213:5, col:41> '<dependent type>'
|       |-UnresolvedLookupExpr 0x5577338b2668 <col:5> '<overloaded function type>' lvalue (ADL) = 'test' 0x557733898c10
|       `-CallExpr 0x5577338b2850 <col:10, col:40> '<dependent type>'
|         |-UnresolvedLookupExpr 0x5577338b27b0 <col:10, col:33> '<dependent type>' lvalue (no ADL) = 'forward' 0x5577338961d8 0x557733896818
|         `-DeclRefExpr 0x5577338b2830 <col:35> 'WrapperEx<T>':'WrapperEx<T>' lvalue ParmVar 0x5577338b23e0 'value' 'WrapperEx<T>':'WrapperEx<T>'

LINUX (when function is called)
|-FunctionTemplateDecl 0x556b33e8a5b0 <line:211:1, line:216:1> line:212:6 testPartialTemplate
| |-TemplateTypeParmDecl 0x556b33e8a2b8 <line:211:10, col:19> col:19 referenced typename depth 0 index 0 T
| |-FunctionDecl 0x556b33e8a508 <line:212:1, line:216:1> line:212:6 testPartialTemplate 'void (WrapperEx<T>)'
| | |-ParmVarDecl 0x556b33e8a410 <col:26, col:39> col:39 referenced value 'WrapperEx<T>':'WrapperEx<T>'
| | `-CompoundStmt 0x556b33e8a8d0 <col:46, line:216:1>
| |   `-CallExpr 0x556b33e8a8a8 <line:213:5, col:41> '<dependent type>'
| |     |-UnresolvedLookupExpr 0x556b33e8a698 <col:5> '<overloaded function type>' lvalue (ADL) = 'test' 0x556b33e70c40
| |     `-CallExpr 0x556b33e8a880 <col:10, col:40> '<dependent type>'
| |       |-UnresolvedLookupExpr 0x556b33e8a7e0 <col:10, col:33> '<dependent type>' lvalue (no ADL) = 'forward' 0x556b33e6e208 0x556b33e6e848
| |       `-DeclRefExpr 0x556b33e8a860 <col:35> 'WrapperEx<T>':'WrapperEx<T>' lvalue ParmVar 0x556b33e8a410 'value' 'WrapperEx<T>':'WrapperEx<T>'
| `-FunctionDecl 0x556b33e8e888 <line:212:1, line:216:1> line:212:6 used testPartialTemplate 'void (WrapperEx<TestType>)'

WINDOWS (when function is never called):
|-FunctionTemplateDecl 0x1ac13659de0 <line:211:1, line:212:44> col:6 testPartialTemplate
| |-TemplateTypeParmDecl 0x1ac13652238 <line:211:10, col:19> col:19 referenced typename depth 0 index 0 T
| `-FunctionDecl 0x1ac13659d38 <line:212:1, col:44> col:6 testPartialTemplate 'void (WrapperEx<T>)'
|   |-ParmVarDecl 0x1ac13652390 <col:26, col:39> col:39 value 'WrapperEx<T>':'WrapperEx<T>'
|   `-<<<NULL>>>

WINDOWS (when function is called):
|-FunctionTemplateDecl 0x1fe6f1b99a0 <line:211:1, line:216:1> line:212:6 testPartialTemplate
| |-TemplateTypeParmDecl 0x1fe6f1b9568 <line:211:10, col:19> col:19 referenced typename depth 0 index 0 T
| |-FunctionDecl 0x1fe6f1b98f8 <line:212:1, line:216:1> line:212:6 testPartialTemplate 'void (WrapperEx<T>)'
| | |-ParmVarDecl 0x1fe6f1b96c0 <col:26, col:39> col:39 referenced value 'WrapperEx<T>':'WrapperEx<T>'
| | `-CompoundStmt 0x1fe6f1c5070 <col:46, line:216:1>
| |   `-CallExpr 0x1fe6f1c5048 <line:213:5, col:41> '<dependent type>'
| |     |-UnresolvedLookupExpr 0x1fe6f1c4e40 <col:5> '<overloaded function type>' lvalue (ADL) = 'test' 0x1fe6f18df70
| |     `-CallExpr 0x1fe6f1c5020 <col:10, col:40> '<dependent type>'
| |       |-UnresolvedLookupExpr 0x1fe6f1c4f80 <col:10, col:33> '<dependent type>' lvalue (no ADL) = 'forward' 0x1fe6f18c3e8 0x1fe6f18ca28
| |       `-DeclRefExpr 0x1fe6f1c5000 <col:35> 'WrapperEx<T>':'WrapperEx<T>' lvalue ParmVar 0x1fe6f1b96c0 'value' 'WrapperEx<T>':'WrapperEx<T>'
| `-FunctionDecl 0x1fe6f1bebd8 <line:212:1, line:216:1> line:212:6 used testPartialTemplate 'void (WrapperEx<TestType>)'
|   |-TemplateArgument type 'TestType'

EDIT: Ok, found that -fno-delayed-template-parsing is an workaround for this (but still strange).

[PiotrZSL]

Fixed review comments and added -fno-delayed-template-parsing

[PiotrZSL]

Ready for review

[PiotrZSL]

Ping, Rebase

[PiotrZSL]

Rebase due to broken baseline

[PiotrZSL]

Rebase due to broken baseline

[ccotter]

Is it worth adding a cppcoreguidelines alias (ES.56)?

clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst

99

Curious what others thing but I think the tool should by default not remove or replace forward with static_cast. When an author has written forward, there is a good chance they intended to forward something (e.g., they forgot to use && in the parameter declaration). My hypothesis is that it's more likely the author intended to forward something, rather than than they were trying to use it as a cast (but had not intention of forward - why else would they have typed it out?). In this case, I think it merits manual review by default (so the tool should always warn, but without the fixits by default).

[PiotrZSL]

In D144347#4222534, @ccotter wrote:

Is it worth adding a cppcoreguidelines alias (ES.56)?

Yes, you can pull this change (arc patch D144347 --nobranch) and create change that would depend on this one, and would register alias, also there you could override DisableRemoveSuggestion.
You can also help reviewing this check, to push it forward, point missing tests, scenarios. We could merge tests...

clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst

99

No, in this case if they used forward, they used this with different type, not adding & is not sufficient to trigger this case.

In theory we could, create alias bugprone-std-forward that would redirect to this check, but would set DisableTypeMismatchSuggestion to false, and would set DisableRemoveSuggestion & DisableMoveSuggestion to true.

In such case we could have readability check that would just suggest remove of std::forward or usage of std::move, and bugprone check that would only warn of casts between different types.

[ccotter]

clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp

99

Curious, why have isExpansionInSystemHeader here, rather than rely on the system-headers command line option. Is that a matter of performance (this tool is not likely to apply for system headers)?

clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst

99

Makes sense to me. Happy to work on that once this is merged.

[PiotrZSL]

I will leave this review open for 1 more week, in case someone have some comments, and by someone I mean you @carlosgalvezp.

clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp

99

isExpansionInSystemHeader works on check level, when --system-headers on diagnostic level. This mean that check will be executed, and warning will be excluded during call to diag.
For some checks this can be expensive.
Read more here: https://discourse.llvm.org/t/rfc-exclude-issues-from-system-headers-as-early-as-posible/68483

[ccotter]

clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp

100

I think we might need more tests when IgnoreDependentExpresions is true. When I was playing around and hardcoded IgnoreDependentExpresions to false on line 99, the tests still pass.

clang-tools-extra/docs/ReleaseNotes.rst

122

Would you see use in expanding use of this check to cases that (broadly, cppcoreguiideline ES.56) apply forward when the argument is not a forwarding reference? Or separate check (as the one I proposed in https://reviews.llvm.org/D146888)? Specifically, when the template parameter is deduced:

template <class T> void forwards_incorrectly(T& t) { T other = std::forward<T>(t); } // ignored by readability-forward-usage

[PiotrZSL]

TODO: Extract DisableTypeMismatchSuggestion & that logic into separate check in bugprone category.
TODO: Validate more situations with templates.

[njames93]

clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp

46

Use static instead of inline. Inline should only be used for functions(or variables) defined in header files.
Same goes for the other inline functions.
Also this function should be moved outside the anonymous namespace.

48

Remove the llvm:: qualifier and the null check is redundant here as its guaranteed to never be called with a non-null argument.

53

Ditton

95–97

Rather than calling traverse, the canoncial way to handle this is by overriding the getCheckTraversalKind virtual function to return TK_IgnoreUnlessSpelledInSource

100

This needs addressing before this can be landed

108

It's a good idea to follow to DRY principle:

auto IsNamedStdForward = hasName("::std::forward");

Then you can use that here and below.

A similar point can be made with the bind IDs, if you define the IDs as a static constexpr char [] you can reuse them during`bind` and getNodeAs calls, helps avoid bugs if the check is ever upgraded.

[ccotter]

@PiotrZSL - checking back, do you plan to revisit this change soon (I think there are some pending feedback and you planned some changes)? I'd like to see this in change merged in. Let me know if I can help.

[PiotrZSL]

In D144347#4469770, @ccotter wrote:

@PiotrZSL - checking back, do you plan to revisit this change soon (I think there are some pending feedback and you planned some changes)? I'd like to see this in change merged in. Let me know if I can help.

I will try to re-visit this change this week. I need to split this check somehow. Probably extract casting between types into separate bugprone check and add some better support for template types.
Give me 2-3 days.

[ccotter]

Great, thanks!

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyReadabilityModule
   AvoidConstParamsInDecls.cpp
   BracesAroundStatementsCheck.cpp
   ConstReturnTypeCheck.cpp
   ContainerContainsCheck.cpp
   ContainerDataPointerCheck.cpp
   ContainerSizeEmptyCheck.cpp
   ConvertMemberFunctionsToStatic.cpp
   DeleteNullPointerCheck.cpp
   DuplicateIncludeCheck.cpp
   ElseAfterReturnCheck.cpp
+  ForwardUsageCheck.cpp
   FunctionCognitiveComplexityCheck.cpp
   FunctionSizeCheck.cpp
   IdentifierLengthCheck.cpp
   IdentifierNamingCheck.cpp
   ImplicitBoolConversionCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
   IsolateDeclarationCheck.cpp
   MagicNumbersCheck.cpp

clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.h

+//===--- 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

clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp

+//===--- 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 SourceRange();
+}
+
+} // 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

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "ConstReturnTypeCheck.h"
 #include "ContainerContainsCheck.h"
 #include "ContainerDataPointerCheck.h"
 #include "ContainerSizeEmptyCheck.h"
 #include "ConvertMemberFunctionsToStatic.h"
 #include "DeleteNullPointerCheck.h"
 #include "DuplicateIncludeCheck.h"
 #include "ElseAfterReturnCheck.h"
+#include "ForwardUsageCheck.h"
 #include "FunctionCognitiveComplexityCheck.h"
 #include "FunctionSizeCheck.h"
 #include "IdentifierLengthCheck.h"
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolConversionCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
 #include "IsolateDeclarationCheck.h"
 #include "MagicNumbersCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<ConvertMemberFunctionsToStatic>(
         "readability-convert-member-functions-to-static");
     CheckFactories.registerCheck<DeleteNullPointerCheck>(
         "readability-delete-null-pointer");
     CheckFactories.registerCheck<DuplicateIncludeCheck>(
         "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>(
         "readability-function-size");
     CheckFactories.registerCheck<IdentifierLengthCheck>(
         "readability-identifier-length");
     CheckFactories.registerCheck<IdentifierNamingCheck>(
         "readability-identifier-naming");

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`cppcoreguidelines-avoid-capture-default-when-capturing-this
   Warns when lambda specify a capture default and capture ``this``.
 
 - New :doc: `llvmlibc-inline-function-decl
   <clang-tidy/checks/llvmlibc/inline-function-decl>` check.
 
   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
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-msc24-c
   <clang-tidy/checks/cert/msc24-c>` to :doc:`bugprone-unsafe-functions
   <clang-tidy/checks/bugprone/unsafe-functions>` was added.
 
 - New alias :doc:`cert-msc33-c

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `readability-const-return-type <readability/const-return-type.html>`_, "Yes"
    `readability-container-contains <readability/container-contains.html>`_, "Yes"
    `readability-container-data-pointer <readability/container-data-pointer.html>`_, "Yes"
    `readability-container-size-empty <readability/container-size-empty.html>`_, "Yes"
    `readability-convert-member-functions-to-static <readability/convert-member-functions-to-static.html>`_, "Yes"
    `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>`_,
    `readability-identifier-naming <readability/identifier-naming.html>`_, "Yes"
    `readability-implicit-bool-conversion <readability/implicit-bool-conversion.html>`_, "Yes"
    `readability-inconsistent-declaration-parameter-name <readability/inconsistent-declaration-parameter-name.html>`_, "Yes"
    `readability-isolate-declaration <readability/isolate-declaration.html>`_, "Yes"
    `readability-magic-numbers <readability/magic-numbers.html>`_,

clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst

+.. 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`.

clang-tools-extra/test/clang-tidy/checkers/readability/forward-usage.cpp

+// RUN: %check_clang_tidy -std=c++11-or-later %s readability-forward-usage %t
+
+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>())...);{{$}}
+}