Add an option to misc-move-const-arg to not diagnose on trivially copyable types.

Patch by Oleg Smolsky

llvm-svn: 319111

clang-tools-extra/clang-tidy/misc/MoveConstantArgumentCheck.cpp

@@ -36,6 +36,11 @@ static void ReplaceCallWithArg(const CallExpr *Call, DiagnosticBuilder &Diag,
   }
 }
 
+void MoveConstantArgumentCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "CheckTriviallyCopyableMove", CheckTriviallyCopyableMove);
+}
+
 void MoveConstantArgumentCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
     return;
@@ -85,6 +90,10 @@ void MoveConstantArgumentCheck::check(const MatchFinder::MatchResult &Result) {
           return;
       }
     }
+
+    if (!IsConstArg && IsTriviallyCopyable && !CheckTriviallyCopyableMove)
+      return;
+
     bool IsVariable = isa<DeclRefExpr>(Arg);
     const auto *Var =
         IsVariable ? dyn_cast<DeclRefExpr>(Arg)->getDecl() : nullptr;

clang-tools-extra/clang-tidy/misc/MoveConstantArgumentCheck.h

@@ -16,12 +16,24 @@ namespace clang {
 namespace tidy {
 namespace misc {
 
+/// Find casts of calculation results to bigger type. Typically from int to
+///
+/// There is one option:
+///
+///   - `CheckTriviallyCopyableMove`: Whether to check for trivially-copyable
+//      types as their objects are not moved but copied. Enabled by default.
 class MoveConstantArgumentCheck : public ClangTidyCheck {
 public:
   MoveConstantArgumentCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+      : ClangTidyCheck(Name, Context),
+        CheckTriviallyCopyableMove(
+            Options.get("CheckTriviallyCopyableMove", true)) {}
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool CheckTriviallyCopyableMove;
 };
 
 } // namespace misc

clang-tools-extra/docs/clang-tidy/checks/misc-move-const-arg.rst

@@ -27,3 +27,11 @@ Here are examples of each of the three cases:
   void f(const string &s);
   string s;
   f(std::move(s));  // Warning: passing result of std::move as a const reference argument; no move will actually happen
+
+Options
+-------
+
+.. option:: CheckTriviallyCopyableMove
+
+   If non-zero, enables detection of trivially copyable types that do not
+   have a move constructor. Default is non-zero.

clang-tools-extra/test/clang-tidy/misc-move-const-arg-trivially-copyable.cpp

@@ -0,0 +1,98 @@
+// RUN: %check_clang_tidy %s misc-move-const-arg %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: misc-move-const-arg.CheckTriviallyCopyableMove, value: 0}]}' \
+// RUN: -- -std=c++14
+
+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
+
+class NoMoveSemantics {
+ public:
+  NoMoveSemantics();
+  NoMoveSemantics(const NoMoveSemantics &);
+
+  NoMoveSemantics &operator=(const NoMoveSemantics &);
+};
+
+void callByConstRef(const NoMoveSemantics &);
+void callByConstRef(int i, const NoMoveSemantics &);
+
+void moveToConstReferencePositives() {
+  NoMoveSemantics obj;
+
+  // Basic case. It is here just to have a single "detected and fixed" case.
+  callByConstRef(std::move(obj));
+  // CHECK-MESSAGES: :[[@LINE-1]]:18:  warning: passing result of std::move() as a const reference argument; no move will actually happen [misc-move-const-arg]
+  // CHECK-FIXES: callByConstRef(obj);
+}
+
+struct TriviallyCopyable {
+  int i;
+};
+
+void f(TriviallyCopyable) {}
+
+void g() {
+  TriviallyCopyable obj;
+  f(std::move(obj));
+}
+
+class MoveSemantics {
+ public:
+  MoveSemantics();
+  MoveSemantics(MoveSemantics &&);
+
+  MoveSemantics &operator=(MoveSemantics &&);
+};
+
+void fmovable(MoveSemantics);
+
+void lambda1() {
+  auto f = [](MoveSemantics m) {
+    fmovable(std::move(m));
+  };
+  f(MoveSemantics());
+}
+
+template<class T> struct function {};
+
+template<typename Result, typename... Args>
+class function<Result(Args...)> {
+public:
+  function() = default;
+  void operator()(Args... args) const {
+    fmovable(std::forward<Args>(args)...);
+  }
+};
+
+void functionInvocation() {
+  function<void(MoveSemantics)> callback;
+  MoveSemantics m;
+  callback(std::move(m));
+}
+
+void lambda2() {
+  function<void(MoveSemantics)> callback;
+
+  auto f = [callback = std::move(callback)](MoveSemantics m) mutable {
+    callback(std::move(m));
+  };
+  f(MoveSemantics());
+}

clang-tools-extra/test/clang-tidy/misc-move-const-arg.cpp

@@ -14,6 +14,12 @@ 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
 
 class A {
@@ -23,6 +29,19 @@ public:
   A(A &&rhs) {}
 };
 
+struct TriviallyCopyable {
+  int i;
+};
+
+void f(TriviallyCopyable) {}
+
+void g() {
+  TriviallyCopyable obj;
+  f(std::move(obj));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: std::move of the variable 'obj' of the trivially-copyable type 'TriviallyCopyable' has no effect; remove std::move() [misc-move-const-arg]
+  // CHECK-FIXES: f(obj);
+}
+
 int f1() {
   return std::move(42);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: std::move of the expression of the trivially-copyable type 'int' has no effect; remove std::move() [misc-move-const-arg]
@@ -176,3 +195,40 @@ void Q(T);
 void moveOnlyNegatives(MoveOnly val) {
   Q(std::move(val));
 }
+
+void fmovable(MoveSemantics);
+
+void lambda1() {
+  auto f = [](MoveSemantics m) {
+    fmovable(std::move(m));
+  };
+  f(MoveSemantics());
+}
+
+template<class T> struct function {};
+
+template<typename Result, typename... Args>
+class function<Result(Args...)> {
+public:
+  function() = default;
+  void operator()(Args... args) const {
+    fmovable(std::forward<Args>(args)...);
+  }
+};
+
+void functionInvocation() {
+  function<void(MoveSemantics)> callback;
+  MoveSemantics m;
+  callback(std::move(m));
+}
+
+void lambda2() {
+  function<void(MoveSemantics)> callback;
+
+  auto f = [callback = std::move(callback)](MoveSemantics m) mutable {
+    // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: std::move of the variable 'callback' of the trivially-copyable type 'function<void (MoveSemantics)>' has no effect; remove std::move()
+    // CHECK-FIXES: auto f = [callback = callback](MoveSemantics m) mutable {
+    callback(std::move(m));
+  };
+  f(MoveSemantics());
+}