whisperity created this revision.
whisperity added reviewers: aaron.ballman, alexfh, hokein, njames93.
whisperity added projects: clang, clang-tools-extra.
Herald added subscribers: nullptr.cpp, rnkovacs, xazax.hun.
whisperity requested review of this revision.
Herald added a subscriber: cfe-commits.
Adds a relaxation option **`QualifiersMix`** which will make the check report
for cases where parameters refer to the same type if they only differ in
qualifiers.
This makes cases, such as the following, not warned about by default, produce a
warning.
void* memcpy(void* dst, const void* src, unsigned size) {}
The reasoning behind this is that several projects, code styles, and
preferences might consider `T` and `const T` fundamentally different types. The
related C++ Core Guidelines
<http://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-unrelated>
rule itself shows an example where `void T*, const void T*` is considered a
"good solution".
However, unless people meticulously `const` their local variables,
unfortunately, even such a function carry a potential swap:
T* obj = new T; // Not const!!!
void* buf = malloc(sizeof(T));
memcpy(obj, buf, sizeof(T));
// ^~~ ^~~ accidental swap here, even though the interface "specified" a
const.
Of course, keeping your things `const` where appropriate results in an error:
const T* obj = new T;
void* buf = malloc(sizeof(T));
memcpy(obj, buf, sizeof(T));
// error: 1st argument ('const T*') loses qualifiers
Due to the rationale behind this depending on project-specific guidelines and
preferences, the modelling is introduced as a check option. The default value
is **`false`**, i.e. `T*` and `const T*` are considered **unmixable, distinct
types**.
> Originally, the implementation of this feature was part of the very first
> patch related to this check, D69560 <https://reviews.llvm.org/D69560> (diff
> 259320 <http://reviews.llvm.org/D69560?id=259320>). It has been factored out
> for clarity and to allow more on-point discussion.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D96355
Files:
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.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-qualifiermixing.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
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -2,7 +2,8 @@
// 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: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0} \
// RUN: ]}' -- -x c
#define bool _Bool
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
@@ -0,0 +1,112 @@
+// 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: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 1} \
+// RUN: ]}' --
+
+typedef int MyInt1;
+typedef int MyInt2;
+using CInt = const int;
+using CMyInt1 = const MyInt1;
+using CMyInt2 = const MyInt2;
+
+void qualified1(int I, const int CI) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent parameters of 'qualified1' of similar type are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:21: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:34: note: the last parameter in the range is 'CI'
+// CHECK-MESSAGES: :[[@LINE-4]]:24: note: a call site binds an expression to 'int' and 'const int' with the same force
+
+void qualified2(int I, volatile int VI) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent parameters of 'qualified2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:21: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:37: note: the last parameter in the range is 'VI'
+// CHECK-MESSAGES: :[[@LINE-4]]:24: note: a call site binds an expression to 'int' and 'volatile int'
+
+void qualified3(int I, const volatile int CVI) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent parameters of 'qualified3' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:21: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'CVI'
+// CHECK-MESSAGES: :[[@LINE-4]]:24: note: a call site binds an expression to 'int' and 'const volatile int'
+
+void qualified4(int *IP, const int *CIP) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent parameters of 'qualified4' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:22: note: the first parameter in the range is 'IP'
+// CHECK-MESSAGES: :[[@LINE-3]]:37: note: the last parameter in the range is 'CIP'
+// CHECK-MESSAGES: :[[@LINE-4]]:26: note: a call site binds an expression to 'int *' and 'const int *'
+
+void qualified5(const int CI, const long CL) {} // NO-WARN: Not the same type
+
+void qualifiedPtr1(int *IP, int *const IPC) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 2 adjacent parameters of 'qualifiedPtr1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'IP'
+// CHECK-MESSAGES: :[[@LINE-3]]:40: note: the last parameter in the range is 'IPC'
+// CHECK-MESSAGES: :[[@LINE-4]]:29: note: a call site binds an expression to 'int *' and 'int *const'
+
+void qualifiedPtr2(int *IP, int *volatile IPV) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 2 adjacent parameters of 'qualifiedPtr2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'IP'
+// CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'IPV'
+// CHECK-MESSAGES: :[[@LINE-4]]:29: note: a call site binds an expression to 'int *' and 'int *volatile'
+
+void qualifiedTypeAndQualifiedPtr1(const int *CIP, int *const volatile IPCV) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:36: warning: 2 adjacent parameters of 'qualifiedTypeAndQualifiedPtr1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:47: note: the first parameter in the range is 'CIP'
+// CHECK-MESSAGES: :[[@LINE-3]]:72: note: the last parameter in the range is 'IPCV'
+// CHECK-MESSAGES: :[[@LINE-4]]:52: note: a call site binds an expression to 'const int *' and 'int *const volatile'
+
+void qualifiedThroughTypedef1(int I, CInt CI) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:35: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'CI'
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type aliases, 'int' and 'CInt' share a common type
+// CHECK-MESSAGES: :[[@LINE-5]]:38: note: a call site binds an expression to 'int' and 'CInt' with the same force
+
+void qualifiedThroughTypedef2(CInt CI1, const int CI2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type are
+// 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'
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type aliases, 'CInt' and 'const int' are the same
+
+void qualifiedThroughTypedef3(CInt CI1, const MyInt1 CI2, const int CI3) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 3 adjacent parameters of 'qualifiedThroughTypedef3' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:36: note: the first parameter in the range is 'CI1'
+// CHECK-MESSAGES: :[[@LINE-3]]:69: note: the last parameter in the range is 'CI3'
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type aliases, the common type of 'CInt' and 'const MyInt1' is 'const int'
+// CHECK-MESSAGES: :[[@LINE-5]]:31: note: after resolving type aliases, 'CInt' and 'const int' are the same
+// CHECK-MESSAGES: :[[@LINE-6]]:41: note: after resolving type aliases, 'const MyInt1' and 'const int' are the same
+
+void qualifiedThroughTypedef4(CInt CI1, const MyInt1 CI2, const MyInt2 CI3) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 3 adjacent parameters of 'qualifiedThroughTypedef4' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:36: note: the first parameter in the range is 'CI1'
+// CHECK-MESSAGES: :[[@LINE-3]]:72: note: the last parameter in the range is 'CI3'
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type aliases, the common type of 'CInt' and 'const MyInt1' is 'const int'
+// CHECK-MESSAGES: :[[@LINE-5]]:31: note: after resolving type aliases, the common type of 'CInt' and 'const MyInt2' is 'const int'
+// CHECK-MESSAGES: :[[@LINE-6]]:41: note: after resolving type aliases, the common type of 'const MyInt1' and 'const MyInt2' is 'const int'
+
+void qualifiedThroughTypedef5(CMyInt1 CMI1, CMyInt2 CMI2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef5' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:39: note: the first parameter in the range is 'CMI1'
+// CHECK-MESSAGES: :[[@LINE-3]]:53: note: the last parameter in the range is 'CMI2'
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type aliases, the common type of 'CMyInt1' and 'CMyInt2' is 'const int'
+
+void qualifiedThroughTypedef6(CMyInt1 CMI1, int I) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef6' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:39: note: the first parameter in the range is 'CMI1'
+// CHECK-MESSAGES: :[[@LINE-3]]:49: note: the last parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type aliases, 'CMyInt1' and 'int' share a common type
+// CHECK-MESSAGES: :[[@LINE-5]]:45: note: a call site binds an expression to 'CMyInt1' and 'int' with the same force
+
+void referenceToTypedef1(CInt &CIR, int I) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 2 adjacent parameters of 'referenceToTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'CIR'
+// CHECK-MESSAGES: :[[@LINE-3]]:41: note: the last parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-4]]:37: note: a call site binds an expression to 'CInt &' and 'int'
+
+template <typename T>
+void copy(const T *Dest, T *Source) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 2 adjacent parameters of 'copy' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:20: note: the first parameter in the range is 'Dest'
+// CHECK-MESSAGES: :[[@LINE-3]]:29: note: the last parameter in the range is 'Source'
+// CHECK-MESSAGES: :[[@LINE-4]]:26: note: a call site binds an expression to 'const T *' and 'T *'
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.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-len3.cpp
@@ -2,7 +2,8 @@
// 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: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0} \
// RUN: ]}' --
int add(int Left, int Right) { return Left + Right; } // NO-WARN: Only 2 parameters.
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.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-len2.cpp
@@ -2,7 +2,8 @@
// 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: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0} \
// RUN: ]}' --
#define assert(X)
@@ -75,6 +76,14 @@
typedef int MyInt1;
using MyInt2 = int;
+typedef MyInt2 MyInt2b;
+
+using CInt = const int;
+using CMyInt1 = const MyInt1;
+using CMyInt2 = const MyInt2;
+
+typedef long MyLong1;
+using MyLong2 = long;
void typedefAndTypedef1(MyInt1 I1, MyInt1 I2) {}
// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 2 adjacent parameters of 'typedefAndTypedef1' of similar type ('MyInt1')
@@ -104,8 +113,6 @@
// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
-typedef MyInt2 MyInt2b;
-
void typedefChain(int I, MyInt1 MI1, MyInt2 MI2, MyInt2b MI2b) {}
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 4 adjacent parameters of 'typedefChain' of similar type are
// CHECK-MESSAGES: :[[@LINE-2]]:23: note: the first parameter in the range is 'I'
@@ -114,22 +121,21 @@
// CHECK-MESSAGES: :[[@LINE-5]]:19: note: after resolving type aliases, 'int' and 'MyInt2' are the same
// CHECK-MESSAGES: :[[@LINE-6]]:19: note: after resolving type aliases, 'int' and 'MyInt2b' are the same
-typedef long MyLong1;
-using MyLong2 = long;
-
void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: int and long.
-void qualified1(int I, const int CI) {} // NO-WARN: Not the same type.
+void qualified1(int I, const int CI) {} // NO-WARN: Different qualifiers.
-void qualified2(int I, volatile int VI) {} // NO-WARN: Not the same type.
+void qualified2(int I, volatile int VI) {} // NO-WARN: Different qualifiers.
-void qualified3(int *IP, const int *CIP) {} // NO-WARN: Not the same type.
+void qualified3(int *IP, const int *CIP) {} // NO-WARN: Different qualifiers.
void qualified4(const int CI, const long CL) {} // NO-WARN: Not the same type.
-using CInt = const int;
+void qualifiedPtr1(int *IP, int *const IPC) {} // NO-WARN: Different qualifiers.
+
+void qualifiedTypeAndQualifiedPtr1(const int *CIP, int *const volatile IPCV) {} // NO-WARN: Not the same type.
-void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Not the same type.
+void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Different qualifiers.
void qualifiedThroughTypedef2(CInt CI1, const int CI2) {}
// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type are
@@ -137,13 +143,32 @@
// CHECK-MESSAGES: :[[@LINE-3]]:51: note: the last parameter in the range is 'CI2'
// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type aliases, 'CInt' and 'const int' are the same
-void qualifiedThroughTypedef3(CInt CI1, const MyInt1 CI2, const int CI3) {} // NO-WARN: Not the same type.
+void qualifiedThroughTypedef3(CInt CI1, const MyInt1 CI2, const int CI3) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 3 adjacent parameters of 'qualifiedThroughTypedef3' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:36: note: the first parameter in the range is 'CI1'
+// CHECK-MESSAGES: :[[@LINE-3]]:69: note: the last parameter in the range is 'CI3'
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type aliases, the common type of 'CInt' and 'const MyInt1' is 'const int'
+// CHECK-MESSAGES: :[[@LINE-5]]:31: note: after resolving type aliases, 'CInt' and 'const int' are the same
+// CHECK-MESSAGES: :[[@LINE-6]]:41: note: after resolving type aliases, 'const MyInt1' and 'const int' are the same
void qualifiedThroughTypedef4(CInt CI1, const MyInt1 CI2, const MyInt2 CI3) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:41: warning: 2 adjacent parameters of 'qualifiedThroughTypedef4' of similar type are
-// CHECK-MESSAGES: :[[@LINE-2]]:54: note: the first parameter in the range is 'CI2'
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 3 adjacent parameters of 'qualifiedThroughTypedef4' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:36: note: the first parameter in the range is 'CI1'
// CHECK-MESSAGES: :[[@LINE-3]]:72: note: the last parameter in the range is 'CI3'
-// CHECK-MESSAGES: :[[@LINE-4]]:41: note: after resolving type aliases, the common type of 'const MyInt1' and 'const MyInt2' is 'int'
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type aliases, the common type of 'CInt' and 'const MyInt1' is 'const int'
+// CHECK-MESSAGES: :[[@LINE-5]]:31: note: after resolving type aliases, the common type of 'CInt' and 'const MyInt2' is 'const int'
+// CHECK-MESSAGES: :[[@LINE-6]]:41: note: after resolving type aliases, the common type of 'const MyInt1' and 'const MyInt2' is 'const int'
+
+void qualifiedThroughTypedef5(CMyInt1 CMI1, CMyInt2 CMI2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef5' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:39: note: the first parameter in the range is 'CMI1'
+// CHECK-MESSAGES: :[[@LINE-3]]:53: note: the last parameter in the range is 'CMI2'
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type aliases, the common type of 'CMyInt1' and 'CMyInt2' is 'const int'
+
+void qualifiedThroughTypedef6(CMyInt1 CMI1, int I) {} // NO-WARN: Different qualifiers.
+
+template <typename T>
+void copy(const T *Dest, T *Source) {} // NO-WARN: Different qualifiers.
void reference1(int I, int &IR) {} // NO-WARN: Distinct semantics when called.
@@ -172,16 +197,21 @@
using ICRTy = const int &;
using MyIntCRTy = const MyInt1 &;
+void referenceToTypedef1(CInt &CIR, int I) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 2 adjacent parameters of 'referenceToTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'CIR'
+// CHECK-MESSAGES: :[[@LINE-3]]:41: note: the last parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-4]]:37: note: a call site binds an expression to 'CInt &' and 'int'
+
void referenceThroughTypedef(int I, ICRTy Builtin, MyIntCRTy MyInt) {}
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: 3 adjacent parameters of 'referenceThroughTypedef' of similar type are
// CHECK-MESSAGES: :[[@LINE-2]]:34: note: the first parameter in the range is 'I'
// CHECK-MESSAGES: :[[@LINE-3]]:62: note: the last parameter in the range is 'MyInt'
-// CHECK-MESSAGES: :[[@LINE-4]]:30: note: after resolving type aliases, the common type of 'int' and 'ICRTy' is 'const int'
-// CHECK-MESSAGES: :[[@LINE-5]]:37: note: a call site binds an expression to 'int' and 'ICRTy'
-// CHECK-MESSAGES: :[[@LINE-6]]:30: note: after resolving type aliases, 'int' and 'MyIntCRTy' are the same
-// CHECK-MESSAGES: :[[@LINE-7]]:52: note: a call site binds an expression to 'int' and 'MyIntCRTy'
-// CHECK-MESSAGES: :[[@LINE-8]]:37: note: after resolving type aliases, the common type of 'ICRTy' and 'MyIntCRTy' is 'int'
-// CHECK-MESSAGES: :[[@LINE-9]]:52: note: a call site binds an expression to 'ICRTy' and 'MyIntCRTy'
+// CHECK-MESSAGES: :[[@LINE-4]]:37: note: a call site binds an expression to 'int' and 'ICRTy'
+// CHECK-MESSAGES: :[[@LINE-5]]:30: note: after resolving type aliases, 'int' and 'MyIntCRTy' are the same
+// CHECK-MESSAGES: :[[@LINE-6]]:52: note: a call site binds an expression to 'int' and 'MyIntCRTy'
+// CHECK-MESSAGES: :[[@LINE-7]]:37: note: after resolving type aliases, the common type of 'ICRTy' and 'MyIntCRTy' is 'int'
+// CHECK-MESSAGES: :[[@LINE-8]]:52: note: a call site binds an expression to 'ICRTy' and 'MyIntCRTy'
short const typedef int unsigned Eldritch;
typedef const unsigned short Holy;
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
@@ -2,7 +2,8 @@
// 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: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "T"}, \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0} \
// RUN: ]}' --
void ignoredUnnamed(int I, int, int) {} // NO-WARN: No >= 2 length of non-unnamed.
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
@@ -32,6 +32,35 @@
Options
-------
+Extension/relaxation options
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Relaxation (or extension) options can be used to broaden the scope of the
+analysis and fine-tune the enabling of more mix between types.
+Some mix may depend on coding style or preference specific to a project,
+however, it should be noted that enabling *all* of these relaxations model the
+way of mixing at call sites the most.
+These options are expected to make the check report for more functions, and
+report longer mixable ranges.
+
+.. option:: QualifiersMix
+
+ Whether to consider parameters of some *cvr-qualified* ``T`` and a
+ differently *cvr-qualified* ``T`` (i.e. ``T`` and ``const T``, ``const T``
+ and ``volatile T``, etc.) mixable between one another.
+ If ``0``, the check assumes that such parameters cannot be
+ mixed.
+ A *non-zero value* turns this assumption **off**.
+ Defaults to ``0``.
+
+ The following example produces a diagnostic only if `QualifiersMix` is
+ enabled:
+
+ .. code-block:: c++
+
+ void *memcpy(const void *Destination, void *Source, std::size_t N) {}
+
+
Filtering options
^^^^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
@@ -38,6 +38,9 @@
/// The parameter typename suffixes (as written in the source code) to be
/// ignored.
const std::vector<std::string> IgnoredParameterTypeSuffixes;
+
+ /// Whether to consider an unqualified and a qualified type mixable.
+ const bool QualifiersMix;
};
} // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
@@ -54,6 +54,9 @@
"reverse_iterator",
"reverse_const_iterator"});
+/// The default value for the QualifiersMix check option.
+static constexpr bool DefaultQualifiersMix = false;
+
#ifndef NDEBUG
template <std::size_t Width>
@@ -111,10 +114,11 @@
FB(TypeAlias, 4), //< The path from one type to the other involves
// desugaring type aliases.
FB(ReferenceBind, 5), //< The mix involves the binding power of "const &".
+ FB(Qualifiers, 6), //< The mix involves change in the qualifiers.
#undef FB
- LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue =*/MIX_ReferenceBind)
+ LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue =*/MIX_Qualifiers)
};
LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
@@ -153,13 +157,24 @@
Flags &= ~MIX_Trivial;
}
+ /// Add the specified flag bits to the flags.
MixData operator|(MixFlags EnableFlags) const {
return {Flags | EnableFlags, CommonType};
}
+
+ /// Add the specified flag bits to the flags.
MixData &operator|=(MixFlags EnableFlags) {
Flags |= EnableFlags;
return *this;
}
+
+ /// Add the specified qualifiers to the common type in the Mix.
+ MixData operator<<(Qualifiers Quals) const {
+ SplitQualType Split = CommonType.split();
+ Split.Quals.addQualifiers(Quals);
+
+ return {Flags, QualType(Split.Ty, Split.Quals.getAsOpaqueValue())};
+ }
};
/// A named tuple that contains the information for a mix between two concrete
@@ -249,18 +264,6 @@
RType.getSingleStepDesugaredType(Ctx), Ctx);
}
- // Dissolve typedefs.
- if (const auto *LTypedef = LType->getAs<TypedefType>()) {
- LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. LHS is typedef.\n");
- return calculateMixability(Check, LTypedef->desugar(), RType, Ctx) |
- MIX_TypeAlias;
- }
- if (const auto *RTypedef = RType->getAs<TypedefType>()) {
- LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. RHS is typedef.\n");
- return calculateMixability(Check, LType, RTypedef->desugar(), Ctx) |
- MIX_TypeAlias;
- }
-
// At a particular call site, what could be passed to a 'T' or 'const T' might
// also be passed to a 'const T &' without the call site putting a direct
// side effect on the passed expressions.
@@ -275,6 +278,62 @@
MIX_ReferenceBind;
}
+ // Dissolve typedefs after the qualifiers outside the typedef is dealt with.
+ if (LType->getAs<TypedefType>()) {
+ LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. LHS is typedef.\n");
+ return calculateMixability(Check, LType.getSingleStepDesugaredType(Ctx),
+ RType, Ctx) |
+ MIX_TypeAlias;
+ }
+ if (RType->getAs<TypedefType>()) {
+ LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. RHS is typedef.\n");
+ return calculateMixability(Check, LType,
+ RType.getSingleStepDesugaredType(Ctx), Ctx) |
+ MIX_TypeAlias;
+ }
+
+ // A parameter of type 'cvr1 T' and another of potentially differently
+ // qualified 'cvr2 T' may bind with the same power, if the user so requested.
+ if (LType.getLocalCVRQualifiers() != RType.getLocalCVRQualifiers()) {
+ LLVM_DEBUG(if (LType.getLocalCVRQualifiers()) llvm::dbgs()
+ << "--- calculateMixability. LHS is CVR: "
+ << formatBits(LType.getLocalCVRQualifiers()) << "\n");
+ LLVM_DEBUG(if (RType.getLocalCVRQualifiers()) llvm::dbgs()
+ << "--- calculateMixability. RHS is CVR: "
+ << formatBits(RType.getLocalCVRQualifiers()) << "\n");
+
+ if (!Check.QualifiersMix) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "<<< calculateMixability. QualifiersMix turned off.\n");
+ return {MIX_None};
+ }
+
+ return calculateMixability(Check, LType.getLocalUnqualifiedType(),
+ RType.getLocalUnqualifiedType(), Ctx) |
+ MIX_Qualifiers;
+ }
+ if (LType.getLocalCVRQualifiers() == RType.getLocalCVRQualifiers() &&
+ LType.getLocalCVRQualifiers() != 0) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "--- calculateMixability. LHS and RHS same CVR: "
+ << formatBits(LType.getLocalCVRQualifiers()) << "\n");
+ // Apply the same qualifier back into the found common type if we found
+ // a common type between the unqualified versions.
+ return calculateMixability(Check, LType.getLocalUnqualifiedType(),
+ RType.getLocalUnqualifiedType(), Ctx)
+ << LType.getLocalQualifiers();
+ }
+
+ if (LType->isPointerType() && RType->isPointerType()) {
+ // If both types are pointers, and pointed to the exact same type,
+ // LType == RType took care of that.
+ // Try to see if the pointee type has some other match.
+ LLVM_DEBUG(llvm::dbgs()
+ << "--- calculateMixability. LHS and RHS are Ptrs.\n");
+ return calculateMixability(Check, LType->getPointeeType(),
+ RType->getPointeeType(), Ctx);
+ }
+
// 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()) {
@@ -298,21 +357,44 @@
Ty.dump(llvm::dbgs(), Ctx); llvm::dbgs() << '\n';);
QualType ReferredType = LRef->getPointeeType();
- if (!ReferredType.isLocalConstQualified()) {
+ if (!ReferredType.isLocalConstQualified() &&
+ ReferredType->getAs<TypedefType>()) {
+ LLVM_DEBUG(
+ llvm::dbgs()
+ << "--- isLRefEquallyBindingToType. Non-const LRef to Typedef.\n");
+ ReferredType = ReferredType.getDesugaredType(Ctx);
+ if (!ReferredType.isLocalConstQualified()) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "<<< isLRefEquallyBindingToType. Typedef is not const.\n");
+ return {MIX_None};
+ }
+
+ LLVM_DEBUG(llvm::dbgs() << "--- isLRefEquallyBindingToType. Typedef is "
+ "const, considering as const LRef.\n");
+ } else if (!ReferredType.isLocalConstQualified()) {
LLVM_DEBUG(llvm::dbgs()
- << "<<< isLRefEquallyBindingToType. Not const ref.\n");
+ << "<<< isLRefEquallyBindingToType. Not const LRef.\n");
return {MIX_None};
};
- QualType NonConstReferredType = ReferredType;
- NonConstReferredType.removeLocalConst();
- if (ReferredType == Ty || NonConstReferredType == Ty) {
+ assert(ReferredType.isLocalConstQualified() &&
+ "Reaching this point means we are sure LRef is effectively a const&.");
+
+ if (ReferredType == Ty) {
LLVM_DEBUG(
llvm::dbgs()
<< "<<< isLRefEquallyBindingToType. Type of referred matches.\n");
return {MIX_Trivial, ReferredType};
}
+ QualType NonConstReferredType = ReferredType;
+ NonConstReferredType.removeLocalConst();
+ if (NonConstReferredType == Ty) {
+ LLVM_DEBUG(llvm::dbgs() << "<<< isLRefEquallyBindingToType. Type of "
+ "referred matches to non-const qualified.\n");
+ return {MIX_Trivial, NonConstReferredType};
+ }
+
LLVM_DEBUG(
llvm::dbgs()
<< "--- isLRefEquallyBindingToType. Checking mix for underlying type.\n");
@@ -478,7 +560,8 @@
/// Returns whether a particular Mix between two parameters should have the
/// types involved diagnosed to the user. This is only a flag check.
static inline bool needsToPrintTypeInDiagnostic(const model::Mix &M) {
- return M.flags() & (model::MIX_TypeAlias | model::MIX_ReferenceBind);
+ return M.flags() & (model::MIX_TypeAlias | model::MIX_ReferenceBind |
+ model::MIX_Qualifiers);
}
namespace {
@@ -562,7 +645,8 @@
Options.get("IgnoredParameterNames", DefaultIgnoredParameterNames))),
IgnoredParameterTypeSuffixes(optutils::parseStringList(
Options.get("IgnoredParameterTypeSuffixes",
- DefaultIgnoredParameterTypeSuffixes))) {}
+ DefaultIgnoredParameterTypeSuffixes))),
+ QualifiersMix(Options.get("QualifiersMix", DefaultQualifiersMix)) {}
void EasilySwappableParametersCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
@@ -571,6 +655,7 @@
optutils::serializeStringList(IgnoredParameterNames));
Options.store(Opts, "IgnoredParameterTypeSuffixes",
optutils::serializeStringList(IgnoredParameterTypeSuffixes));
+ Options.store(Opts, "QualifiersMix", QualifiersMix);
}
void EasilySwappableParametersCheck::registerMatchers(MatchFinder *Finder) {
@@ -664,18 +749,21 @@
QualType LType = M.First->getType();
QualType RType = M.Second->getType();
QualType CommonType = M.commonUnderlyingType();
- std::string LTypeAsWritten = LType.getAsString(PP);
- std::string RTypeAsWritten = RType.getAsString(PP);
+ std::string LTypeStr = LType.getAsString(PP);
+ std::string RTypeStr = RType.getAsString(PP);
std::string CommonTypeStr = CommonType.getAsString(PP);
if (M.flags() & model::MIX_TypeAlias &&
UniqueTypeAlias(LType, RType, CommonType)) {
StringRef DiagText;
bool ExplicitlyPrintCommonType = false;
- if (LTypeAsWritten == CommonTypeStr ||
- RTypeAsWritten == CommonTypeStr)
- DiagText =
- "after resolving type aliases, '%0' and '%1' are the same";
+ if (LTypeStr == CommonTypeStr || RTypeStr == CommonTypeStr)
+ if (M.flags() & model::MIX_Qualifiers)
+ DiagText = "after resolving type aliases, '%0' and '%1' share a "
+ "common type";
+ else
+ DiagText =
+ "after resolving type aliases, '%0' and '%1' are the same";
else {
DiagText = "after resolving type aliases, the common type of '%0' "
"and '%1' is '%2'";
@@ -684,17 +772,17 @@
auto Diag =
diag(M.First->getOuterLocStart(), DiagText, DiagnosticIDs::Note)
- << LTypeAsWritten << RTypeAsWritten;
+ << LTypeStr << RTypeStr;
if (ExplicitlyPrintCommonType)
Diag << CommonTypeStr;
}
- if (M.flags() & model::MIX_ReferenceBind &&
+ if (M.flags() & (model::MIX_ReferenceBind | model::MIX_Qualifiers) &&
UniqueBindPower({LType, RType})) {
StringRef DiagText = "a call site binds an expression to '%0' and "
"'%1' with the same force";
diag(M.Second->getOuterLocStart(), DiagText, DiagnosticIDs::Note)
- << LTypeAsWritten << RTypeAsWritten;
+ << LTypeStr << RTypeStr;
}
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
