Commit Graph

15 Commits

Author SHA1 Message Date
Nathan James a4bdeb9a56
[clang-tidy][NFC] replace some redundant std::string creations 2022-01-19 18:36:35 +00:00
Salman Javed ade0662c51 [clang-tidy] Fix lint warnings in clang-tidy source code (NFC)
Run clang-tidy on all source files under `clang-tools-extra/clang-tidy`
with `-header-filter=clang-tidy.*` and make suggested corrections.

Differential Revision: https://reviews.llvm.org/D112864
2021-11-02 20:14:25 +13:00
Salman Javed 897402e959 [clang-tidy] Correct typo in bugprone-easily-swappable-parameters
The string table `DefaultIgnoredParameterTypeSuffixes` has a typo:
`ForwardIt` is mistyped as `FowardIt`.

Correct typo and add test coverage.

Differential Revision: https://reviews.llvm.org/D112596
2021-10-27 22:38:52 +13:00
Whisperity 8d50a847d4 [clang-tidy] Hotfix default parameter value in 'bugprone-easily-swappable-parameters'
As identified by @RKSimon, there was a missing comma in the default
value for the "ignored parameter type suffixes" array, resulting in
bogus concatenation of two elements.
2021-08-24 16:10:19 +02:00
Whisperity 21832121e1 [clang-tidy] Fix crash on "reference-to-array" parameters in 'bugprone-easily-swappable-parameters'
An otherwise unexercised code path related to trying to model
"array-to-pointer decay" resulted in a null pointer dereference crash
when parameters of type "reference to array" were encountered.

Fixes crash report http://bugs.llvm.org/show_bug.cgi?id=50995.

Reviewed By: aaron.ballman

Differential Revision: http://reviews.llvm.org/D106946
2021-07-28 14:44:20 +02:00
Whisperity 8b0cc4a65d [clang-tidy] Improve "common type" diagnostic output in 'bugprone-easily-swappable-parameters'
Make the check handle cases of the "common type" involved in the mix
being non-trivial, e.g. pointers, references, attributes, these things
coming from typedefs, etc.

This results in clearer diagnostics that have more coverage in their
explanation, such as saying `const int &` as common type instead of
`int`.

Reviewed By: aaron.ballman

Differential Revision: http://reviews.llvm.org/D106442
2021-07-23 10:26:22 +02:00
Whisperity 473eff1c30 [clang-tidy] Fix crash and handle AttributedType in 'bugprone-easily-swappable-parameters'
@vabridgers identified a way to crash the check by running on code that
involve `AttributedType`s. This patch fixes the check to first and
foremost not crash, but also improves the logic handling qualifiers.

If the types contain any additional (not just CVR) qualifiers that are
not the same, they will not be deemed mixable. The logic for CVR-Mixing
and the `QualifiersMix` check option remain unchanged.

Reviewed By: aaron.ballman, vabridgers

Differential Revision: http://reviews.llvm.org/D106361
2021-07-22 10:20:17 +02:00
David Blaikie 632e15e766 Conditionalize function only used in an assert to address -Wunused-function 2021-06-29 16:39:59 -07:00
Whisperity f3b55a8a06 [clang-tidy][NFC] Fix buildbot failures in 'bugprone-easily-swappable-parameters' 2021-06-28 11:19:16 +02:00
Whisperity 0fba450b97 [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'
While the original check's purpose is to identify potentially dangerous
functions based on the parameter types (as identifier names do not mean
anything when it comes to the language rules), unfortunately, such a plain
interface check rule can be incredibly noisy. While the previous
"filtering heuristic" is able to find many similar usages, there is an entire
class of parameters that should not be warned about very easily mixed by that
check: parameters that have a name and their name follows a pattern,
e.g. `text1, text2, text3, ...`.`

This patch implements a simple, but powerful rule, that allows us to detect
such cases and ensure that no warnings are emitted for parameter sequences that
follow a pattern, even if their types allow for them to be potentially mixed at a call site.

Given a threshold `k`, warnings about two parameters are filtered from the
result set if the names of the parameters are either prefixes or suffixes of
each other, with at most k letters difference on the non-common end.
(Assuming that the names themselves are at least `k` long.)

 - The above `text1, text2` is an example of this. (Live finding from Xerces.)
 - `LHS` and `RHS` are also fitting the bill here. (Live finding from... virtually any project.)
 - So does `Qmat, Tmat, Rmat`. (Live finding from I think OpenCV.)

Reviewed By: aaron.ballman

Differential Revision: http://reviews.llvm.org/D97297
2021-06-28 10:49:37 +02:00
Whisperity b9ece03461 [clang-tidy] Suppress reports to similarly used parameters in 'bugprone-easily-swappable-parameters'
There are several types of functions and various reasons why some
"swappable parameters" cannot be fixed with changing the parameters' types, etc.
The most common example might be int `min(int a, int b)`... no matter what you
do, the two parameters must remain the same type.

The **filtering heuristic** implemented in this patch deals with trying to find
such functions during the modelling and building of the swappable parameter
range.
If the parameter currently scrutinised matches either of the predicates below,
it will be regarded as **not swappable** even if the type of the parameter
matches.

Reviewed By: aaron.ballman

Differential Revision: http://reviews.llvm.org/D78652
2021-06-28 10:49:37 +02:00
Whisperity e33d047883 [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions
Adds a relaxation option ModelImplicitConversions which will make the check
report for cases where parameters refer to types that are implicitly
convertible to one another.

Example:

    struct IntBox { IntBox(int); operator int(); };
    void foo(int i, double d, IntBox ib) {}

Implicit conversions are the last to model in the set of things that are
reasons for the possibility of a function being called the wrong way which is
not always immediately apparent when looking at the function (signature or
call).

Reviewed By: aaron.ballman, martong

Differential Revision: http://reviews.llvm.org/D75041
2021-06-28 10:49:37 +02:00
Whisperity 961e9e6af6 [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with optionally considering differently qualified types mixable
Adds a relaxation option QualifiersMix which will make the check report for
cases where parameters refer to the same type if they only differ in qualifiers.

This makes cases, such as the following, not warned about by default, produce
a warning.

    void* memcpy(void* dst, const void* src, unsigned size) {}

However, unless people meticulously const their local variables, unfortunately,
even such a function carry a potential swap:

    T* obj = new T; // Not const!!!
    void* buf = malloc(sizeof(T));

    memcpy(obj, buf, sizeof(T));
    //     ^~~  ^~~ accidental swap here, even though the interface "specified" a const.

Reviewed By: aaron.ballman

Differential Revision: http://reviews.llvm.org/D96355
2021-06-28 10:49:37 +02:00
Whisperity 26d864b44b [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics
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:

 * Proper typedef diagnostics and explanations to the user.
 * "Reference bind power" matching.

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.

Reviewed By: aaron.ballman

Differential Revision: http://reviews.llvm.org/D95736
2021-06-28 10:49:37 +02:00
Whisperity 499e39c598 [clang-tidy] Add 'bugprone-easily-swappable-parameters' check
Finds function definitions where parameters of convertible types follow
each other directly, making call sites prone to calling the function
with swapped (or badly ordered) arguments.

Such constructs are usually the result of inefficient design and lack of
exploitation of strong type capabilities that are possible in the
language.

This check finds and flags **function definitions** and **not** call
sites!

Reviewed By: aaron.ballman, alexfh

Differential Revision: http://reviews.llvm.org/D69560
2021-06-28 10:49:37 +02:00