[clang-tidy] Adds performance-returning-type check.

[staronj]

Summary

Adds performance-returning-type check.

This check is trying to find places, where one is returning different type than function returning type and when returned object is being copied instead of moved. Check wraps it with std::move when it could make it being moved.

I ran this check on LLVM codebase and it produced following changes: http://reviews.llvm.org/D21302

[Prazek]

clang-tidy/performance/ReturningTypeCheck.cpp

43 ↗

(On Diff #60587)

This seems like a usefull matcher. We should probably add it to ASTMatchers, or at least move it to Utils

docs/clang-tidy/checks/performance-returning-type.rst

26 ↗

(On Diff #60587)

It actually becomes boost::optional<std::string>(std::move(s));

35 ↗

(On Diff #60587)

Please add some subsection, or something similiar with note:
"This check requires C++11 or higher to run"

35 ↗

(On Diff #60587)

Also describe a little bit when the checks doesn't triggers, or when it triggers, like
"for function returning A, that is constructible from B
check triggers when:

• B::B(A &&)
• B::B(A) when A is movable and not trivially copyable
• A::operator B() when B is movable

check doesn't trigger when

• B::B(const A&)
• a is const

(and some other rules that I didn't add here)

[Prazek]

clang-tidy/performance/ReturningTypeCheck.cpp

53 ↗

(On Diff #60587)

A little bit too much used word matcher/matches :D

132 ↗

(On Diff #60587)

I am aware of LLVM style guide, but I think there is exception for the matchers - they looks much more like a matchers when they starts with lower case. I am not sure if it is not spoken rule, but I don't remember single binded matcher that would start with capital case.

[Eugene.Zelenko]

docs/ReleaseNotes.rst

238

One sentence should be enough. Please also remove "This check". "We" should be replaced with "function".

docs/clang-tidy/checks/performance-returning-type.rst

7 ↗

(On Diff #60587)

Please highlight std::move with ``. Same below

[mnbvmar]

clang-tidy/performance/ReturningTypeCheck.cpp

69 ↗

(On Diff #60587)

Probably hasOneActiveArgument.

72 ↗

(On Diff #60587)

In hasParameter(N, ...), argument id is 0-based. Did you mean to match the third parameter (1-based)?

[alexfh]

Apart from the other reviewers' comments, I have a few concerns:

1. The "performance-returning-type" name is missing an important part of information. Maybe "performance-return-value-conversion", "performance-return-value-copy", "performance-type-conversion-on-return", "performance-return-different-type" or something like that.
2. The "expression could be wrapped with std::move" warning is close to useless. There are many things that "could be", but the main question is "why"? The first thing the message has to explain, is what's wrong with the current state, and only then suggest a solution. Also think about cases where the suggested solution is not the optimal one (e.g. changing the return type is better).
3. What's so special about return? Looks like a more general problem is: passing an lvalue to a function/constructor/operator by const reference where it could instead be moved (the lvalue isn't used afterwards and the appropriate overload for rvalue reference is available).

[Prazek]

In D21303#460814, @alexfh wrote:

3. What's so special about return? Looks like a more general problem is: passing an lvalue to a function/constructor/operator by const reference where it could instead be moved (the lvalue isn't used afterwards and the appropriate overload for rvalue reference is available).

Yes, you are right. We could generalize it more to suggest std::move for any last usage of variable (if there would be rvalue overload), but it is harde to do than this. The special thing about return, is that in most cases you shouldn't write 'return std::move(...)', so we want to find the cases where you should do it.

[staronj]

1. Name changed to return-value-copy.
2. Changed warning message.
3. Fixed check description.

clang-tidy/performance/ReturningTypeCheck.cpp

132 ↗

(On Diff #60587)

See UnnecessaryValueParamCheck.cpp line 33 or LoopConvertCheck.cpp lines 37-47. They all start with capital letter.

[Prazek]

clang-tidy/performance/ReturnValueCopyCheck.cpp

107–108

You have bug here, but it should not affect the check -
hasDescendant(cxxConstructorDecl()) could possibly match to constructor of another class defined inside class.

Try
cxxRecordDecl(hasMethod(cxxConstructorDecl(InnerMatcher))))

[Prazek]

clang-tidy/performance/ReturnValueCopyCheck.cpp

53

This one is usefull AF. Can you put into Traversal AST Matchers?

114

maybe isConstructibleFromType? hasConstructorFromType seems like it would take cxxConstructorDecl or something.
BTW what about conversion operator? I don't see any test using it. If implementing this would be too much work, then just write FIXME for now. I guess this case would be very uncommon.

142

dot at the end

[aaron.ballman]

clang-tidy/performance/ReturnValueCopyCheck.cpp

53

I don't think this is a good candidate for a public AST matcher -- it does too many disparate things.

64

With proper documentation and tests, this would be a good candidate to move into ASTMatchers.h though. However, it should be called hasDefaultArg() instead.

69

Arguments are what the caller provides. Parameters are what the function accepts. Also "active" is a bit of a strange term for here, so I think this should be renamed to something like hasOneNonDefaultedParam() or something similar.

77

I think we should make hasType() work for TemplateTypeParmDecl instead of doing it this way (as part of the public AST matcher interface), assuming it doesn't currently work.

86

I think this might actually be better expressed as a local variable in registerMatchers() instead of a matcher like this.

88

s/danger/dangerous
s/with/which

89

s/mean/means
s/cant/can't

111–112

Can you fix the grammatical issues with the comment? I'm not quite certain what it should say, but I think it's something like:

Matches a QualType whose declaration has a converting constructor accepting an argument of the type from the given inner matcher.

130–131

Also ungrammatical (and missing a period at the end of the sentence).

clang-tidy/performance/ReturnValueCopyCheck.h

20

an object of a different type.

[Prazek]

Adding Richard who was working on standard defect report that deprived sense of this check. There are some corner cases that are not in the standard and probably won't be handled in the future that we could handle still.

[alexfh]

removing from my dashboard, while the check is waiting for Richard's comments.

[staronj]

Because of https://reviews.llvm.org/D21619 the check is no longer required.

clang-tidy/performance/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyPerformanceModule
   FasterStringFindCheck.cpp
   ForRangeCopyCheck.cpp
   ImplicitCastInLoopCheck.cpp
   PerformanceTidyModule.cpp
+  ReturnValueCopyCheck.cpp
   UnnecessaryCopyInitialization.cpp
   UnnecessaryValueParamCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyUtils
   )

clang-tidy/performance/PerformanceTidyModule.cpp

 //===--- PeformanceTidyModule.cpp - clang-tidy ----------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "ReturnValueCopyCheck.h"
 
 #include "FasterStringFindCheck.h"
 #include "ForRangeCopyCheck.h"
 #include "ImplicitCastInLoopCheck.h"
 #include "UnnecessaryCopyInitialization.h"
 #include "UnnecessaryValueParamCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace performance {
 
 class PerformanceModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<FasterStringFindCheck>(
         "performance-faster-string-find");
     CheckFactories.registerCheck<ForRangeCopyCheck>(
         "performance-for-range-copy");
     CheckFactories.registerCheck<ImplicitCastInLoopCheck>(
         "performance-implicit-cast-in-loop");
+    CheckFactories.registerCheck<ReturnValueCopyCheck>(
+        "performance-return-value-copy");
     CheckFactories.registerCheck<UnnecessaryCopyInitialization>(
         "performance-unnecessary-copy-initialization");
     CheckFactories.registerCheck<UnnecessaryValueParamCheck>(
         "performance-unnecessary-value-param");
   }
 };
 
 // Register the PerformanceModule using this statically initialized variable.

clang-tidy/performance/ReturnValueCopyCheck.h

+//===--- ReturnValueCopyCheck.h - clang-tidy-----------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_RETURN_VALUE_COPY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_RETURN_VALUE_COPY_H
+
+#include "../ClangTidy.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// This check finds places where we are returning object of a different type than
+/// the function return type. In such places, we should use std::move, otherwise
+/// the object will not be moved automatically.
+/// Check adds std::move if it could be beneficial.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance-return-value-copy.html
+class ReturnValueCopyCheck : public ClangTidyCheck {
+public:
+  ReturnValueCopyCheck(StringRef Name, ClangTidyContext *Context);
+
+  void registerPPCallbacks(CompilerInstance &Compiler) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  std::unique_ptr<utils::IncludeInserter> Inserter;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_RETURN_VALUE_COPY_H

clang-tidy/performance/ReturnValueCopyCheck.cpp

+//===--- ReturnValueCopyCheck.cpp - clang-tidy-----------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ReturnValueCopyCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+using namespace llvm;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+namespace {
+
+/// \brief Matches if the construct expression's callee's declaration matches
+/// the given matcher.
+///
+/// Given
+/// \code
+///   // POD types are trivially move constructible.
+///   struct Foo {
+///     Foo() = default;
+///     Foo(int i) { };
+///   };
+///
+///   Foo a;
+///   Foo b(1);
+///
+/// \endcode
+/// ctorCallee(parameterCountIs(1))
+///   matches "b(1)".
+AST_MATCHER_P(CXXConstructExpr, ctorCallee,
+              ast_matchers::internal::Matcher<CXXConstructorDecl>,
+              InnerMatcher) {
+  const CXXConstructorDecl *CtorDecl = Node.getConstructor();
+  return (CtorDecl != nullptr &&
+          InnerMatcher.matches(*CtorDecl, Finder, Builder));
+}
+
+/// \brief Matches QualType which after removing const and reference
+/// matches the given matcher.
+AST_MATCHER_P(QualType, ignoringRefsAndConsts,
+              ast_matchers::internal::Matcher<QualType>, InnerMatcher) {
+  if (Node.isNull())
+    return false;
+
+  QualType Unqualified = Node.getNonReferenceType();
+  Unqualified.removeLocalConst();
+  return InnerMatcher.matches(Unqualified, Finder, Builder);
+}
+
+/// \brief Matches ParmVarDecls which have default arguments.
+AST_MATCHER(ParmVarDecl, hasDefaultArgument) { return Node.hasDefaultArg(); }
+
+/// \brief Matches function declarations which have all arguments defaulted
+/// except first.
+AST_MATCHER_FUNCTION(ast_matchers::internal::Matcher<FunctionDecl>,
+                     hasOneActiveArgument) {
+  return anyOf(parameterCountIs(1),
+               allOf(unless(parameterCountIs(0)),
+                     hasParameter(1, hasDefaultArgument())));
+}
+
+/// \brief Matches declarations of template type which matches the given
+/// matcher.
+AST_MATCHER_P(TemplateTypeParmDecl, hasTemplateType,
+              ast_matchers::internal::Matcher<QualType>, InnerMatcher) {
+  const QualType TemplateType =
+      Node.getTypeForDecl()->getCanonicalTypeInternal();
+  return InnerMatcher.matches(TemplateType, Finder, Builder);
+}
+
+/// \brief Matches constructor declarations which are templates and can be used
+/// to move-construct from any type.
+AST_MATCHER_FUNCTION(ast_matchers::internal::Matcher<CXXConstructorDecl>,
+                     moveConstructorFromAnyType) {
+  // Potentially danger: this matcher binds a name, with probably
+  // mean that you cant use it twice in your check!
+  const char TemplateArgument[] = "templateArgument";
+  return cxxConstructorDecl(
+      hasParent(functionTemplateDecl(has(templateTypeParmDecl(
+          hasTemplateType(qualType().bind(TemplateArgument)))))),
+      hasOneActiveArgument(),
+      hasParameter(
+          0, hasType(hasCanonicalType(allOf(
+                 rValueReferenceType(),
+                 ignoringRefsAndConsts(equalsBoundNode(TemplateArgument)))))));
+}
+
+/// \brief Matches to qual types which are cxx records and has constructors that
+/// matches the given matcher.
+AST_MATCHER_FUNCTION_P(ast_matchers::internal::Matcher<QualType>,
+                       hasConstructor,
+                       ast_matchers::internal::Matcher<CXXConstructorDecl>,
+                       InnerMatcher) {
+  return hasDeclaration(
+      cxxRecordDecl(hasDescendant(cxxConstructorDecl(InnerMatcher))));
+}
+
+/// \brief Matches to qual types which has constructors from type that matches
+/// the given matcher.
+AST_MATCHER_FUNCTION_P(ast_matchers::internal::Matcher<QualType>,
+                       hasConstructorFromType,
+                       ast_matchers::internal::Matcher<QualType>,
+                       InnerMatcher) {
+  auto ConstructorMatcher = cxxConstructorDecl(
+      unless(isDeleted()), hasOneActiveArgument(),
+      hasParameter(0, hasType(hasCanonicalType(qualType(InnerMatcher)))));
+
+  return hasConstructor(ConstructorMatcher);
+}
+
+} // namespace
+
+void ReturnValueCopyCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  // Matches to type with after ignoring refs and consts is the same as
+  // "constructedType"
+  auto HasTypeSameAsConstructed = hasType(hasCanonicalType(
+      ignoringRefsAndConsts(equalsBoundNode("constructedType"))));
+
+  auto HasRvalueReferenceType =
+      hasType(hasCanonicalType(qualType(rValueReferenceType())));
+
+  auto RefOrConstVarDecl = varDecl(hasType(
+      hasCanonicalType(qualType(anyOf(referenceType(), isConstQualified())))));
+
+  // Matches to expression expression that have declaration with is reference or
+  // const
+  auto IsDeclaredAsRefOrConstType =
+      anyOf(hasDescendant(declRefExpr(to(RefOrConstVarDecl))),
+            declRefExpr(to(RefOrConstVarDecl)));
+
+  // Constructor parameter must not already be rreference
+  auto ParameterMatcher =
+      hasType(hasCanonicalType(qualType(unless(rValueReferenceType()))));
+
+  // Constructor argument must
+  // * have different type than constructed type
+  // * not be r value reference type
+  // * not be declared as const or as reference to other variable
+  // * not be temporary object
+  // * not be cast from temporary object
+  auto ArgumentMatcher =
+      unless(anyOf(HasTypeSameAsConstructed, HasRvalueReferenceType,
+                   IsDeclaredAsRefOrConstType, cxxTemporaryObjectExpr(),
+                   hasDescendant(cxxTemporaryObjectExpr())));
+
+  // Matches to type constructible from argumentCanonicalType type
+  // * by r reference or
+  // * by value, and argumentCanonicalType is move constructible
+  auto IsMoveOrCopyConstructibleFromParam = hasConstructorFromType(anyOf(
+      allOf(rValueReferenceType(),
+            ignoringRefsAndConsts(equalsBoundNode("argumentCanonicalType"))),
+      allOf(equalsBoundNode("argumentCanonicalType"),
+            hasConstructor(isMoveConstructor()))));
+
+  // Matches to type that has template constructor which is
+  // move constructor from any type (like boost::any)
+  auto IsCopyConstructibleFromParamViaTemplate =
+      hasConstructor(moveConstructorFromAnyType());
+
+  // Matches construct expr that
+  // * has one argument
+  // * that argument satisfies ArgumentMatcher
+  // * argument is not the result of move constructor
+  // * parameter of constructor satisfies ParameterMatcher
+  // * constructed type is move constructible from argument
+  // ** or is value constructible from argument and argument is movable
+  //    constructible
+  // ** constructed type has template constructor that can take by rref
+  //    (like boost::any)
+  auto ConstructExprMatcher =
+      cxxConstructExpr(
+          hasType(qualType().bind("constructedType")), argumentCountIs(1),
+          unless(has(ignoringParenImpCasts(
+              cxxConstructExpr(ctorCallee(isMoveConstructor()))))),
+          hasArgument(0, hasType(hasCanonicalType(ignoringRefsAndConsts(
+                             qualType().bind("argumentCanonicalType"))))),
+          ctorCallee(hasParameter(0, ParameterMatcher)),
+          hasArgument(0, ArgumentMatcher),
+          hasArgument(0, expr().bind("argument")),
+          hasType(qualType(anyOf(IsMoveOrCopyConstructibleFromParam,
+                                 IsCopyConstructibleFromParamViaTemplate))))
+          .bind("construct");
+
+  auto IsCopyOrMoveConstructor =
+      anyOf(isCopyConstructor(), isMoveConstructor());
+
+  Finder->addMatcher(
+      returnStmt(has(ignoringParenImpCasts(cxxConstructExpr(
+                     ctorCallee(IsCopyOrMoveConstructor),
+                     has(ignoringParenImpCasts(ConstructExprMatcher))))))
+          .bind("return"),
+      this);
+}
+
+ReturnValueCopyCheck::ReturnValueCopyCheck(StringRef Name,
+                                       ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+          Options.get("IncludeStyle", "llvm"))) {}
+
+void ReturnValueCopyCheck::registerPPCallbacks(CompilerInstance &Compiler) {
+  // Only register the preprocessor callbacks for C++; the functionality
+  // currently does not provide any benefit to other languages, despite being
+  // benign.
+  if (getLangOpts().CPlusPlus11) {
+    Inserter.reset(new utils::IncludeInserter(
+        Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle));
+    Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks());
+  }
+}
+
+void ReturnValueCopyCheck::check(const MatchFinder::MatchResult &Result) {
+  const LangOptions &Opts = Result.Context->getLangOpts();
+
+  const auto *Argument = Result.Nodes.getNodeAs<Expr>("argument");
+  assert(Argument != nullptr);
+
+  const auto *Type = Result.Nodes.getNodeAs<QualType>("constructedType");
+  assert(Type != nullptr);
+  assert(!Type->isNull());
+
+  std::string ReplacementText = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(Argument->getSourceRange()),
+      *Result.SourceManager, Opts);
+
+  ReplacementText =
+      Type->getAsString(getLangOpts()) + "(std::move(" + ReplacementText + "))";
+
+  auto Diag = diag(Argument->getExprLoc(),
+                   "returned object is not moved; consider wrapping it with "
+                   "std::move or changing return type to avoid the copy");
+  Diag << FixItHint::CreateReplacement(Argument->getSourceRange(),
+                                       ReplacementText);
+
+  if (auto IncludeFixit = Inserter->CreateIncludeInsertion(
+          Result.SourceManager->getFileID(Argument->getLocStart()), "utility",
+          /*IsAngled=*/true)) {
+    Diag << *IncludeFixit;
+  }
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang

docs/ReleaseNotes.rst

- New `performance-faster-string-find
   is a single character string literal.
 
 - New `performance-implicit-cast-in-loop
   <http://clang.llvm.org/extra/clang-tidy/checks/performance-implicit-cast-in-loop.html>`_ check
 
   Warns about range-based loop with a loop variable of const ref type where the
   type of the variable does not match the one returned by the iterator.
 
+- New `performance-return-value-copy
+  <http://clang.llvm.org/extra/clang-tidy/checks/performance-return-value-copy.html>`_ check
+
+  Adds `std::move` in returns statements where returned object is copied and
+  adding `std::move` can make it being moved.
+
 - New `performance-unnecessary-value-param
   <http://clang.llvm.org/extra/clang-tidy/checks/performance-unnecessary-value-param.html>`_ check
 
   Flags value parameter declarations of expensive to copy types that are copied
   for each invocation but it would suffice to pass them by const reference.
 
 - New `readability-avoid-const-params-in-decls
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-avoid-const-params-in-decls.html>`_ check

docs/clang-tidy/checks/list.rst

.. toctree::
    modernize-use-auto
    modernize-use-bool-literals
    modernize-use-default
    modernize-use-nullptr
    modernize-use-override
    performance-faster-string-find
    performance-for-range-copy
    performance-implicit-cast-in-loop
+   performance-return-value-copy
    performance-unnecessary-copy-initialization
    performance-unnecessary-value-param
    readability-avoid-const-params-in-decls
    readability-braces-around-statements
    readability-container-size-empty
    readability-deleted-default
    readability-else-after-return
    readability-function-size

docs/clang-tidy/checks/performance-return-value-copy.rst

+.. title:: clang-tidy - performance-return-value-copy
+
+performance-return-value-copy
+==========================
+
+Adds `std::move` in returns statements where returned object is copied and
+adding `std::move` can make it being moved.
+
+This check requires C++11 or higher to run.
+
+For returning object of type A in function with return type B
+check triggers if:
+- B is move constructible from A
+- B is constructible from value of A and A is movable.
+
+check doesn't trigger when
+- A is same type as B
+- temporary object is returned
+- returned object was declared as const or as reference
+- A already has rvalue reference type
+
+For example:
+
+.. code-block:: c++
+
+  boost::optional<std::string> get() {
+    string s;
+    ;;;
+    return s;
+  }
+
+  // becomes
+
+  boost::optional<std::string> get() {
+    string s;
+    ;;;
+    return boost::optional<std::string>(std::move(s));
+  }
+
+Of course if we return an rvalue (e.g., temporary) we don’t have to move it:
+
+.. code-block:: c++
+
+  boost::optional<std::string> get() {
+    return "str";
+  }

test/clang-tidy/performance-return-value-copy.cpp

+// RUN: %check_clang_tidy %s performance-return-value-copy %t
+// CHECK-FIXES: {{^}}#include <utility>{{$}}
+
+// we need std::move mock
+namespace std {
+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) noexcept { return static_cast<typename std::remove_reference<_Tp>::type &&>(__t); }
+}
+
+class SimpleClass {
+public:
+  SimpleClass() = default;
+  SimpleClass(const SimpleClass &) = default;
+  SimpleClass(SimpleClass &&) = default;
+
+  // We don't want to add std::move here because it will be added by compiler
+  SimpleClass foo(SimpleClass a, const SimpleClass b, SimpleClass &c, const SimpleClass &d, SimpleClass &&e, const SimpleClass &&f, char k) {
+    switch (k) {
+    case 'a':
+      return a;
+    case 'b':
+      return b;
+    case 'c':
+      return c;
+    case 'd':
+      return d;
+    case 'e':
+      return e;
+    case 'f':
+      return f;
+    default:
+      return SimpleClass();
+    }
+  }
+};
+
+SimpleClass simpleClassFoo() {
+  return SimpleClass();
+}
+
+class FromValueClass {
+public:
+  FromValueClass(SimpleClass a) {}
+
+  FromValueClass foo(SimpleClass a, const SimpleClass b, SimpleClass &c, const SimpleClass &d, SimpleClass &&e, const SimpleClass &&f, char k) {
+    switch (k) {
+    case 'a':
+      // Because SimpleClass is move constructible
+      return a;
+    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: returned object is not moved; consider wrapping it with std::move or changing return type to avoid the copy [performance-return-value-copy]
+    // CHECK-FIXES: {{^ *}}return FromValueClass(std::move(a));{{$}}
+    case 'b':
+      return b;
+    case 'c':
+      return c;
+    case 'd':
+      return d;
+    case 'e':
+      return e;
+    case 'f':
+      return f;
+    case 'g':
+      return simpleClassFoo();
+    default:
+      return SimpleClass();
+    }
+  }
+};
+
+class FromRRefClass {
+public:
+  FromRRefClass() = default;
+  FromRRefClass(const SimpleClass &a) {}
+  FromRRefClass(SimpleClass &&a) {}
+
+  FromRRefClass foo1(SimpleClass a, const SimpleClass b, SimpleClass &c, const SimpleClass &d, SimpleClass &&e, const SimpleClass &&f, char k) {
+    switch (k) {
+    case 'a':
+      return a;
+    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: {{..}}
+    // CHECK-FIXES: {{^ *}}return FromRRefClass(std::move(a));{{$}}
+
+    // We don't want to add std::move in cases 'b-f because
+    case 'b':
+      return b;
+    case 'c':
+      return c;
+    case 'd':
+      return d;
+    case 'e':
+      return e;
+    case 'f':
+      return f;
+    case 'g':
+      return simpleClassFoo();
+    default:
+      return SimpleClass();
+    }
+  }
+
+  FromRRefClass foo2(char k) {
+    SimpleClass a;
+    const SimpleClass &b = a;
+    SimpleClass &c = a;
+    SimpleClass *d = &a;
+    const SimpleClass e;
+
+    switch (k) {
+    case 'a':
+      return a;
+    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: {{..}}
+    // CHECK-FIXES: {{^ *}}return FromRRefClass(std::move(a));{{$}}
+    case 'b':
+      return b;
+    case 'c':
+      return c;
+    case 'd':
+      return *d;
+    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: {{..}}
+    // CHECK-FIXES: {{^ *}}return FromRRefClass(std::move(*d));{{$}}
+    case 'e':
+      return e;
+    case 'f':
+      return simpleClassFoo();
+    case 'x':
+      return SimpleClass();
+    case 'y':
+      return FromRRefClass(SimpleClass());
+    }
+  }
+
+  FromRRefClass foo3(char k) {
+    SimpleClass a;
+    SimpleClass b;
+    FromRRefClass c;
+    switch (k) {
+    case 'a':
+      return std::move(a);
+    case 'b':
+      return FromRRefClass(std::move(a));
+    case 'c':
+      return c;
+    default:
+      return FromRRefClass();
+    }
+  }
+};
+
+template <typename T>
+FromRRefClass justTemplateFunction(T &&t) {
+  return t;
+}
+
+void call_justTemplateFunction() {
+  justTemplateFunction(SimpleClass{});
+  SimpleClass a;
+  justTemplateFunction(a);
+  justTemplateFunction(FromRRefClass{});
+  FromRRefClass b;
+  justTemplateFunction(b);
+}
+
+class FromValueWithDeleted {
+public:
+  FromValueWithDeleted() = default;
+  FromValueWithDeleted(SimpleClass a) {}
+  FromValueWithDeleted(SimpleClass &&a) = delete;
+};
+
+FromValueWithDeleted foo9() {
+  SimpleClass a;
+  return a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: {{..}}
+  // CHECK-FIXES: {{^ *}}return FromValueWithDeleted(std::move(a));{{$}}
+}
+
+template <typename T>
+struct old_optional {
+  old_optional(const T &t) {}
+};
+
+old_optional<SimpleClass> test_old_optional() {
+  SimpleClass a;
+  return a;
+}
+
+template <typename T>
+struct optional {
+  optional(const T &) {}
+  optional(T &&) {}
+};
+
+optional<SimpleClass> test_optional() {
+  SimpleClass a;
+  return a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: {{..}}
+  // CHECK-FIXES: {{^ *}}return optional<SimpleClass>(std::move(a));{{$}}
+}
+
+using Opt = optional<SimpleClass>;
+Opt test_Opt() {
+  SimpleClass a;
+  return a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: {{..}}
+  // CHECK-FIXES: {{^ *}}return Opt(std::move(a));{{$}}
+}
+
+struct any {
+  any() = default;
+
+  template <typename T>
+  any(T &&) {}
+
+  any(any &&) = default;
+  any(const any &) = default;
+};
+
+any test_any() {
+  SimpleClass a;
+  return a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: {{..}}
+  // CHECK-FIXES: {{^ *}}return any(std::move(a));{{$}}
+}
+
+any test_any2() {
+  SimpleClass a;
+  return std::move(a);
+}
+any test_any3() {
+  SimpleClass a;
+  return any(std::move(a));
+}
+
+any test_any4() {
+  any a;
+  return a;
+}
+
+class FromRefWithDeleted {
+public:
+  FromRefWithDeleted(SimpleClass &&) = delete;
+  FromRefWithDeleted(const SimpleClass &a) {}
+};
+
+FromRefWithDeleted foo11(SimpleClass a) {
+  return a;
+}
+
+class ClassWithUsings {
+public:
+  using value = SimpleClass;
+  using const_value = const SimpleClass;
+  using lreference = SimpleClass &;
+  using const_lreference = const SimpleClass &;
+  using rreference = SimpleClass &&;
+  using const_rreference = const SimpleClass &&;
+
+  ClassWithUsings(rreference) {}
+  ClassWithUsings(const_lreference) {}
+
+  ClassWithUsings foo(value a, const_value b, lreference c, const_lreference d, rreference e, const_rreference f, char k) {
+    switch (k) {
+    case 'a':
+      return a;
+    // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: {{..}}
+    // CHECK-FIXES: {{^ *}}return ClassWithUsings(std::move(a));{{$}}
+    case 'b':
+      return b;
+    case 'c':
+      return c;
+    case 'd':
+      return d;
+    case 'e':
+      return e;
+    case 'f':
+      return f;
+    case 'g':
+      return simpleClassFoo();
+    default:
+      return SimpleClass();
+    }
+  }
+};
+
+class FromRRefWithDefaultArgs {
+public:
+  FromRRefWithDefaultArgs(SimpleClass &&, int k = 0) {}
+  FromRRefWithDefaultArgs(const SimpleClass &) {}
+
+  FromRRefWithDefaultArgs foo(SimpleClass a) {
+    return a;
+    // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: {{..}}
+    // CHECK-FIXES: {{^ *}}return FromRRefWithDefaultArgs(std::move(a));{{$}}
+  }
+};