[PATCH] D78652: [clang-tidy] Add "ignore related parameters" heuristics to 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type'

[Whisperity via Phabricator via cfe-commits]

whisperity created this revision.
whisperity added reviewers: aaron.ballman, alexfh, hokein, JonasToth, zporky.
whisperity added projects: clang-tools-extra, clang.
Herald added subscribers: cfe-commits, martong, gamesh411, dkrupp, rnkovacs, kbarton, nemanjai.
whisperity added a parent revision: D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type' check.
Herald added a subscriber: wuzish.

The basic version of the checker only considers the pseudo-equality between types of parameters as the predicate on deciding whether or not the two can be mixed up with each other at a potential call site. This, together with a low minimum range length requirement, results in an incredibly verbose checker with report count potentially in the multiple thousands.

This patch aims to mitigate that problem by silencing warnings about adjacent parameter ranges in which the parameters are //related// to each other.
While this change definitely brings in false negatives (as mixing the arguments to `max(a, b)` is still a potential issue, but we really can't do better in that case...), the sheer amount of reduction in the report count should make up for it. Across multiple projects (from both C and C++), the number of reports has dropped by **at least 40%**.
This should help to make sure that the dodgiest interfaces are not hidden under clumps of naively generated reports.

Currently, three relatedness heuristics are implemented, each in a way that it is easy to, in the future, remove them or add new ones.

- Common expression: if both parameters are found in a common expression somewhere in the function, they are considered related. This heuristic is an absolute blast because it deals with arithmetics, comparisons, and forwarding functions all in one.
- Return: if both parameters are returned on different execution paths of the function, they are considered related. This deals with switch-like functions.
- Pass to same function: if both parameters are passed to the same function in the same index, they are considered related. This is a special case of forwarding. I.e. for `a` and `b` if there exists a call `f(foo, a);` and `f(bar, b);`, they are deemed related. Thanks to @zporky, our C++ expert, the idea.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78652

Files:
  clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.cpp
  clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.h
  clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-cvr-on.cpp
  clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-relatedness.cpp
  clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78652.259321.patch
Type: text/x-patch
Size: 21439 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200422/e82ff218/attachment-0001.bin>