Quuxplusone added inline comments.

================
Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:263
+  forwardToShowInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' 
of the trivially-copyable type 'int' has no effect; consider changing 
forwardToShowInt's parameter from 'int'&& to 'int'&
+}
----------------
`forwardToShowInt` takes `T&&`, not `int&&`, and so it can't be changed in the 
way the diagnostic is suggesting. I think the right answer here is not to emit 
the diagnostic at all, when the offending function is a template.

(Relatively minor nits, all moot because this diagnostic should not be emitted 
at all: `'int'&&` should be `'int&&'`, `trivially copyable` should not be 
hyphenated, and `int&` is a strictly worse suggestion than either `const int&` 
or plain `int`.  IMVHO it would be reasonable to land a very trivial patch to 
remove the hyphen from `trivially copyable` and update the tests, and then 
rebase this PR on top of that.)


================
Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:271
+  showTmp(std::move(t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 't' 
of the trivially-copyable type 'Tmp' has no effect; remove std::move()
+}
----------------
This is precisely the bogus message you're trying to eliminate, isn't it? 
Removing `std::move` would make the code fail to compile because `t` is an 
lvalue but `showTmp` requires an rvalue.


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

https://reviews.llvm.org/D107450

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to