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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
