Quuxplusone added inline comments.
================ 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 +} ---------------- Sockke wrote: > 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. I'll defer on this subject to whomever you get to review the code change in `MoveConstArgCheck.cpp`. (@whisperity perhaps?) 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
