[PATCH] D106361: [clang-tidy] Fix crash and handle AttributedTypes in 'bugprone-easily-swappable-parameters'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The rename from `CommonType` to `CoreType` could probably be done as a separate NFC to make the review a bit easier, if we decide that's a good terminology switch. I'm not certain switching from `common` to `core` makes things more clear though; what's the distinction trying to be made?



================
Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:476-477
+
+    QualType NewCoreType = CoreType;
+    NewCoreType.addFastQualifiers(Quals.getFastQualifiers());
+    NewCoreType.getQualifiers().addQualifiers(Quals);
----------------



================
Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:2201
             DiagText = "after resolving type aliases, '%0' and '%1' share a "
-                       "common type";
+                       "core type";
           else
----------------
I don't know that "core type" is a term of art that will convey much meaning to users in a diagnostic message. Do we need to change the wording of the diagnostic?


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp:362-364
+// FIXME: The last diagnostic line is a bit bad: the core type should be a
+// pointer type -- it is not clear right now, how it would be possible to
+// properly wire a logic in that fixes it.
----------------
Yeah, the diagnostic here is a bit unfortunate. This was an existing deficiency with the check though, right? I assume it also impacts references and not just pointers?


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp:122
+// CHECK-MESSAGES: :[[@LINE-3]]:73: note: the last parameter in the range is 'Two'
+// CHECK-MESSAGES: :[[@LINE-5]]:23: note: after resolving type aliases, the core type of '__attribute__((address_space(256))) int *' and 'const __attribute__((address_space(256))) MyInt1 *' is 'int'
+// CHECK-MESSAGES: :[[@LINE-5]]:23: note: '__attribute__((address_space(256))) int *' and 'const __attribute__((address_space(256))) MyInt1 *' parameters accept and bind the same kind of values
----------------
Should probably have a FIXME comment here as well given the lack of a pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106361