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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:172
+  /// Add the specified qualifiers to the common type in the Mix.
+  MixData operator<<(Qualifiers Quals) const {
+    SplitQualType Split = CommonType.split();
----------------
Hmm, use of `<<` is a bit novel but not entirely indefensible. I guess my initial inclination is that you're combing this information into the mix data and so an overload of `|` was what I was sort of expecting to see here (despite it not really being a bitmask operation). These aren't strong opinions, but I'm curious if you have thoughts.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:281
 
+  // Dissolve typedefs after the qualifiers outside the typedef is dealt with.
+  if (LType->getAs<TypedefType>()) {
----------------



================
Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:327
+
+  if (LType->isPointerType() && RType->isPointerType()) {
+    // If both types are pointers, and pointed to the exact same type,
----------------
`isAnyPointerType()` for ObjC pointers? Should we care about rvalue references here similar to pointers?


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst:39
+Relaxation (or extension) options can be used to broaden the scope of the
+analysis and fine-tune the enabling of more mix between types.
+Some mix may depend on coding style or preference specific to a project,
----------------



================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst:40
+analysis and fine-tune the enabling of more mix between types.
+Some mix may depend on coding style or preference specific to a project,
+however, it should be noted that enabling *all* of these relaxations model the
----------------



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96355/new/

https://reviews.llvm.org/D96355