[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

In D141569#4139320 <https://reviews.llvm.org/D141569#4139320>, @ccotter wrote:

> I was split on handling the case where the parameter that was never moved from was not named. For this guideline enforcement to flag all unmoved rvalue reference parameters, code that `std::moves` into an argument to a virtual method call could be especially confusing or dangerous with regards to whether the object was truly "moved" (move constructor/assignment invoked within the method call). E.g.,
>
>   LargeObject obj;
>   ClassWithVirtualMerthods* v = ...;
>   v->some_virtual_call(std::move(obj));
>
> If would be confusing if some implementations of the virtual method actually moved the parameter, while others didn't. I'm inclined in this case (regardless of whether the parameter is named in a virtual overridden method) to still flag this code. Looking at the rule enforcement criteria again, the rule states "Don’t conditionally move from objects" which this case falls into.
>
> I could see there being an option for this specific case (defaulted off), although it's very specific of a case, and for something I think the rule should be especially attentive towards. Could I get some additional feedback on this specific case?

I'm not so sure, you may have Base class Car that would have void changeBackRightDoor(Door&&) method, and then you could have class Car5Doors and Car3Doors, in such case its easy to known that Car3Doors wouldn't implement changeBackRightDoor method, and that method wouldnt move anything, using unnamed parameters is a way to go. If you dont exclude this, then at least provide option for that, like IgnoreUnnamedParameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569