[PATCH] D107450: [clang-tidy] Fix wrong and missing warnings in performance-move-const-arg

[liushuai wang via Phabricator via cfe-commits]

MTC added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:75
+  return std::move(x4); 
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: passing result of std::move() as a const reference argument; no move will actually happen
+  // CHECK-FIXES: return x4;
----------------
Quuxplusone wrote:
> You're right that //some// diagnostic should be given here, but //this// is not the right diagnostic. The issue is not that the `std::move` is non-functional, but rather that it's superfluous; a move //will// happen, with or without the `std::move`. (And the `std::move` prevents move elision, so removing it may actually improve codegen.)
Correct!  @Sockke would you self-examination these cases again? I have checked this case, see https://godbolt.org/z/3ch3sbG17.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:261
+  showInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect; remove std::move()
+  return std::move(a);
----------------
Quuxplusone wrote:
> This warning is incorrect, as indicated in your PR summary. Isn't the point of this PR mainly to remove this incorrect warning?
I guess what @Sockke wants to express is that applying `std::move` on the integer type makes no sense.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107450/new/

https://reviews.llvm.org/D107450