[all-commits] [llvm/llvm-project] 499e39: [clang-tidy] Add 'bugprone-easily-swappable-parame...

Whisperity via All-commits all-commits at lists.llvm.org
Mon Jun 28 01:50:46 PDT 2021


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 499e39c5983dba35861b5482bd298a8da726f1b6
      https://github.com/llvm/llvm-project/commit/499e39c5983dba35861b5482bd298a8da726f1b6
  Author: Whisperity <whisperity at gmail.com>
  Date:   2021-06-28 (Mon, 28 Jun 2021)

  Changed paths:
    M clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
    M clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
    A clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
    A clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
    M clang-tools-extra/docs/ReleaseNotes.rst
    A clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
    M clang-tools-extra/docs/clang-tidy/checks/list.rst
    A clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
    A clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
    A clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
    A clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

  Log Message:
  -----------
  [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


  Commit: 26d864b44b9d3326984a7041124aa0f9e8ebc5cb
      https://github.com/llvm/llvm-project/commit/26d864b44b9d3326984a7041124aa0f9e8ebc5cb
  Author: Whisperity <whisperity at gmail.com>
  Date:   2021-06-28 (Mon, 28 Jun 2021)

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

  Log Message:
  -----------
  [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


  Commit: 961e9e6af65ef097678c57fe5f1c18b825eb723f
      https://github.com/llvm/llvm-project/commit/961e9e6af65ef097678c57fe5f1c18b825eb723f
  Author: Whisperity <whisperity at gmail.com>
  Date:   2021-06-28 (Mon, 28 Jun 2021)

  Changed paths:
    M clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
    M clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
    M clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
    A clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

  Log Message:
  -----------
  [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


  Commit: e33d0478831e4a295cb136ce1f58587155309fa2
      https://github.com/llvm/llvm-project/commit/e33d0478831e4a295cb136ce1f58587155309fa2
  Author: Whisperity <whisperity at gmail.com>
  Date:   2021-06-28 (Mon, 28 Jun 2021)

  Changed paths:
    M clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
    M clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
    M clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
    A clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicit-qualifiers.cpp
    A clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
    A clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

  Log Message:
  -----------
  [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


  Commit: b9ece034611239d008ac11d8bb9b3af91313c41f
      https://github.com/llvm/llvm-project/commit/b9ece034611239d008ac11d8bb9b3af91313c41f
  Author: Whisperity <whisperity at gmail.com>
  Date:   2021-06-28 (Mon, 28 Jun 2021)

  Changed paths:
    M clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
    M clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
    M clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicit-qualifiers.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
    A clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
    A clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

  Log Message:
  -----------
  [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


  Commit: 0fba450b9756a496224efd06e5ba76c9a61d3e15
      https://github.com/llvm/llvm-project/commit/0fba450b9756a496224efd06e5ba76c9a61d3e15
  Author: Whisperity <whisperity at gmail.com>
  Date:   2021-06-28 (Mon, 28 Jun 2021)

  Changed paths:
    M clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
    M clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
    M clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicit-qualifiers.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
    A clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
    M clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

  Log Message:
  -----------
  [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


Compare: https://github.com/llvm/llvm-project/compare/a49855316251...0fba450b9756


More information about the All-commits mailing list