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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
