PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Detects instances where std::forward is called with a different type as an
argument compared to the type specified as the template parameter.
Extracted from D144347 <https://reviews.llvm.org/D144347>.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154999
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/StdForwardTypeMismatchCheck.cpp
clang-tools-extra/clang-tidy/bugprone/StdForwardTypeMismatchCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/std-forward-type-mismatch.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/std-forward-type-mismatch.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/std-forward-type-mismatch.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/std-forward-type-mismatch.cpp
@@ -0,0 +1,144 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-std-forward-type-mismatch %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 testCorrectForwardVariadicTemplate(Args&& ...args) {
+ test(std::forward<Args>(args)...);
+}
+
+void testExplicit1(TestTypeEx value) {
+ test(std::forward<TestType>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: using 'std::forward' for type conversions from 'TestTypeEx' to 'TestType' is not recommended here, use 'static_cast' instead
+// CHECK-FIXES: {{^ }}test(static_cast<TestType &&>(value));{{$}}
+}
+
+void testExplicit2(TestTypeEx value) {
+ test(std::forward<TestType&>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: using 'std::forward' for type conversions from 'TestTypeEx' to 'TestType &' is not recommended here, use 'static_cast' instead
+// CHECK-FIXES: {{^ }}test(static_cast<TestType&>(value));{{$}}
+}
+
+void testExplicit3(TestTypeEx value) {
+ test(std::forward<TestType&&>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: using 'std::forward' for type conversions from 'TestTypeEx' to 'TestType &&' is not recommended here, use 'static_cast' instead
+// CHECK-FIXES: {{^ }}test(static_cast<TestType&&>(value));{{$}}
+}
+
+#define FORWARD(x,y) std::forward<x>(y)
+#define FORWARD_FUNC std::forward
+#define TEMPLATE_ARG(x) <x>
+
+void testMacro(TestTypeEx value) {
+ test(FORWARD(TestType, value));
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: using 'std::forward' for type conversions from 'TestTypeEx' to 'TestType' is not recommended here, use 'static_cast' instead
+ test(FORWARD_FUNC<TestType>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: using 'std::forward' for type conversions from 'TestTypeEx' to 'TestType' is not recommended here, use 'static_cast' instead
+// CHECK-FIXES: {{^ }}test(static_cast<TestType &&>(value));{{$}}
+ test(std::forward TEMPLATE_ARG(TestType)(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: using 'std::forward' for type conversions from 'TestTypeEx' to 'TestType' is not recommended here, use 'static_cast' instead
+}
+
+template<typename T>
+struct Wrapper {};
+
+template<typename T>
+struct WrapperEx : Wrapper<T> {};
+
+template<typename T>
+void testPartialTemplateBad(WrapperEx<T> value) {
+ test(std::forward<Wrapper<T>>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: using 'std::forward' for type conversions from 'WrapperEx<T>' to 'Wrapper<T>' is not recommended here, use 'static_cast' instead
+// CHECK-FIXES: {{^ }}test(static_cast<Wrapper<T> &&>(value));{{$}}
+}
+
+template<typename T>
+void testPartialTemplateCorrect(WrapperEx<T> value) {
+ test(std::forward<WrapperEx<T>>(value));
+}
+
+template<typename T1, typename T2>
+void testTemplateBad(T2&& value) {
+ test(std::forward<T1>(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: using 'std::forward' for type conversions from 'T2' to 'T1' is not recommended here, use 'static_cast' instead
+// CHECK-FIXES: {{^ }}test(static_cast<T1 &&>(value));{{$}}
+}
+
+void callAll() {
+ TestType type1;
+ const TestType type2;
+ testCorrectForward(type1);
+ testCorrectForward(type2);
+ testCorrectForwardVariadicTemplate(type1, type2, TestType());
+ testCorrectForward(TestType());
+ testPartialTemplateBad(WrapperEx<TestType>());
+ testPartialTemplateCorrect(WrapperEx<TestType>());
+ testTemplateBad<TestType>(TestType());
+}
+
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
@@ -118,6 +118,7 @@
`bugprone-sizeof-expression <bugprone/sizeof-expression.html>`_,
`bugprone-spuriously-wake-up-functions <bugprone/spuriously-wake-up-functions.html>`_,
`bugprone-standalone-empty <bugprone/standalone-empty.html>`_, "Yes"
+ `bugprone-std-forward-type-mismatch <bugprone/std-forward-type-mismatch.html>`_,
`bugprone-string-constructor <bugprone/string-constructor.html>`_, "Yes"
`bugprone-string-integer-assignment <bugprone/string-integer-assignment.html>`_, "Yes"
`bugprone-string-literal-with-embedded-nul <bugprone/string-literal-with-embedded-nul.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/std-forward-type-mismatch.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/std-forward-type-mismatch.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - bugprone-std-forward-type-mismatch
+
+bugprone-std-forward-type-mismatch
+==================================
+
+Detects instances where ``std::forward`` is called with a different type as an
+argument compared to the type specified as the template parameter. It takes
+into consideration unwinding type aliases and excludes references or qualifiers
+when comparing the types.
+
+.. 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
+ }
+
+The ``std::forward`` function is primarily intended to forward the value
+category of an argument while preserving its constness and reference qualifiers.
+Using it with different types can lead to issues like object slicing or
+incorrect behavior.
+
+It is recommended to use ``static_cast`` instead of ``std::forward`` when there
+is a mismatch between the type passed as an argument and the template parameter.
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -121,6 +121,12 @@
Detect implicit and explicit casts of ``enum`` type into ``bool`` where ``enum`` type
doesn't have a zero-value enumerator.
+- New :doc:`bugprone-std-forward-type-mismatch
+ <clang-tidy/checks/bugprone/std-forward-type-mismatch>` check.
+
+ Detects instances where ``std::forward`` is called with a different type as an
+ argument compared to the type specified as the template parameter.
+
- New :doc:`bugprone-unique-ptr-array-mismatch
<clang-tidy/checks/bugprone/unique-ptr-array-mismatch>` check.
Index: clang-tools-extra/clang-tidy/bugprone/StdForwardTypeMismatchCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/StdForwardTypeMismatchCheck.h
@@ -0,0 +1,33 @@
+//===--- StdForwardTypeMismatchCheck.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_BUGPRONE_STDFORWARDTYPEMISMATCHCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_STDFORWARDTYPEMISMATCHCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Detects instances where ``std::forward`` is called with a different type as
+/// an argument compared to the type specified as the template parameter.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/std-forward-type-mismatch.html
+class StdForwardTypeMismatchCheck : public ClangTidyCheck {
+public:
+ StdForwardTypeMismatchCheck(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;
+ std::optional<TraversalKind> getCheckTraversalKind() const override;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_STDFORWARDTYPEMISMATCHCHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/StdForwardTypeMismatchCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/StdForwardTypeMismatchCheck.cpp
@@ -0,0 +1,111 @@
+//===--- StdForwardTypeMismatchCheck.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 "StdForwardTypeMismatchCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER_P(UnresolvedLookupExpr, hasOnyTemplateArgumentLoc,
+ ast_matchers::internal::Matcher<TemplateArgumentLoc>,
+ InnerMatcher) {
+ return Node.getNumTemplateArgs() == 1U &&
+ InnerMatcher.matches(Node.getTemplateArgs()[0], Finder, Builder);
+}
+
+} // namespace
+
+static SourceRange getAnglesLoc(const Expr *MatchedCallee) {
+ if (const auto *DeclRefExprCallee = dyn_cast<DeclRefExpr>(MatchedCallee))
+ return SourceRange(DeclRefExprCallee->getLAngleLoc(),
+ DeclRefExprCallee->getRAngleLoc());
+
+ if (const auto *UnresolvedLookupExprCallee =
+ dyn_cast<UnresolvedLookupExpr>(MatchedCallee))
+ return SourceRange(UnresolvedLookupExprCallee->getLAngleLoc(),
+ UnresolvedLookupExprCallee->getRAngleLoc());
+ return {};
+}
+
+void StdForwardTypeMismatchCheck::registerMatchers(MatchFinder *Finder) {
+ auto IsNamedStdForwardMatcher = hasName("::std::forward");
+ auto TemplateArgumentTypeMatcher =
+ templateArgumentLoc(hasTypeLoc(loc(qualType().bind("template"))));
+
+ Finder->addMatcher(
+ callExpr(
+ argumentCountIs(1U), unless(isExpansionInSystemHeader()),
+ callee(
+ expr(anyOf(declRefExpr(hasDeclaration(functionDecl(
+ IsNamedStdForwardMatcher)),
+ hasTemplateArgumentLoc(
+ 0U, templateArgumentLoc(
+ TemplateArgumentTypeMatcher))),
+ unresolvedLookupExpr(
+ hasAnyDeclaration(
+ namedDecl(IsNamedStdForwardMatcher)),
+ hasOnyTemplateArgumentLoc(templateArgumentLoc(
+ TemplateArgumentTypeMatcher)))))
+ .bind("callee")),
+ hasArgument(0U, expr(hasType(qualType().bind("argument")))))
+ .bind("call"),
+ this);
+}
+
+bool StdForwardTypeMismatchCheck::isLanguageVersionSupported(
+ const LangOptions &LangOpts) const {
+ return LangOpts.CPlusPlus11;
+}
+
+std::optional<TraversalKind>
+StdForwardTypeMismatchCheck::getCheckTraversalKind() const {
+ return TK_IgnoreUnlessSpelledInSource;
+}
+
+void StdForwardTypeMismatchCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *MatchedTemplateType =
+ Result.Nodes.getNodeAs<QualType>("template");
+ const auto *MatchedArgumentType =
+ Result.Nodes.getNodeAs<QualType>("argument");
+
+ const QualType CleanMatchedTemplateType =
+ MatchedTemplateType->getCanonicalType().getNonReferenceType();
+ const QualType CleanMatchedArgumentType =
+ MatchedArgumentType->getCanonicalType().getNonReferenceType();
+ if (CleanMatchedTemplateType == CleanMatchedArgumentType)
+ return;
+
+ const auto *MatchedExpr = Result.Nodes.getNodeAs<CallExpr>("call");
+ const auto *MatchedCallee = Result.Nodes.getNodeAs<Expr>("callee");
+
+ 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<");
+}
+
+} // namespace clang::tidy::bugprone
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -48,6 +48,7 @@
SmartPtrArrayMismatchCheck.cpp
SpuriouslyWakeUpFunctionsCheck.cpp
StandaloneEmptyCheck.cpp
+ StdForwardTypeMismatchCheck.cpp
StringConstructorCheck.cpp
StringIntegerAssignmentCheck.cpp
StringLiteralWithEmbeddedNulCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -52,6 +52,7 @@
#include "SizeofExpressionCheck.h"
#include "SpuriouslyWakeUpFunctionsCheck.h"
#include "StandaloneEmptyCheck.h"
+#include "StdForwardTypeMismatchCheck.h"
#include "StringConstructorCheck.h"
#include "StringIntegerAssignmentCheck.h"
#include "StringLiteralWithEmbeddedNulCheck.h"
@@ -166,6 +167,8 @@
"bugprone-spuriously-wake-up-functions");
CheckFactories.registerCheck<StandaloneEmptyCheck>(
"bugprone-standalone-empty");
+ CheckFactories.registerCheck<StdForwardTypeMismatchCheck>(
+ "bugprone-std-forward-type-mismatch");
CheckFactories.registerCheck<StringConstructorCheck>(
"bugprone-string-constructor");
CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
