[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

Gabor Marton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 3 06:29:26 PDT 2021


martong added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:122
 
 #define FB(Name, K) MIX_##Name = (1ull << (K##ull - 1ull))
 
----------------
FB stands for FunnyBitmask? Could you please either describe that in a comment or just spell it out?


================
Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:182
+  /// The intermediate type after the first Standard Conversion Sequence.
+  QualType AfterPreStd;
+
----------------
Maybe it's just me, but AfterPre sounds ambiguous and AfterPost seems redundant.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:210
+  /// the conversion sequence. This method does **NOT** return Begin and End.
+  SmallVector<QualType, 4> getInvolvedTypesInSequence() const {
+    SmallVector<QualType, 4> Ret;
----------------
So, we can never return with a vector with size > 4?


================
Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h:42
 
-  /// Whether to consider an unqualified and a qualified type mixable.
+  /// Whether to consider differently qualified versions of the same type
+  /// mixable.
----------------
"qualified"
Do you consider `volatile` here as well, or just `const`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75041



More information about the cfe-commits mailing list