courbet created this revision.
courbet added a reviewer: aaron.ballman.
Herald added subscribers: xazax.hun, mgorny.
Herald added a project: clang.
The check flags constructs that prevent automatic move of local variables.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D70390
Files:
clang-tools-extra/clang-tidy/performance/CMakeLists.txt
clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.h
clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-no-automatic-move.cpp
@@ -0,0 +1,165 @@
+// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s performance-no-automatic-move %t
+
+namespace std {
+template <typename>
+struct remove_reference;
+
+template <typename _Tp>
+struct remove_reference {
+ typedef _Tp type;
+};
+
+template <typename _Tp>
+struct remove_reference<_Tp &> {
+ typedef _Tp type;
+};
+
+template <typename _Tp>
+struct remove_reference<_Tp &&> {
+ typedef _Tp type;
+};
+
+template <typename _Tp>
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) {
+ return static_cast<typename std::remove_reference<_Tp>::type &&>(__t);
+}
+
+template <typename _Tp>
+constexpr _Tp &&
+forward(typename remove_reference<_Tp>::type &__t) noexcept {
+ return static_cast<_Tp &&>(__t);
+}
+
+} // namespace std
+
+struct Obj {
+ Obj();
+ Obj(const Obj &);
+ Obj(Obj &&);
+ virtual ~Obj();
+};
+
+template <typename T>
+struct StatusOr {
+ StatusOr(const T &);
+ StatusOr(T &&);
+};
+
+struct NonTemplate {
+ NonTemplate(const Obj &);
+ NonTemplate(Obj &&);
+};
+
+template <typename T>
+T Make();
+
+StatusOr<Obj> PositiveStatusOrConstValue() {
+ const Obj obj = Make<Obj>();
+ return obj;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveNonTemplateConstValue() {
+ const Obj obj = Make<Obj>();
+ return obj;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+Obj PositiveSelfConstValue() {
+ const Obj obj = Make<Obj>();
+ return obj;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr<Obj> PositiveStatusOrRefRef() {
+ Obj &&obj = Make<Obj>();
+ return obj;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr<Obj> PositiveNonTemplateRefRef() {
+ Obj &&obj = Make<Obj>();
+ return obj;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr<Obj> PositiveSelfRefRef() {
+ Obj &&obj = Make<Obj>();
+ return obj;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+StatusOr<Obj> PositiveStatusOrConstRefRef() {
+ const Obj &&obj = Make<Obj>();
+ return obj;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveNonTemplateConstRefRef() {
+ const Obj &&obj = Make<Obj>();
+ return obj;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+NonTemplate PositiveSelfConstRefRef() {
+ const Obj &&obj = Make<Obj>();
+ return obj;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: rvalue-ness of 'obj' prevents automatic move [performance-no-automatic-move]
+}
+
+// FIXME: Ideally we would warn here too.
+NonTemplate PositiveNonTemplateLifetimeExtension() {
+ const Obj &obj = Make<Obj>();
+ return obj;
+}
+
+// FIXME: Ideally we would warn here too.
+StatusOr<Obj> PositiveStatusOrLifetimeExtension() {
+ const Obj &obj = Make<Obj>();
+ return obj;
+}
+
+// Negatives.
+
+StatusOr<Obj> Temporary() {
+ return Make<const Obj>();
+}
+
+StatusOr<Obj> ConstTemporary() {
+ return Make<const Obj>();
+}
+
+StatusOr<Obj> Nrvo() {
+ Obj obj = Make<Obj>();
+ return obj;
+}
+
+StatusOr<Obj> Ref() {
+ Obj &obj = Make<Obj &>();
+ return obj;
+}
+
+StatusOr<Obj> ConstRef() {
+ const Obj &obj = Make<Obj &>();
+ return obj;
+}
+
+NonTemplate RefRefMove() {
+ Obj &&obj = Make<Obj>();
+ return std::move(obj);
+}
+
+const Obj global;
+
+StatusOr<Obj> Global() {
+ return global;
+}
+
+struct FromConstRefOnly {
+ FromConstRefOnly(const Obj &);
+};
+
+FromConstRefOnly FromConstRefOnly() {
+ const Obj obj = Make<Obj>();
+ return obj;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst
@@ -0,0 +1,61 @@
+.. title:: clang-tidy - performance-no-automatic-move
+
+performance-no-automatic-move
+=============================
+
+Under
+`certain conditions <https://en.cppreference.com/w/cpp/language/return#automatic_move_from_local_variables_and_parameters>`_,
+local values are automatically moved out when returning from a function.
+
+This check detects some usage patterns that prevent this optimization from
+happening. The most common one is local ``lvalue`` variables that are declared
+``const`` and can therefore not be automatically converted to ``xvalues`` on return,
+preventing the move.
+
+Example `[1] <https://godbolt.org/z/x7SYYA>`_:
+
+.. code-block:: c++
+
+ StatusOr<std::vector<int>> Cool() {
+ std::vector<int> obj = ...;
+ return obj; // calls StatusOr::StatusOr(std::vector<int>&&)
+ }
+
+ StatusOr<std::vector<int>> NotCool() {
+ const std::vector<int> obj = ...;
+ return obj; // calls `StatusOr::StatusOr(const std::vector<int>&)`
+ }
+
+The former version (``Cool``) should be preferred over the latter (``Uncool``) as it
+will avoid allocations and potentially large memory copies.
+
+A variant of this is the following:
+
+.. code-block:: c++
+
+ StatusOr<std::vector<int>> Uncool() {
+ std::vector<int>&& obj = ...;
+ return obj; // calls `StatusOr::StatusOr(const std::vector<int>&)`
+ }
+
+The right way to achieve the same thing is to move obj:
+
+.. code-block:: c++
+
+ StatusOr<std::vector<int>> UncoolFix1() {
+ std::vector<int>&& obj = ...;
+ return std::move(obj); // calls StatusOr::StatusOr(std::vector<int>&&)
+ }
+
+ StatusOr<std::vector<int>> UncoolFix2() {
+ std::vector<int> obj = ...;
+ return obj; // calls StatusOr::StatusOr(std::vector<int>&&)
+ }
+
+## Semantics
+
+In the example above, ``StatusOr::StatusOr(T&&)`` have the same semantics as long
+as the copy and move constructors for ``T`` have the same semantics. Note that
+there is no guarantee that ``S::S(T&&)`` and ``S::S(const T&)`` have the same
+semantics for any single ``S``, so we're not providing automated fixes for this
+check, and judgement should be exerted when making the suggested changes.
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
@@ -343,6 +343,7 @@
performance-inefficient-vector-operation
performance-move-const-arg
performance-move-constructor-init
+ performance-no-automatic-move
performance-noexcept-move-constructor
performance-trivially-destructible
performance-type-promotion-in-math-fn
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -137,6 +137,11 @@
Finds Objective-C implementations that implement ``-isEqual:`` without also
appropriately implementing ``-hash``.
+- New :doc:`performance-no-automatic-move
+ <clang-tidy/checks/performance-no-automatic-move>` check.
+
+ The check flags constructs that prevent automatic move of local variables.
+
- New :doc:`performance-trivially-destructible
<clang-tidy/checks/performance-trivially-destructible>` check.
Index: clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
+++ clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -17,6 +17,7 @@
#include "InefficientVectorOperationCheck.h"
#include "MoveConstArgCheck.h"
#include "MoveConstructorInitCheck.h"
+#include "NoAutomaticMoveCheck.h"
#include "NoexceptMoveConstructorCheck.h"
#include "TriviallyDestructibleCheck.h"
#include "TypePromotionInMathFnCheck.h"
@@ -46,6 +47,8 @@
"performance-move-const-arg");
CheckFactories.registerCheck<MoveConstructorInitCheck>(
"performance-move-constructor-init");
+ CheckFactories.registerCheck<NoAutomaticMoveCheck>(
+ "performance-no-automatic-move");
CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
"performance-noexcept-move-constructor");
CheckFactories.registerCheck<TriviallyDestructibleCheck>(
Index: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.h
@@ -0,0 +1,36 @@
+//===--- NoAutomaticMoveCheck.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_PERFORMANCE_NOAUTOMATICMOVECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOAUTOMATICMOVECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// The check flags constructs that prevent automatic move of local variables.
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance-no-automatic-move.html
+class NoAutomaticMoveCheck : public ClangTidyCheck {
+public:
+ NoAutomaticMoveCheck(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;
+
+private:
+ const std::vector<std::string> AllowedTypes;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOAUTOMATICMOVECHECK_H
Index: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
@@ -0,0 +1,75 @@
+//===--- NoAutomaticMoveCheck.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 "NoAutomaticMoveCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+NoAutomaticMoveCheck::NoAutomaticMoveCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ AllowedTypes(
+ utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
+
+void NoAutomaticMoveCheck::registerMatchers(MatchFinder *finder) {
+ const auto const_local_variable =
+ varDecl(hasLocalStorage(), unless(hasType(lValueReferenceType())),
+ hasType(qualType(
+ hasCanonicalType(matchers::isExpensiveToCopy()),
+ unless(hasDeclaration(namedDecl(
+ matchers::matchesAnyListedName(AllowedTypes)))))))
+ .bind("vardecl");
+
+ // A matcher for a `DstT::DstT(const Src&)` where DstT also has a
+ // `DstT::DstT(Src&&)`.
+ const auto lvalue_ref_ctor = cxxConstructorDecl(
+ hasParameter(0,
+ hasType(lValueReferenceType(pointee(type().bind("SrcT"))))),
+ ofClass(cxxRecordDecl(hasMethod(cxxConstructorDecl(
+ hasParameter(0, hasType(rValueReferenceType(
+ pointee(type(equalsBoundNode("SrcT")))))))))));
+
+ finder->addMatcher(
+ returnStmt(
+ hasReturnValue(ignoringElidableConstructorCall(ignoringParenImpCasts(
+ cxxConstructExpr(hasDeclaration(lvalue_ref_ctor),
+ hasArgument(0, ignoringParenImpCasts(declRefExpr(
+ to(const_local_variable)))))
+ .bind("ctor_call"))))),
+ this);
+}
+
+void NoAutomaticMoveCheck::check(const MatchFinder::MatchResult &result) {
+ const auto *vardecl = result.Nodes.getNodeAs<VarDecl>("vardecl");
+ const auto *ctor_call = result.Nodes.getNodeAs<Expr>("ctor_call");
+ const QualType var_type = vardecl->getType();
+ if (var_type.isConstQualified()) {
+ diag(ctor_call->getExprLoc(), "constness of '%0' prevents automatic move")
+ << vardecl->getName();
+ } else if (var_type->isRValueReferenceType()) {
+ diag(ctor_call->getExprLoc(), "rvalue-ness of '%0' prevents automatic move")
+ << vardecl->getName();
+ }
+}
+
+void NoAutomaticMoveCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "AllowedTypes",
+ utils::options::serializeStringList(AllowedTypes));
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/performance/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -9,6 +9,7 @@
InefficientVectorOperationCheck.cpp
MoveConstArgCheck.cpp
MoveConstructorInitCheck.cpp
+ NoAutomaticMoveCheck.cpp
NoexceptMoveConstructorCheck.cpp
PerformanceTidyModule.cpp
TriviallyDestructibleCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
