[PATCH] D107450: [clang-tidy] Fix wrong FixIt in performance-move-const-arg

Whisperity via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 26 06:03:46 PDT 2021


whisperity added a comment.

I think this is becoming okay and looks safe enough. I can't really grasp what `HasCheckedMoveSet` means, and how it synergises with storing `CallExpr`s so maybe a better name should be added. Do you mean `AlreadyCheckedMoves`? When is it possible that the same `CallExpr` of `std::move` will be matched multiple times?

In general going from `T&&` to `const T&` would be a broken change if the function implementation //calls// a non-const member method on the `T` instance, but I can be convinced that saying `consider [...]` instead of generating a bogus fix-it is enough, there is hopefully no need to extensively analyse and guard against this.

There has been a new release since the patch was proposed so a rebase should be needed.
@aaron.ballman What do you think, does this warrant a ReleaseNotes entry?


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

https://reviews.llvm.org/D107450



More information about the cfe-commits mailing list