[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check

Fri Jan 8 08:59:51 PST 2021

aaron.ballman added a comment.

In D69560#2487093 <https://reviews.llvm.org/D69560#2487093>, @whisperity wrote:

> I have posted two questions to GitHub, mostly related to the guideline rule and how free the implementation could be: #1732 <http://github.com/isocpp/CppCoreGuidelines/issues/1732> and #1733 <http://github.com/isocpp/CppCoreGuidelines/issues/1733>, I think I tagged you on both.
>
> However, if we are going down the alias route, I think it should be the guideline version which is the alias, and this check should live under a different name. Where do you think should we put it? I think `bugprone-` is the best fit, and `readability-` is the second-best...

I think `bugprone` is where it should ideally live, but if we still think we have "too high" of a false positive rate (for however we decide we want to measure that), then `readability` is a good fallback. (I'm assuming more people enable `bugprone-*` than enable `readability-*`.)

> The "avoid adjacent parameter of similar types" name could stay... or something like `...-easily-swappable-function-parameters`, that involves all the features here, with things like "default length 3" and such, "relatedness" heuristics turned on.

Hmm, how about `bugprone-easily-swappable-parameters`? We can drop the `function` from it because we probably also care about function-like things (like lambdas and blocks) and `parameters` should be sufficiently clear as to what's getting checked.

> Not sure what CVR-modelling's default should be... it finds less when "off", but too easily silences crucial issues (such as `memcpy(T*, const T*)`).

My instinct is that if you accidentally swap a qualified parameter you'll get some other diagnostic about dropped qualifiers and so perhaps the default should be `off`, but perhaps there's something I'm not thinking of there.

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

https://reviews.llvm.org/D69560