[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

[Whisperity via Phabricator via cfe-commits]

whisperity created this revision.
whisperity added a reviewer: aaron.ballman.
whisperity added projects: clang, clang-tools-extra.
Herald added subscribers: martong, gamesh411, Szelethus, rnkovacs, xazax.hun.
whisperity requested review of this revision.

The base patch only deals with strict (canonical) type equality, which is merely a subset of all the dangerous function interfaces that we intend to find. In addition, in the base patch, canonical type equivalence is not diagnosed in a way that is immediately apparent to the user.

This patch extends the check with two features:

1. Proper `typedef` diagnostics and explanations to the user.
2. //"Reference bind power"// matching.

**Case 1** will tell the user //the common type of "SomeIntegerType" and "SomeOtherIntegerType" is "int"// or //"my_int" and "int" are the same//. This makes the (otherwise, through `CanonicalType` desugaring already handled and diagnosed, but not //explained//) typedef case obvious to the user without having to invoke code comprehension tools.

**Case 2** is a necessary addition because in every case someone encounters a function `f(T t, const T& tr)`, any expression that might be passed to either can be passed to both. Thus, such adjacent parameter sequences should be matched. This is //specifically// modelled for `const&` only, as any other pair combination from "value, ref, qualified ref, rref" have //some// broad value category in which some actual swap of the arguments at a call would be a compiler error. (E.g. `f(int, int&)`  and `f(2, Val)` breaks if swapped, similarly how rrefs cannot bind lvalues, etc.) This case **broadens** the result set, but only with the most dangerous and "Please think about this and refactor..." case.

(Note, that //Case 2// does not model things like `f(int, std::reference_wrapper<const int>)`.)

Due to the cases above always being a problem (and there isn't a good enough reason to say //typedefs should not be diagnosed//, everyone hates the weak typedef...), I decided **not** to make matching these configurable.

This patch also serves as a proof-of-concept on how I wish the check to evolve over some more subsequent patches, which all aim to introduce user-toggleable modelling.

> Originally, the implementation of this feature was part of the very first patch related to this check, D69560 <https://reviews.llvm.org/D69560> (diff 259320 <http://reviews.llvm.org/D69560?id=259320>). It has been factored out for clarity and to allow more on-point discussion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95736

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D95736.320297.patch
Type: text/x-patch
Size: 28804 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210130/f817bde2/attachment-0001.bin>