Sockke added a comment.

Which do you prefer? @Quuxplusone



================
Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:262-263
+  int a = 10;
+  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
+}
----------------
Quuxplusone wrote:
> ```
>   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
> ```
> I continue to want-not-to-block this PR, since I think it's improving the 
> situation. However, FWIW...
> It's good that this message doesn't contain a fixit, since there's nothing 
> the programmer can really do to "fix" this call. But then, should this 
> message be emitted at all? If this were `clang -Wall`, then we definitely 
> would //not// want to emit a "noisy" warning where there's basically nothing 
> the programmer can do about it. Does clang-tidy have a similar philosophy? Or 
> is it okay for clang-tidy to say "This code looks wonky" even when there's no 
> obvious way to fix it?
> ```
>   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
> ```
> I continue to want-not-to-block this PR, since I think it's improving the 
> situation. However, FWIW...
> It's good that this message doesn't contain a fixit, since there's nothing 
> the programmer can really do to "fix" this call. But then, should this 
> message be emitted at all? If this were `clang -Wall`, then we definitely 
> would //not// want to emit a "noisy" warning where there's basically nothing 
> the programmer can do about it. Does clang-tidy have a similar philosophy? Or 
> is it okay for clang-tidy to say "This code looks wonky" even when there's no 
> obvious way to fix it?

Yes, I agree with you. I did not remove warning to maintain the original 
behavior, which will make the current patch clearer. In any case, it is easy 
for me to make this modification if you want. 


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