This revision was automatically updated to reflect the committed changes.
Closed by commit rG499e39c5983d: [clang-tidy] Add
'bugprone-easily-swappable-parameters' check (authored by whisperity).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69560/new/
https://reviews.llvm.org/D69560
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -0,0 +1,148 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"} \
+// RUN: ]}' -- -x c
+
+#define bool _Bool
+#define true 1
+#define false 0
+
+typedef bool MyBool;
+
+#define TheLogicalType bool
+
+void declVoid(void); // NO-WARN: Declaration only.
+void decl(); // NO-WARN: Declaration only.
+void oneParam(int I) {} // NO-WARN: 1 parameter.
+void variadic(int I, ...) {} // NO-WARN: 1 visible parameter.
+
+void trivial(int I, int J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 2 adjacent parameters of 'trivial' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:18: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:25: note: the last parameter in the range is 'J'
+
+void qualifier(int I, const int CI) {} // NO-WARN: Distinct types.
+
+void restrictQualifier(char *restrict CPR1, char *restrict CPR2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 2 adjacent parameters of 'restrictQualifier' of similar type ('char *restrict')
+// CHECK-MESSAGES: :[[@LINE-2]]:39: note: the first parameter in the range is 'CPR1'
+// CHECK-MESSAGES: :[[@LINE-3]]:60: note: the last parameter in the range is 'CPR2'
+
+void pointer1(int *IP1, int *IP2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 2 adjacent parameters of 'pointer1' of similar type ('int *')
+// CHECK-MESSAGES: :[[@LINE-2]]:20: note: the first parameter in the range is 'IP1'
+// CHECK-MESSAGES: :[[@LINE-3]]:30: note: the last parameter in the range is 'IP2'
+
+void pointerConversion(int *IP, long *LP) {}
+// NO-WARN: Even though C can convert any T* to U* back and forth, compiler
+// warnings already exist for this.
+
+void testVariadicsCall() {
+ int IVal = 1;
+ decl(IVal); // NO-WARN: Particular calls to "variadics" are like template
+ // instantiations, and we do not model them.
+
+ variadic(IVal); // NO-WARN.
+ variadic(IVal, 2, 3, 4); // NO-WARN.
+}
+
+struct S {};
+struct T {};
+
+void taggedTypes1(struct S SVar, struct T TVar) {} // NO-WARN: Distinct types.
+
+void taggedTypes2(struct S SVar1, struct S SVar2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 2 adjacent parameters of 'taggedTypes2' of similar type ('struct S')
+// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'SVar1'
+// CHECK-MESSAGES: :[[@LINE-3]]:44: note: the last parameter in the range is 'SVar2'
+
+void wrappers(struct { int I; } I1, struct { int I; } I2) {} // NO-WARN: Distinct anonymous types.
+
+void knr(I, J)
+ int I;
+ int J;
+{}
+// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: 2 adjacent parameters of 'knr' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-4]]:7: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-4]]:7: note: the last parameter in the range is 'J'
+
+void boolAsWritten(bool B1, bool B2) {} // NO-WARN: The type name is ignored.
+// Note that "bool" is a macro that expands to "_Bool" internally, but it is
+// only "bool" that is ignored from the two.
+
+void underscoreBoolAsWritten(_Bool B1, _Bool B2) {}
+// Even though it is "_Bool" that is written in the code, the diagnostic message
+// respects the printing policy as defined by the compilation commands. Clang's
+// default in C mode seems to say that the type itself is "bool", not "_Bool".
+// CHECK-MESSAGES: :[[@LINE-4]]:30: warning: 2 adjacent parameters of 'underscoreBoolAsWritten' of similar type ('bool')
+// CHECK-MESSAGES: :[[@LINE-5]]:36: note: the first parameter in the range is 'B1'
+// CHECK-MESSAGES: :[[@LINE-6]]:46: note: the last parameter in the range is 'B2'
+
+void typedefdBoolAsWritten(MyBool MB1, MyBool MB2) {} // NO-WARN: "MyBool" as written type name ignored.
+
+void otherBoolMacroAsWritten(TheLogicalType TLT1, TheLogicalType TLT2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: 2 adjacent parameters of 'otherBoolMacroAsWritten' of similar type ('bool')
+// CHECK-MESSAGES: :[[@LINE-2]]:45: note: the first parameter in the range is 'TLT1'
+// CHECK-MESSAGES: :[[@LINE-3]]:66: note: the last parameter in the range is 'TLT2'
+
+struct U {};
+typedef struct U U;
+
+void typedefStruct(U X, U Y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 2 adjacent parameters of 'typedefStruct' of similar type ('U')
+// CHECK-MESSAGES: :[[@LINE-2]]:22: note: the first parameter in the range is 'X'
+// CHECK-MESSAGES: :[[@LINE-3]]:27: note: the last parameter in the range is 'Y'
+
+void ignoredStructU(struct U X, struct U Y) {} // NO-WARN: "struct U" ignored.
+
+#define TYPE_TAG_TO_USE struct // We are in C!
+#define MAKE_TYPE_NAME(T) TYPE_TAG_TO_USE T
+
+void macroMagic1(TYPE_TAG_TO_USE T X, TYPE_TAG_TO_USE T Y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 2 adjacent parameters of 'macroMagic1' of similar type ('struct T')
+// CHECK-MESSAGES: :[[@LINE-5]]:25: note: expanded from macro 'TYPE_TAG_TO_USE'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the first parameter in the range is 'X'
+// CHECK-MESSAGES: :[[@LINE-4]]:57: note: the last parameter in the range is 'Y'
+
+void macroMagic2(TYPE_TAG_TO_USE U X, TYPE_TAG_TO_USE U Y) {}
+// "struct U" is ignored, but that is not what is written here!
+// CHECK-MESSAGES: :[[@LINE-2]]:18: warning: 2 adjacent parameters of 'macroMagic2' of similar type ('struct U')
+// CHECK-MESSAGES: :[[@LINE-12]]:25: note: expanded from macro 'TYPE_TAG_TO_USE'
+// CHECK-MESSAGES: :[[@LINE-4]]:36: note: the first parameter in the range is 'X'
+// CHECK-MESSAGES: :[[@LINE-5]]:57: note: the last parameter in the range is 'Y'
+
+void evenMoreMacroMagic1(MAKE_TYPE_NAME(T) X, MAKE_TYPE_NAME(T) Y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 2 adjacent parameters of 'evenMoreMacroMagic1' of similar type ('struct T')
+// CHECK-MESSAGES: :[[@LINE-17]]:27: note: expanded from macro 'MAKE_TYPE_NAME'
+// CHECK-MESSAGES: :[[@LINE-19]]:25: note: expanded from macro 'TYPE_TAG_TO_USE'
+// CHECK-MESSAGES: :[[@LINE-4]]:44: note: the first parameter in the range is 'X'
+// CHECK-MESSAGES: :[[@LINE-5]]:65: note: the last parameter in the range is 'Y'
+
+void evenMoreMacroMagic2(MAKE_TYPE_NAME(U) X, MAKE_TYPE_NAME(U) Y) {}
+// "struct U" is ignored, but that is not what is written here!
+// CHECK-MESSAGES: :[[@LINE-2]]:26: warning: 2 adjacent parameters of 'evenMoreMacroMagic2' of similar type ('struct U')
+// CHECK-MESSAGES: :[[@LINE-25]]:27: note: expanded from macro 'MAKE_TYPE_NAME'
+// CHECK-MESSAGES: :[[@LINE-27]]:25: note: expanded from macro 'TYPE_TAG_TO_USE'
+// CHECK-MESSAGES: :[[@LINE-5]]:44: note: the first parameter in the range is 'X'
+// CHECK-MESSAGES: :[[@LINE-6]]:65: note: the last parameter in the range is 'Y'
+
+#define MAKE_PRIMITIVE_WRAPPER(WRAPPED_TYPE) \
+ MAKE_TYPE_NAME() { \
+ WRAPPED_TYPE Member; \
+ }
+
+void thisIsGettingRidiculous(MAKE_PRIMITIVE_WRAPPER(int) I1,
+ MAKE_PRIMITIVE_WRAPPER(int) I2) {} // NO-WARN: Distinct anonymous types.
+
+#define MAKE_LOGICAL_TYPE(X) bool
+
+void macroMagic3(MAKE_LOGICAL_TYPE(char) B1, MAKE_LOGICAL_TYPE(long) B2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 2 adjacent parameters of 'macroMagic3' of similar type ('bool')
+// CHECK-MESSAGES: :[[@LINE-4]]:30: note: expanded from macro 'MAKE_LOGICAL_TYPE'
+// CHECK-MESSAGES: :[[@LINE-136]]:14: note: expanded from macro 'bool'
+// CHECK-MESSAGES: :[[@LINE-4]]:42: note: the first parameter in the range is 'B1'
+// CHECK-MESSAGES: :[[@LINE-5]]:70: note: the last parameter in the range is 'B2'
+
+void macroMagic4(MAKE_LOGICAL_TYPE(int) B1, MAKE_LOGICAL_TYPE(int) B2) {} // NO-WARN: "Type name" ignored.
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 3}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""} \
+// RUN: ]}' --
+
+int add(int Left, int Right) { return Left + Right; } // NO-WARN: Only 2 parameters.
+
+int magic(int Left, int Right, int X, int Y) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 4 adjacent parameters of 'magic' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:15: note: the first parameter in the range is 'Left'
+// CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'Y'
+
+void multipleDistinctTypes(int I, int J, int K,
+ long L, long M,
+ double D, double E, double F) {}
+// CHECK-MESSAGES: :[[@LINE-3]]:28: warning: 3 adjacent parameters of 'multipleDistinctTypes' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-4]]:32: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-5]]:46: note: the last parameter in the range is 'K'
+// NO-WARN: The [long, long] range is length of 2.
+// CHECK-MESSAGES: :[[@LINE-5]]:28: warning: 3 adjacent parameters of 'multipleDistinctTypes' of similar type ('double')
+// CHECK-MESSAGES: :[[@LINE-6]]:35: note: the first parameter in the range is 'D'
+// CHECK-MESSAGES: :[[@LINE-7]]:55: note: the last parameter in the range is 'F'
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
@@ -0,0 +1,188 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""} \
+// RUN: ]}' --
+
+namespace std {
+using size_t = decltype(sizeof(int));
+} // namespace std
+
+#define assert(X) ((void)(X))
+
+void declaration(int Param, int Other); // NO-WARN: No chance to change this function.
+
+struct S {};
+
+S *allocate() { return nullptr; } // NO-WARN: 0 parameters.
+void allocate(S **Out) {} // NO-WARN: 1 parameter.
+bool operator<(const S &LHS, const S &RHS) { return true; } // NO-WARN: Binary operator.
+
+struct MyComparator {
+ bool operator()(const S &LHS, const S &RHS) { return true; } // NO-WARN: Binary operator.
+};
+
+struct MyFactory {
+ S operator()() { return {}; } // NO-WARN: 0 parameters, overloaded operator.
+ S operator()(int I) { return {}; } // NO-WARN: 1 parameter, overloaded operator.
+ S operator()(int I, int J) { return {}; } // NO-WARN: Binary operator.
+
+ S operator()(int I, int J, int K) { return {}; }
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 3 adjacent parameters of 'operator()' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+ // CHECK-MESSAGES: :[[@LINE-2]]:20: note: the first parameter in the range is 'I'
+ // CHECK-MESSAGES: :[[@LINE-3]]:34: note: the last parameter in the range is 'K'
+};
+
+// Variadic functions are not checked because the types are not seen from the
+// *definition*. It would require analysing the call sites to do something
+// for these.
+int printf(const char *Format, ...) { return 0; } // NO-WARN: Variadic function not checked.
+int sum(...) { return 0; } // NO-WARN: Variadic function not checked.
+
+void *operator new(std::size_t Count, S &Manager, S &Janitor) noexcept { return nullptr; }
+// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: 2 adjacent parameters of 'operator new' of similar type ('S &')
+// CHECK-MESSAGES: :[[@LINE-2]]:42: note: the first parameter in the range is 'Manager'
+// CHECK-MESSAGES: :[[@LINE-3]]:54: note: the last parameter in the range is 'Janitor'
+
+void redeclChain(int, int, int);
+void redeclChain(int I, int, int);
+void redeclChain(int, int J, int);
+void redeclChain(int I, int J, int K) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 3 adjacent parameters of 'redeclChain' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:22: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'K'
+
+void copyMany(S *Src, S *Dst, unsigned Num) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 2 adjacent parameters of 'copyMany' of similar type ('S *')
+// CHECK-MESSAGES: :[[@LINE-2]]:18: note: the first parameter in the range is 'Src'
+// CHECK-MESSAGES: :[[@LINE-3]]:26: note: the last parameter in the range is 'Dst'
+
+template <typename T, typename U>
+bool binaryPredicate(T L, U R) { return false; } // NO-WARN: Distinct types in template.
+
+template <> // Explicit specialisation.
+bool binaryPredicate(S *L, S *R) { return true; }
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'binaryPredicate<S *, S *>' of similar type ('S *')
+// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'L'
+// CHECK-MESSAGES: :[[@LINE-3]]:31: note: the last parameter in the range is 'R'
+
+template <typename T>
+T algebraicOperation(T L, T R) { return L; }
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'algebraicOperation' of similar type ('T')
+// CHECK-MESSAGES: :[[@LINE-2]]:24: note: the first parameter in the range is 'L'
+// CHECK-MESSAGES: :[[@LINE-3]]:29: note: the last parameter in the range is 'R'
+
+void applyBinaryToS(S SInstance) { // NO-WARN: 1 parameter.
+ assert(binaryPredicate(SInstance, SInstance) !=
+ binaryPredicate(&SInstance, &SInstance));
+ // NO-WARN: binaryPredicate(S, S) is instantiated, but it's not written
+ // by the user.
+}
+
+void unnamedParameter(int I, int, int K, int) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 4 adjacent parameters of 'unnamedParameter' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:27: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:45: note: the last parameter in the range is '<unnamed>'
+
+void fullyUnnamed(int, int) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 2 adjacent parameters of 'fullyUnnamed' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:22: note: the first parameter in the range is '<unnamed>'
+// CHECK-MESSAGES: :[[@LINE-3]]:27: note: the last parameter in the range is '<unnamed>'
+
+void multipleDistinctTypes(int I, int J, long L, long M) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: 2 adjacent parameters of 'multipleDistinctTypes' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:42: warning: 2 adjacent parameters of 'multipleDistinctTypes' of similar type ('long')
+// CHECK-MESSAGES: :[[@LINE-5]]:47: note: the first parameter in the range is 'L'
+// CHECK-MESSAGES: :[[@LINE-6]]:55: note: the last parameter in the range is 'M'
+
+void variableAndPtr(int I, int *IP) {} // NO-WARN: Not the same type.
+
+void differentPtrs(int *IP, long *LP) {} // NO-WARN: Not the same type.
+
+typedef int MyInt1;
+using MyInt2 = int;
+
+void typedefAndTypedef1(MyInt1 I1, MyInt1 I2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 2 adjacent parameters of 'typedefAndTypedef1' of similar type ('MyInt1')
+// CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
+
+void typedefAndTypedef2(MyInt2 I1, MyInt2 I2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 2 adjacent parameters of 'typedefAndTypedef2' of similar type ('MyInt2')
+// CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
+
+void throughTypedef(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+
+void betweenTypedef(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'betweenTypedef' of similar type ('MyInt1')
+// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:38: note: the last parameter in the range is 'J'
+
+typedef long MyLong1;
+using MyLong2 = long;
+
+void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: Not the same type.
+
+void qualified1(int I, const int CI) {} // NO-WARN: Not the same type.
+
+void qualified2(int I, volatile int VI) {} // NO-WARN: Not the same type.
+
+void qualified3(int *IP, const int *CIP) {} // NO-WARN: Not the same type.
+
+void qualified4(const int CI, const long CL) {} // NO-WARN: Not the same type.
+
+using CInt = const int;
+
+void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Not the same type.
+
+void qualifiedThroughTypedef2(CInt CI1, const int CI2) {} // NO-WARN: Not the same type.
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type ('CInt')
+// CHECK-MESSAGES: :[[@LINE-2]]:36: note: the first parameter in the range is 'CI1'
+// CHECK-MESSAGES: :[[@LINE-3]]:51: note: the last parameter in the range is 'CI2'
+
+void reference1(int I, int &IR) {} // NO-WARN: Not the same type.
+
+void reference2(int I, const int &CIR) {} // NO-WARN: Not the same type.
+
+void reference3(int I, int &&IRR) {} // NO-WARN: Not the same type.
+
+void reference4(int I, const int &&CIRR) {} // NO-WARN: Not the same type.
+
+template <typename T1, typename T2>
+struct Pair {};
+
+void templateParam1(Pair<int, int> P1, Pair<int, int> P2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'templateParam1' of similar type ('Pair<int, int>')
+// CHECK-MESSAGES: :[[@LINE-2]]:36: note: the first parameter in the range is 'P1'
+// CHECK-MESSAGES: :[[@LINE-3]]:55: note: the last parameter in the range is 'P2'
+
+void templateParam2(Pair<int, long> P1, Pair<int, long> P2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'templateParam2' of similar type ('Pair<int, long>')
+// CHECK-MESSAGES: :[[@LINE-2]]:37: note: the first parameter in the range is 'P1'
+// CHECK-MESSAGES: :[[@LINE-3]]:57: note: the last parameter in the range is 'P2'
+
+void templateParam3(Pair<int, int> P1, Pair<int, long> P2) {} // NO-WARN: Not the same type.
+
+template <typename X, typename Y>
+struct Coord {};
+
+void templateAndOtherTemplate1(Pair<int, int> P, Coord<int, int> C) {} // NO-WARN: Not the same type.
+
+template <typename Ts>
+void templateVariadic1(Ts TVars...) {} // NO-WARN: Requires instantiation to check.
+
+template <typename T, typename... Us>
+void templateVariadic2(T TVar, Us... UVars) {} // NO-WARN: Distinct types in primary template.
+
+template <>
+void templateVariadic2(int TVar, int UVars1, int UVars2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 3 adjacent parameters of 'templateVariadic2<int, int, int>' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'TVar'
+// CHECK-MESSAGES: :[[@LINE-3]]:50: note: the last parameter in the range is 'UVars2'
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: "\"\";Foo;Bar"}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "T"} \
+// RUN: ]}' --
+
+void ignoredUnnamed(int I, int, int) {} // NO-WARN: No >= 2 length of non-unnamed.
+
+void nothingIgnored(int I, int J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'nothingIgnored' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:32: note: the last parameter in the range is 'J'
+
+void ignoredParameter(int Foo, int I, int J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: 2 adjacent parameters of 'ignoredParameter' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:36: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'J'
+
+void ignoredParameterBoth(int Foo, int Bar) {} // NO-WARN.
+
+struct S {};
+struct T {};
+struct MyT {};
+
+void notIgnoredType(S S1, S S2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'notIgnoredType' of similar type ('S')
+// CHECK-MESSAGES: :[[@LINE-2]]:23: note: the first parameter in the range is 'S1'
+// CHECK-MESSAGES: :[[@LINE-3]]:29: note: the last parameter in the range is 'S2'
+
+void ignoredTypeExact(T T1, T T2) {} // NO-WARN.
+
+void ignoredTypeSuffix(MyT M1, MyT M2) {} // NO-WARN.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -59,6 +59,7 @@
`bugprone-copy-constructor-init <bugprone-copy-constructor-init.html>`_, "Yes"
`bugprone-dangling-handle <bugprone-dangling-handle.html>`_,
`bugprone-dynamic-static-initializers <bugprone-dynamic-static-initializers.html>`_,
+ `bugprone-easily-swappable-parameters <bugprone-easily-swappable-parameters.html>`_,
`bugprone-exception-escape <bugprone-exception-escape.html>`_,
`bugprone-fold-init-type <bugprone-fold-init-type.html>`_,
`bugprone-forward-declaration-namespace <bugprone-forward-declaration-namespace.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
@@ -0,0 +1,113 @@
+.. title:: clang-tidy - bugprone-easily-swappable-parameters
+
+bugprone-easily-swappable-parameters
+====================================
+
+Finds function definitions where parameters of convertible types follow each
+other directly, making call sites prone to calling the function with
+swapped (or badly ordered) arguments.
+
+.. code-block:: c++
+
+ void drawPoint(int X, int Y) { /* ... */ }
+ FILE *open(const char *Dir, const char *Name, Flags Mode) { /* ... */ }
+
+A potential call like ``drawPoint(-2, 5)`` or ``openPath("a.txt", "tmp", Read)``
+is perfectly legal from the language's perspective, but might not be what the
+developer of the function intended.
+
+More elaborate and type-safe constructs, such as opaque typedefs or strong
+types should be used instead, to prevent a mistaken order of arguments.
+
+.. code-block:: c++
+
+ struct Coord2D { int X; int Y; };
+ void drawPoint(const Coord2D Pos) { /* ... */ }
+
+ FILE *open(const Path &Dir, const Filename &Name, Flags Mode) { /* ... */ }
+
+Due to the potentially elaborate refactoring and API-breaking that is necessary
+to strengthen the type safety of a project, no automatic fix-its are offered.
+
+Options
+-------
+
+Filtering options
+^^^^^^^^^^^^^^^^^
+
+Filtering options can be used to lessen the size of the diagnostics emitted by
+the checker, whether the aim is to ignore certain constructs or dampen the
+noisiness.
+
+.. option:: MinimumLength
+
+ The minimum length required from an adjacent parameter sequence to be
+ diagnosed.
+ Defaults to `2`.
+ Might be any positive integer greater or equal to `2`.
+ If `0` or `1` is given, the default value `2` will be used instead.
+
+ For example, if `3` is specified, the examples above will not be matched.
+
+.. option:: IgnoredParameterNames
+
+ The list of parameter **names** that should never be considered part of a
+ swappable adjacent parameter sequence.
+ The value is a `;`-separated list of names.
+ To ignore unnamed parameters, add `""` to the list verbatim (not the
+ empty string, but the two quotes, potentially escaped!).
+ **This options is case-sensitive!**
+
+ By default, the following parameter names, and their Uppercase-initial
+ variants are ignored:
+ `""` (unnamed parameters), `iterator`, `begin`, `end`, `first`, `last`,
+ `lhs`, `rhs`.
+
+.. option:: IgnoredParameterTypeSuffixes
+
+ The list of parameter **type name suffixes** that should never be
+ considered part of a swappable adjacent parameter sequence.
+ Parameters which type, as written in the source code, end with an element
+ of this option will be ignored.
+ The value is a `;`-separated list of names.
+ **This option is case-sensitive!**
+
+ By default, the following, and their lowercase-initial variants are ignored:
+ `bool`, `It`, `Iterator`, `InputIt`, `ForwardIt`, `BidirIt`, `RandomIt`,
+ `random_iterator`, `ReverseIt`, `reverse_iterator`,
+ `reverse_const_iterator`, `RandomIt`, `random_iterator`, `ReverseIt`,
+ `reverse_iterator`, `reverse_const_iterator`, `Const_Iterator`,
+ `ConstIterator`, `const_reverse_iterator`, `ConstReverseIterator`.
+ In addition, `_Bool` (but not `_bool`) is also part of the default value.
+
+
+Limitations
+-----------
+
+**This check is designed to check function signatures!**
+
+The check does not investigate functions that are generated by the compiler
+in a context that is only determined from a call site.
+These cases include variadic functions, functions in C code that do not have
+an argument list, and C++ template instantiations.
+Most of these cases, which are otherwise swappable from a caller's standpoint,
+have no way of getting "fixed" at the definition point.
+In the case of C++ templates, only primary template definitions and explicit
+specialisations are matched and analysed.
+
+None of the following cases produce a diagnostic:
+
+.. code-block:: c++
+
+ int printf(const char *Format, ...) { /* ... */ }
+ int someOldCFunction() { /* ... */ }
+
+ template <typename T, typename U>
+ int add(T X, U Y) { return X + Y };
+
+ void TheseAreNotWarnedAbout() {
+ printf("%d %d\n", 1, 2); // Two ints passed, they could be swapped.
+ someOldCFunction(1, 2, 3); // Similarly, multiple ints passed.
+
+ add(1, 2); // Instantiates 'add<int, int>', but that's not a user-defined function.
+ }
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,13 @@
Finds inner loops that have not been unrolled, as well as fully unrolled
loops with unknown loops bounds or a large number of iterations.
+- New :doc:`bugprone-easily-swappable-parameters
+ <clang-tidy/checks/bugprone-easily-swappable-parameters>` check.
+
+ Finds function definitions where parameters of convertible types follow each
+ other directly, making call sites prone to calling the function with
+ swapped (or badly ordered) arguments.
+
- New :doc:`cppcoreguidelines-prefer-member-initializer
<clang-tidy/checks/cppcoreguidelines-prefer-member-initializer>` check.
Index: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
@@ -0,0 +1,47 @@
+//===--- EasilySwappableParametersCheck.h - clang-tidy ----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EASILYSWAPPABLEPARAMETERSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EASILYSWAPPABLEPARAMETERSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds function definitions where parameters of convertible types follow
+/// each other directly, making call sites prone to calling the function with
+/// swapped (or badly ordered) arguments.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-easily-swappable-parameters.html
+class EasilySwappableParametersCheck : public ClangTidyCheck {
+public:
+ EasilySwappableParametersCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+ /// The minimum length of an adjacent swappable parameter range required for
+ /// a diagnostic.
+ const std::size_t MinimumLength;
+
+ /// The parameter names (as written in the source text) to be ignored.
+ const std::vector<std::string> IgnoredParameterNames;
+
+ /// The parameter typename suffixes (as written in the source code) to be
+ /// ignored.
+ const std::vector<std::string> IgnoredParameterTypeSuffixes;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EASILYSWAPPABLEPARAMETERSCHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
@@ -0,0 +1,495 @@
+//===--- EasilySwappableParametersCheck.cpp - clang-tidy ------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "EasilySwappableParametersCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+#define DEBUG_TYPE "EasilySwappableParametersCheck"
+#include "llvm/Support/Debug.h"
+
+namespace optutils = clang::tidy::utils::options;
+
+/// The default value for the MinimumLength check option.
+static constexpr std::size_t DefaultMinimumLength = 2;
+
+/// The default value for ignored parameter names.
+static const std::string DefaultIgnoredParameterNames =
+ optutils::serializeStringList({"\"\"", "iterator", "Iterator", "begin",
+ "Begin", "end", "End", "first", "First",
+ "last", "Last", "lhs", "LHS", "rhs", "RHS"});
+
+/// The default value for ignored parameter type suffixes.
+static const std::string DefaultIgnoredParameterTypeSuffixes =
+ optutils::serializeStringList({"bool",
+ "Bool",
+ "_Bool",
+ "it",
+ "It",
+ "iterator",
+ "Iterator",
+ "inputit",
+ "InputIt",
+ "forwardit",
+ "FowardIt",
+ "bidirit",
+ "BidirIt",
+ "constiterator",
+ "const_iterator",
+ "Const_Iterator",
+ "Constiterator",
+ "ConstIterator",
+ "RandomIt",
+ "randomit",
+ "random_iterator",
+ "ReverseIt",
+ "reverse_iterator",
+ "reverse_const_iterator",
+ "ConstReverseIterator",
+ "Const_Reverse_Iterator",
+ "const_reverse_iterator"
+ "Constreverseiterator",
+ "constreverseiterator"});
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+using TheCheck = EasilySwappableParametersCheck;
+
+namespace filter {
+static bool isIgnoredParameter(const TheCheck &Check, const ParmVarDecl *Node);
+} // namespace filter
+
+namespace model {
+
+/// The language features involved in allowing the mix between two parameters.
+enum class MixFlags : unsigned char {
+ Invalid = 0, //< Sentinel bit pattern. DO NOT USE!
+
+ None = 1, //< Mix between the two parameters is not possible.
+ Trivial = 2, //< The two mix trivially, and are the exact same type.
+ Canonical = 4, //< The two mix because the types refer to the same
+ // CanonicalType, but we do not elaborate as to how.
+
+ LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue =*/Canonical)
+};
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+
+/// Returns whether the SearchedFlag is turned on in the Data.
+static inline bool hasFlag(MixFlags Data, MixFlags SearchedFlag) {
+ assert(SearchedFlag != MixFlags::Invalid &&
+ "can't be used to detect lack of all bits!");
+
+ // "Data & SearchedFlag" would need static_cast<bool>() in conditions.
+ return (Data & SearchedFlag) == SearchedFlag;
+}
+
+#ifndef NDEBUG
+
+// The modelling logic of this check is more complex than usual, and
+// potentially hard to understand without the ability to see into the
+// representation during the recursive descent. This debug code is only
+// compiled in 'Debug' mode, or if LLVM_ENABLE_ASSERTIONS config is turned on.
+
+/// Formats the MixFlags enum into a useful, user-readable representation.
+static inline std::string formatMixFlags(MixFlags F) {
+ if (F == MixFlags::Invalid)
+ return "#Inv!";
+
+ SmallString<4> Str{"---"};
+
+ if (hasFlag(F, MixFlags::None))
+ // Shows the None bit explicitly, as it can be applied in the recursion
+ // even if other bits are set.
+ Str[0] = '!';
+ if (hasFlag(F, MixFlags::Trivial))
+ Str[1] = 'T';
+ if (hasFlag(F, MixFlags::Canonical))
+ Str[2] = 'C';
+
+ return Str.str().str();
+}
+
+#else
+
+static inline std::string formatMixFlags(MixFlags F);
+
+#endif // NDEBUG
+
+/// Contains the metadata for the mixability result between two types,
+/// independently of which parameters they were calculated from.
+struct MixData {
+ MixFlags Flags;
+
+ MixData(MixFlags Flags) : Flags(Flags) {}
+
+ void sanitize() {
+ assert(Flags != MixFlags::Invalid && "sanitize() called on invalid bitvec");
+ // TODO: There will be statements here in further extensions of the check.
+ }
+};
+
+/// A named tuple that contains the information for a mix between two concrete
+/// parameters.
+struct Mix {
+ const ParmVarDecl *First, *Second;
+ MixData Data;
+
+ Mix(const ParmVarDecl *F, const ParmVarDecl *S, MixData Data)
+ : First(F), Second(S), Data(std::move(Data)) {}
+
+ void sanitize() { Data.sanitize(); }
+ MixFlags flags() const { return Data.Flags; }
+};
+
+// NOLINTNEXTLINE(misc-redundant-expression): Seems to be a bogus warning.
+static_assert(std::is_trivially_copyable<Mix>::value &&
+ std::is_trivially_move_constructible<Mix>::value &&
+ std::is_trivially_move_assignable<Mix>::value,
+ "Keep frequently used data simple!");
+
+struct MixableParameterRange {
+ /// A container for Mixes.
+ using MixVector = SmallVector<Mix, 8>;
+
+ /// The number of parameters iterated to build the instance.
+ std::size_t NumParamsChecked = 0;
+
+ /// The individual flags and supporting information for the mixes.
+ MixVector Mixes;
+
+ /// Gets the leftmost parameter of the range.
+ const ParmVarDecl *getFirstParam() const {
+ // The first element is the LHS of the very first mix in the range.
+ assert(!Mixes.empty());
+ return Mixes.front().First;
+ }
+
+ /// Gets the rightmost parameter of the range.
+ const ParmVarDecl *getLastParam() const {
+ // The builder function breaks building an instance of this type if it
+ // finds something that can not be mixed with the rest, by going *forward*
+ // in the list of parameters. So at any moment of break, the RHS of the last
+ // element of the mix vector is also the last element of the mixing range.
+ assert(!Mixes.empty());
+ return Mixes.back().Second;
+ }
+};
+
+/// Approximate the way how LType and RType might refer to "essentially the
+/// same" type, in a sense that at a particular call site, an expression of
+/// type LType and RType might be successfully passed to a variable (in our
+/// specific case, a parameter) of type RType and LType, respectively.
+/// Note the swapped order!
+///
+/// The returned data structure is not guaranteed to be properly set, as this
+/// function is potentially recursive. It is the caller's responsibility to
+/// call sanitize() on the result once the recursion is over.
+static MixData calculateMixability(const TheCheck &Check, const QualType LType,
+ const QualType RType,
+ const ASTContext &Ctx) {
+ LLVM_DEBUG(llvm::dbgs() << ">>> calculateMixability for LType:\n";
+ LType.dump(llvm::dbgs(), Ctx); llvm::dbgs() << "\nand RType:\n";
+ RType.dump(llvm::dbgs(), Ctx); llvm::dbgs() << '\n';);
+
+ if (LType == RType) {
+ LLVM_DEBUG(llvm::dbgs() << "<<< calculateMixability. Trivial equality.\n");
+ return {MixFlags::Trivial};
+ }
+
+ // TODO: Implement more elaborate logic, such as typedef, implicit
+ // conversions, etc.
+
+ // If none of the previous logic found a match, try if Clang otherwise
+ // believes the types to be the same.
+ if (LType.getCanonicalType() == RType.getCanonicalType()) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "<<< calculateMixability. Same CanonicalType.\n");
+ return {MixFlags::Canonical};
+ }
+
+ LLVM_DEBUG(llvm::dbgs() << "<<< calculateMixability. No match found.\n");
+ return {MixFlags::None};
+}
+
+static MixableParameterRange modelMixingRange(const TheCheck &Check,
+ const FunctionDecl *FD,
+ std::size_t StartIndex) {
+ std::size_t NumParams = FD->getNumParams();
+ assert(StartIndex < NumParams && "out of bounds for start");
+ const ASTContext &Ctx = FD->getASTContext();
+
+ MixableParameterRange Ret;
+ // A parameter at index 'StartIndex' had been trivially "checked".
+ Ret.NumParamsChecked = 1;
+
+ for (std::size_t I = StartIndex + 1; I < NumParams; ++I) {
+ const ParmVarDecl *Ith = FD->getParamDecl(I);
+ LLVM_DEBUG(llvm::dbgs() << "Check param #" << I << "...\n");
+
+ if (filter::isIgnoredParameter(Check, Ith)) {
+ LLVM_DEBUG(llvm::dbgs() << "Param #" << I << " is ignored. Break!\n");
+ break;
+ }
+
+ // Now try to go forward and build the range of [Start, ..., I, I + 1, ...]
+ // parameters that can be messed up at a call site.
+ MixableParameterRange::MixVector MixesOfIth;
+ for (std::size_t J = StartIndex; J < I; ++J) {
+ const ParmVarDecl *Jth = FD->getParamDecl(J);
+ LLVM_DEBUG(llvm::dbgs()
+ << "Check mix of #" << J << " against #" << I << "...\n");
+
+ Mix M{Jth, Ith,
+ calculateMixability(Check, Jth->getType(), Ith->getType(), Ctx)};
+ LLVM_DEBUG(llvm::dbgs() << "Mix flags (raw) : "
+ << formatMixFlags(M.flags()) << '\n');
+ M.sanitize();
+ LLVM_DEBUG(llvm::dbgs() << "Mix flags (after sanitize): "
+ << formatMixFlags(M.flags()) << '\n');
+
+ assert(M.flags() != MixFlags::Invalid && "All flags decayed!");
+
+ if (M.flags() != MixFlags::None)
+ MixesOfIth.emplace_back(std::move(M));
+ }
+
+ if (MixesOfIth.empty()) {
+ // If there weren't any new mixes stored for Ith, the range is
+ // [Start, ..., I].
+ LLVM_DEBUG(llvm::dbgs()
+ << "Param #" << I
+ << " does not mix with any in the current range. Break!\n");
+ break;
+ }
+
+ Ret.Mixes.insert(Ret.Mixes.end(), MixesOfIth.begin(), MixesOfIth.end());
+ ++Ret.NumParamsChecked; // Otherwise a new param was iterated.
+ }
+
+ return Ret;
+}
+
+} // namespace model
+
+namespace filter {
+
+/// Returns whether the parameter's name or the parameter's type's name is
+/// configured by the user to be ignored from analysis and diagnostic.
+static bool isIgnoredParameter(const TheCheck &Check, const ParmVarDecl *Node) {
+ LLVM_DEBUG(llvm::dbgs() << "Checking if '" << Node->getName()
+ << "' is ignored.\n");
+
+ if (!Node->getIdentifier())
+ return llvm::find(Check.IgnoredParameterNames, "\"\"") !=
+ Check.IgnoredParameterNames.end();
+
+ StringRef NodeName = Node->getName();
+ if (llvm::find(Check.IgnoredParameterNames, NodeName) !=
+ Check.IgnoredParameterNames.end()) {
+ LLVM_DEBUG(llvm::dbgs() << "\tName ignored.\n");
+ return true;
+ }
+
+ StringRef NodeTypeName = [Node] {
+ const ASTContext &Ctx = Node->getASTContext();
+ const SourceManager &SM = Ctx.getSourceManager();
+ SourceLocation B = Node->getTypeSpecStartLoc();
+ SourceLocation E = Node->getTypeSpecEndLoc();
+ LangOptions LO;
+
+ LLVM_DEBUG(llvm::dbgs() << "\tType name code is '"
+ << Lexer::getSourceText(
+ CharSourceRange::getTokenRange(B, E), SM, LO)
+ << "'...\n");
+ if (B.isMacroID()) {
+ LLVM_DEBUG(llvm::dbgs() << "\t\tBeginning is macro.\n");
+ B = SM.getTopMacroCallerLoc(B);
+ }
+ if (E.isMacroID()) {
+ LLVM_DEBUG(llvm::dbgs() << "\t\tEnding is macro.\n");
+ E = Lexer::getLocForEndOfToken(SM.getTopMacroCallerLoc(E), 0, SM, LO);
+ }
+ LLVM_DEBUG(llvm::dbgs() << "\tType name code is '"
+ << Lexer::getSourceText(
+ CharSourceRange::getTokenRange(B, E), SM, LO)
+ << "'...\n");
+
+ return Lexer::getSourceText(CharSourceRange::getTokenRange(B, E), SM, LO);
+ }();
+
+ LLVM_DEBUG(llvm::dbgs() << "\tType name is '" << NodeTypeName << "'\n");
+ if (!NodeTypeName.empty()) {
+ if (llvm::any_of(Check.IgnoredParameterTypeSuffixes,
+ [NodeTypeName](const std::string &E) {
+ return !E.empty() && NodeTypeName.endswith(E);
+ })) {
+ LLVM_DEBUG(llvm::dbgs() << "\tType suffix ignored.\n");
+ return true;
+ }
+ }
+
+ return false;
+}
+
+} // namespace filter
+
+/// Matches functions that have at least the specified amount of parameters.
+AST_MATCHER_P(FunctionDecl, parameterCountGE, unsigned, N) {
+ return Node.getNumParams() >= N;
+}
+
+/// Matches *any* overloaded unary and binary operators.
+AST_MATCHER(FunctionDecl, isOverloadedUnaryOrBinaryOperator) {
+ switch (Node.getOverloadedOperator()) {
+ case OO_None:
+ case OO_New:
+ case OO_Delete:
+ case OO_Array_New:
+ case OO_Array_Delete:
+ case OO_Conditional:
+ case OO_Coawait:
+ return false;
+
+ default:
+ return Node.getNumParams() <= 2;
+ }
+}
+
+/// Returns the DefaultMinimumLength if the Value of requested minimum length
+/// is less than 2. Minimum lengths of 0 or 1 are not accepted.
+static inline unsigned clampMinimumLength(const unsigned Value) {
+ return Value < 2 ? DefaultMinimumLength : Value;
+}
+
+// FIXME: Maybe unneeded, getNameForDiagnostic() is expected to change to return
+// a crafted location when the node itself is unnamed. (See D84658, D85033.)
+/// Returns the diagnostic-friendly name of the node, or empty string.
+static SmallString<64> getName(const NamedDecl *ND) {
+ SmallString<64> Name;
+ llvm::raw_svector_ostream OS{Name};
+ ND->getNameForDiagnostic(OS, ND->getASTContext().getPrintingPolicy(), false);
+ return Name;
+}
+
+/// Returns the diagnostic-friendly name of the node, or a constant value.
+static SmallString<64> getNameOrUnnamed(const NamedDecl *ND) {
+ auto Name = getName(ND);
+ if (Name.empty())
+ Name = "<unnamed>";
+ return Name;
+}
+
+EasilySwappableParametersCheck::EasilySwappableParametersCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ MinimumLength(clampMinimumLength(
+ Options.get("MinimumLength", DefaultMinimumLength))),
+ IgnoredParameterNames(optutils::parseStringList(
+ Options.get("IgnoredParameterNames", DefaultIgnoredParameterNames))),
+ IgnoredParameterTypeSuffixes(optutils::parseStringList(
+ Options.get("IgnoredParameterTypeSuffixes",
+ DefaultIgnoredParameterTypeSuffixes))) {}
+
+void EasilySwappableParametersCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "MinimumLength", MinimumLength);
+ Options.store(Opts, "IgnoredParameterNames",
+ optutils::serializeStringList(IgnoredParameterNames));
+ Options.store(Opts, "IgnoredParameterTypeSuffixes",
+ optutils::serializeStringList(IgnoredParameterTypeSuffixes));
+}
+
+void EasilySwappableParametersCheck::registerMatchers(MatchFinder *Finder) {
+ const auto BaseConstraints = functionDecl(
+ // Only report for definition nodes, as fixing the issues reported
+ // requires the user to be able to change code.
+ isDefinition(), parameterCountGE(MinimumLength),
+ unless(isOverloadedUnaryOrBinaryOperator()));
+
+ Finder->addMatcher(
+ functionDecl(BaseConstraints,
+ unless(ast_matchers::isTemplateInstantiation()))
+ .bind("func"),
+ this);
+ Finder->addMatcher(
+ functionDecl(BaseConstraints, isExplicitTemplateSpecialization())
+ .bind("func"),
+ this);
+}
+
+void EasilySwappableParametersCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("func");
+ assert(FD);
+
+ const PrintingPolicy &PP = FD->getASTContext().getPrintingPolicy();
+ std::size_t NumParams = FD->getNumParams();
+ std::size_t MixableRangeStartIndex = 0;
+
+ LLVM_DEBUG(llvm::dbgs() << "Begin analysis of " << getName(FD) << " with "
+ << NumParams << " parameters...\n");
+ while (MixableRangeStartIndex < NumParams) {
+ if (filter::isIgnoredParameter(*this,
+ FD->getParamDecl(MixableRangeStartIndex))) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "Parameter #" << MixableRangeStartIndex << " ignored.\n");
+ ++MixableRangeStartIndex;
+ continue;
+ }
+
+ model::MixableParameterRange R =
+ model::modelMixingRange(*this, FD, MixableRangeStartIndex);
+ assert(R.NumParamsChecked > 0 && "Ensure forward progress!");
+ MixableRangeStartIndex += R.NumParamsChecked;
+ if (R.NumParamsChecked < MinimumLength) {
+ LLVM_DEBUG(llvm::dbgs() << "Ignoring range of " << R.NumParamsChecked
+ << " lower than limit.\n");
+ continue;
+ }
+
+ const ParmVarDecl *First = R.getFirstParam(), *Last = R.getLastParam();
+ std::string FirstParamTypeAsWritten = First->getType().getAsString(PP);
+ {
+ StringRef DiagText = "%0 adjacent parameters of %1 of similar type "
+ "('%2') are easily swapped by mistake";
+ // TODO: This logic will get extended here with future flags.
+
+ auto Diag = diag(First->getOuterLocStart(), DiagText)
+ << static_cast<unsigned>(R.NumParamsChecked) << FD
+ << FirstParamTypeAsWritten;
+
+ CharSourceRange HighlightRange = CharSourceRange::getTokenRange(
+ First->getBeginLoc(), Last->getEndLoc());
+ Diag << HighlightRange;
+ }
+
+ // There is a chance that the previous highlight did not succeed, e.g. when
+ // the two parameters are on different lines. For clarity, show the user
+ // the involved variable explicitly.
+ diag(First->getLocation(), "the first parameter in the range is '%0'",
+ DiagnosticIDs::Note)
+ << getNameOrUnnamed(First)
+ << CharSourceRange::getTokenRange(First->getLocation(),
+ First->getLocation());
+ diag(Last->getLocation(), "the last parameter in the range is '%0'",
+ DiagnosticIDs::Note)
+ << getNameOrUnnamed(Last)
+ << CharSourceRange::getTokenRange(Last->getLocation(),
+ Last->getLocation());
+ }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -13,6 +13,7 @@
CopyConstructorInitCheck.cpp
DanglingHandleCheck.cpp
DynamicStaticInitializersCheck.cpp
+ EasilySwappableParametersCheck.cpp
ExceptionEscapeCheck.cpp
FoldInitTypeCheck.cpp
ForwardDeclarationNamespaceCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -18,6 +18,7 @@
#include "CopyConstructorInitCheck.h"
#include "DanglingHandleCheck.h"
#include "DynamicStaticInitializersCheck.h"
+#include "EasilySwappableParametersCheck.h"
#include "ExceptionEscapeCheck.h"
#include "FoldInitTypeCheck.h"
#include "ForwardDeclarationNamespaceCheck.h"
@@ -91,6 +92,8 @@
"bugprone-dangling-handle");
CheckFactories.registerCheck<DynamicStaticInitializersCheck>(
"bugprone-dynamic-static-initializers");
+ CheckFactories.registerCheck<EasilySwappableParametersCheck>(
+ "bugprone-easily-swappable-parameters");
CheckFactories.registerCheck<ExceptionEscapeCheck>(
"bugprone-exception-escape");
CheckFactories.registerCheck<FoldInitTypeCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
