[PATCH] D96355: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with optionally considering differently qualified types mixable

[Whisperity via Phabricator via cfe-commits]

whisperity created this revision.
whisperity added reviewers: aaron.ballman, alexfh, hokein, njames93.
whisperity added projects: clang, clang-tools-extra.
Herald added subscribers: nullptr.cpp, rnkovacs, xazax.hun.
whisperity requested review of this revision.
Herald added a subscriber: cfe-commits.

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) {}

The reasoning behind this is that several projects, code styles, and preferences might consider `T` and `const T` fundamentally different types. The related C++ Core Guidelines <http://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-unrelated> rule itself shows an example where `void T*, const void T*` is considered a "good solution".

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.

Of course, keeping your things `const` where appropriate results in an error:

  const T* obj = new T;
  void* buf = malloc(sizeof(T));
  
  memcpy(obj, buf, sizeof(T));
  // error: 1st argument ('const T*') loses qualifiers

Due to the rationale behind this depending on project-specific guidelines and preferences, the modelling is introduced as a check option. The default value is **`false`**, i.e. `T*` and `const T*` are considered **unmixable, distinct types**.

> 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/D96355

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96355.322432.patch
Type: text/x-patch
Size: 33069 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210209/31cb8f9a/attachment-0001.bin>