[PATCH] D116513: [clang-tidy] Fix bugs in misc-unused-parameters for Constructors calls site

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

Thanks @aaron.ballman for the review!



================
Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp:157-159
 // CHECK-FIXES: C() {}
   C(int i) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning
----------------
aaron.ballman wrote:
> I think this fix is incorrect and should not be applied; it changes the meaning of the interface from having a converting constructor to having a default constructor. I think that needs to be optional behavior because it's a pretty invasive change to apply automatically. This patch builds on top of the existing incorrect behavior, but would be fine if it was only applied when the option to modify constructors is enabled.
I'm not against an option, but I'd like to get to a default behavior that is "safe". So I guess my first patch should be to undo the transformation happening here in the test already.
We can either never touch any C++ constructor or try to find a conservative logic for it.
I mentioned in the other review that we may avoid touching a Ctor with a single parameter. 

Then we also can't do it for a Ctor with two parameters as it'll turn it into a converting ctor. Unless you can eliminate both parameters, in which case it is become a default ctor (which can conflict with an existing one, in which case it can be just deleted?)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116513