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

[Whisperity via Phabricator via cfe-commits]

whisperity 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();
----------------
aaron.ballman wrote:
> 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.
I looked at all the possible operator tokens and this seemed the most appropriate. It points to the left, where qualifiers are in LLVM's programming style ("west const" and all that). Because sometimes it's variables that get passed to these functions, seeing from the get-go that it's not meddling with the flags but rather with the types involved seemed appropriate to emphasise.


================
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,
----------------
aaron.ballman wrote:
> `isAnyPointerType()` for ObjC pointers? Should we care about rvalue references here similar to pointers?
The reasoning to not be special about && goes back to D95736. If you are given any combination of `T`, `T&`, `T&&` and `const T&` parameters and //some// values of (essentially) `T`, it's only `T` and `const T&` that **always** mix for every potential value of `T`.

`T&&` won't bind variables, and `T&` won't bind temporaries. So their potential to mix is an inherent property of the //call site// where the mix might happen. If one of them is `T` and the other is `T&` and you happen to pass one variable and one literal, and you happen to swap these two, you'll get a compile error.

If both parameters are `T&&`, `LType == RType` for a trivial mix catches it early on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96355