Quuxplusone added inline comments.

================
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);
----------------
whisperity wrote:
> MTC wrote:
> > 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.
> Yes, but there is no //FixIt// for this warning, but even if the user fixes 
> this //manually// according to the warning, it'll result in
> 
> > error: cannot bind rvalue reference of type `int&&` to lvalue of type `int`
> 
> because `showInt` takes `int&&`!
> 
> So this is not just unfixable automatically (hence no `CHECK-FIXES` line), it 
> isn't fixable at all.
At a really high level, yeah, this code is bad code. :) But this is exactly 
Sockke's "case 1":
> A trivially-copyable object wrapped by std::move is passed to the function 
> with rvalue reference parameters. Removing std::move will cause compilation 
> errors.
So a warning here would have to be, like, "`std::move` of `a`, of trivially 
copyable type `int`, has no runtime effect; consider changing `showInt`'s 
parameter from `int&&` to `int`."
Also, should add a test case of the form
```
void showInt(int&&);
template<class T> void forwardToShowInt(T&& t) { showInt(static_cast<T&&>(t)); }
void test() {
    int a = 10;
    forwardToShowInt(std::move(a));
}
```
to make sure the diagnostic (or lack thereof) plays well with templates. Here, 
refactoring `forwardToShowInt`'s parameter from `T&&` to `T` is unlikely to be 
a realistic option.


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