[PATCH] D74463: [clang-tidy] Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

[Kim Viggedal via Phabricator via cfe-commits]

vingeldal marked 2 inline comments as done.
vingeldal added a comment.

In D74463#1878144 <https://reviews.llvm.org/D74463#1878144>, @njames93 wrote:

> I have a feeling this check is going to throw so many false positives that it'll be too noisy to run on any real codebase. 
>  There should be a way to silence this when it is undesired like in the example for `int max(int a, int b);`. 
>  A possible solution could be based on parameter name length, usually interchangeable parameters have short names. 
>  Maybe an option could be added `IgnoreShortParamNamesSize` which takes an int and disregards results if both params are shorter, set as `0` to disable the option


I can't dispute that there will be false positives with this check, but I'm not sure there will be a problematic amount in many code bases.

I am pretty sure that an option to allow short names would cause  a relatively big hit on performance (relative to how it runs without the option) for this check while also potentially causing some false negatives (which I would very much like to avoid).
Since actually running the check is still optional I think it might be better to just not run this check on a code base where it gives a lot of false positives or suppress it in select header files where the false positives are plenty.
I'm feeling a bit reluctant to adding the suggested option, are you sure such an option would be worth the cost?
Also consider that even in the cases where the order of the parameters doesn't matter, like an averaging function, there is also the possibility to re-design to avoid this warning, by passing the parameters as some kind of collection of parameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74463