whisperity created this revision.
whisperity added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, martong, gamesh411, Szelethus, dkrupp,
rnkovacs, kbarton, xazax.hun, nemanjai.
Herald added a project: clang.
whisperity added a parent revision: D69560: [clang-tidy] Add
'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check.
Herald added a subscriber: wuzish.
whisperity planned changes to this revision.
This patch adds an optional modelling of implicit conversions to the
`cppcoreguidelines-avoid-adjacent-arguments-of-same-type` check. This will
allow also diagnosing functions such as `void f(int i, double d);` to have a
mix-up chance, i.e. `f(double_var, int_var);`. User-defined conversions are
also attempted and supported.
Unfortunately, there isn't a sensible way (at least I found none) to ask `Sema`
at //"frontend time"// whether or not the two args could be implicitly
converted between one another, as we do not have the `Sema` instance anymore.
Hence there is extra work done here to model the conversions - we believe the
check's code under-approximates the real issue, but the current findings are
valid semantically.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D75041
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-cvr-on.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-default.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-implicits.c
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-implicits.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-verbose.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.c
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.c
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.c
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.c
@@ -2,7 +2,8 @@
// RUN: cppcoreguidelines-avoid-adjacent-arguments-of-same-type %t \
// RUN: -config='{CheckOptions: [ \
// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.MinimumLength, value: 2}, \
-// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 1} \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 1}, \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.ImplicitConversion, value: 0} \
// RUN: ]}' --
struct T {};
@@ -29,3 +30,6 @@
int equals2(S l, S r) { return l.x == r.x; }
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 2 adjacent arguments for 'equals2' of similar type ('S') are
// CHECK-MESSAGES: :[[@LINE-2]]:20: note: last argument in the adjacent range
+
+void ptrs(int *i, long *l) {}
+// NO-WARN: Mixing fundamentals' pointers is diagnosed by compiler warnings.
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-verbose.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-verbose.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-verbose.cpp
@@ -2,7 +2,8 @@
// RUN: cppcoreguidelines-avoid-adjacent-arguments-of-same-type %t \
// RUN: -config='{CheckOptions: [ \
// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.MinimumLength, value: 2}, \
-// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 1} \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 1}, \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.ImplicitConversion, value: 0} \
// RUN: ]}' --
void library(void *vp, void *vp2, void *vp3, int n, int m);
@@ -124,6 +125,18 @@
// CHECK-MESSAGES: :[[@LINE-3]]:32: note: after resolving type aliases, type of argument 'p1' is 'int *'
// CHECK-MESSAGES: :[[@LINE-4]]:40: note: after resolving type aliases, type of argument 'p2' is 'int *'
+void typedef_and_realtype_ptr_const(int *p1, const I1 *p2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: 2 adjacent arguments for 'typedef_and_realtype_ptr_const' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:56: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:37: note: after resolving type aliases, type of argument 'p1' is 'int *'
+// CHECK-MESSAGES: :[[@LINE-4]]:46: note: after resolving type aliases, type of argument 'p2' is 'const int *'
+// CHECK-MESSAGES: :[[@LINE-5]]:46: note: at a call site, 'const int *' might bind with same force as 'int *'
+
+void typedef_ptr_const(I1 *p1, const I1 *p2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 2 adjacent arguments for 'typedef_ptr_const' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:42: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:32: note: at a call site, 'const int *' might bind with same force as 'I1 *'
+
typedef I2 myInt;
typedef I2 myOtherInt;
void typedef_chain(myInt mi, myOtherInt moi) {}
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-implicits.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-implicits.cpp
@@ -0,0 +1,330 @@
+// RUN: %check_clang_tidy %s \
+// RUN: cppcoreguidelines-avoid-adjacent-arguments-of-same-type %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.MinimumLength, value: 2}, \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 0}, \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.ImplicitConversion, value: 1} \
+// RUN: ]}' --
+
+void compare(int a, int b, int c) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 3 adjacent arguments for 'compare' of similar type ('int') are easily swapped by mistake [cppcoreguidelines-avoid-adjacent-arguments-of-same-type]
+// CHECK-MESSAGES: :[[@LINE-2]]:32: note: last argument in the adjacent range here
+
+void b(bool b1, bool b2, int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 3 adjacent arguments for 'b' of convertible types may be easily swapped by mistake [cppcoreguidelines-avoid-adjacent-arguments-of-same-type]
+// CHECK-MESSAGES: :[[@LINE-2]]:30: note: last argument in the adjacent range here
+// CHECK-MESSAGES: :[[@LINE-3]]:26: note: 'bool' and 'int' can suffer implicit conversions between one another{{$}}
+
+void integral1(signed char sc, int si) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'integral1' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:36: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:32: note: 'signed char' and 'int' can suffer implicit conversions between one another{{$}}
+
+void integral2(long l, int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'integral2' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:28: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:24: note: 'long' and 'int' can suffer implicit conversions between one another{{$}}
+
+void integral3(int i, long l) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'integral3' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:28: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:23: note: 'int' and 'long' can suffer implicit conversions between one another{{$}}
+
+void integral4(char c, int i, long l) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 3 adjacent arguments for 'integral4' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:36: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:24: note: 'char' and 'int' can suffer implicit conversions between one another{{$}}
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: 'char' and 'long' can suffer implicit conversions between one another{{$}}
+
+void floating1(float f, double d) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'floating1' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:32: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:25: note: 'float' and 'double' can suffer implicit conversions between one another{{$}}
+
+typedef double D;
+void floating2(float f, D d) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'floating2' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:27: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:16: note: after resolving type aliases, type of argument 'f' is 'float'
+// CHECK-MESSAGES: :[[@LINE-4]]:25: note: after resolving type aliases, type of argument 'd' is 'double'
+// CHECK-MESSAGES: :[[@LINE-5]]:25: note: 'float' and 'double' can suffer implicit conversions between one another{{$}}
+
+void floatToInt(float f, int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent arguments for 'floatToInt' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:30: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:26: note: 'float' and 'int' can suffer implicit conversions between one another{{$}}
+
+enum En { A,
+ B };
+
+void unscopedEnumToIntegral(En e, int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: 2 adjacent arguments for 'unscopedEnumToIntegral' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:39: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:35: note: 'En' can be implicitly converted to 'int'
+
+void unscopedEnumToIntegral2(En e, unsigned long long ull) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: 2 adjacent arguments for 'unscopedEnumToIntegral2' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:55: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: 'En' can be implicitly converted to 'unsigned long long'
+
+void unscopedEnum3(char c, En e, char c2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 3 adjacent arguments for 'unscopedEnum3' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:39: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:28: note: 'char' can be implicitly converted from 'En'
+// CHECK-MESSAGES: :[[@LINE-4]]:34: note: 'En' can be implicitly converted to 'char'
+
+void unscopedEnumToFloating(En e, long double ld) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: 2 adjacent arguments for 'unscopedEnumToFloating' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:47: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:35: note: 'En' can be implicitly converted to 'long double'
+
+void unscopedEnumToIntOrFloat(En e, int i, float f) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 3 adjacent arguments for 'unscopedEnumToIntOrFloat' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:50: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:37: note: 'En' can be implicitly converted to 'int'
+// CHECK-MESSAGES: :[[@LINE-4]]:44: note: 'En' can be implicitly converted to 'float'
+// CHECK-MESSAGES: :[[@LINE-5]]:44: note: 'int' and 'float' can suffer implicit conversions between one another{{$}}
+
+enum class SEn { C,
+ D };
+void scopedEnumToIntegral(SEn e, int i) {}
+// NO-WARN: Scoped enumerations mustn't be promoted.
+
+struct FromInt {
+ FromInt(int);
+};
+
+void userConv1(int i, FromInt fi) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'userConv1' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:31: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:23: note: 'int' can be implicitly converted to 'FromInt'
+
+void userConv1c(int i, const FromInt cfi) {}
+// NO-WARN: CVRMixPossible is set to 0, so an 'int' and a 'const "int"' does not mix.
+
+void userConv1cr(int i, const FromInt &cfir) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 2 adjacent arguments for 'userConv1cr' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:40: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:25: note: at a call site, 'const FromInt &' might bind with same force as 'int'
+// CHECK-MESSAGES: :[[@LINE-4]]:25: note: 'int' can be implicitly converted to 'const FromInt &'
+
+void userConv2(unsigned long long i, FromInt fi) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'userConv2' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:46: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:38: note: 'unsigned long long' can be implicitly converted to 'FromInt': 'unsigned long long' -> 'int' -> 'FromInt'
+
+struct ToInt {
+ operator int() const;
+};
+
+void userConv3(int i, ToInt ti) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'userConv3' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:23: note: 'int' can be implicitly converted from 'ToInt'
+
+void userConv4(double d, FromInt fi) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'userConv4' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:34: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:26: note: 'double' can be implicitly converted to 'FromInt': 'double' -> 'int' -> 'FromInt'
+
+void userConv4cr(double d, const FromInt &cfir) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 2 adjacent arguments for 'userConv4cr' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:43: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:28: note: at a call site, 'const FromInt &' might bind with same force as 'double'
+// CHECK-MESSAGES: :[[@LINE-4]]:28: note: 'double' can be implicitly converted to 'const FromInt &': 'double' -> 'int' -> 'FromInt'
+
+struct FromDouble {
+ FromDouble(double);
+};
+
+void userConv5(En e, FromDouble fd) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'userConv5' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:33: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:22: note: 'En' can be implicitly converted to 'FromDouble': 'En' -> 'double' -> 'FromDouble'
+
+struct Ambiguous {
+ Ambiguous(int);
+ Ambiguous(long);
+};
+
+void userConv6(En e, Ambiguous a) {}
+// NO-WARN: En -> int -> Ambiguous vs. En -> long -> Ambiguous.
+
+struct ToExplicitInt {
+ explicit operator int() const;
+};
+struct FromExplicitInt {
+ explicit FromExplicitInt(int);
+};
+
+void nonConverting1(int i, FromExplicitInt fei) {}
+// NO-WARN: There is no chance of mix-up as 'fei' has only explicit constructor.
+
+void nonConverting2(int i, ToExplicitInt tei) {}
+// NO-WARN: 'tei' converts to 'int' only in explicit context.
+
+struct Both {
+ Both(int);
+ operator int() const;
+};
+
+typedef int I;
+typedef double D;
+
+void both1(int i, Both b) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent arguments for 'both1' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:24: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:19: note: 'int' and 'Both' can suffer implicit conversions between one another{{$}}
+
+void both2(double d, Both b) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent arguments for 'both2' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:27: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:22: note: 'double' and 'Both' can suffer implicit conversions between one another: 'double' -> 'int' -> 'Both' and 'Both' -> 'int' -> 'double'
+
+void both2typedef(D d, Both b) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 2 adjacent arguments for 'both2typedef' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:19: note: after resolving type aliases, type of argument 'd' is 'double'
+// CHECK-MESSAGES: :[[@LINE-4]]:24: note: after resolving type aliases, type of argument 'b' is 'Both'
+// CHECK-MESSAGES: :[[@LINE-5]]:24: note: 'double' and 'Both' can suffer implicit conversions between one another: 'double' -> 'int' -> 'Both' and 'Both' -> 'int' -> 'double'
+
+struct BoxedInt {
+ BoxedInt();
+ BoxedInt(const BoxedInt &);
+ BoxedInt(BoxedInt &&);
+ BoxedInt(const Both &B);
+};
+
+void both3(int i, BoxedInt biv) {}
+// NO-WARN: Two converting constructor calls (int->both->BoxedInt) is not
+// possible implicitly.
+
+struct IntAndDouble {
+ IntAndDouble(const int);
+ IntAndDouble(const double);
+
+ operator int() const;
+ operator double() const;
+};
+
+void twoTypes1(int i, IntAndDouble iad) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'twoTypes1' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:36: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:23: note: 'int' and 'IntAndDouble' can suffer implicit conversions between one another{{$}}
+
+void twoTypes2(double d, IntAndDouble iad) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'twoTypes2' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:39: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:26: note: 'double' and 'IntAndDouble' can suffer implicit conversions between one another{{$}}
+
+void twoTypes3(unsigned long ul, IntAndDouble iad) {}
+// NO-WARN: Ambiguous constructor call and conversion call, should we take
+// 'int' or 'double' route from 'unsigned long'?
+
+struct IADTypedef {
+ IADTypedef(I);
+ IADTypedef(D);
+ operator I() const;
+ operator D() const;
+};
+
+void twoTypedefs1(int i, IADTypedef iadt) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 2 adjacent arguments for 'twoTypedefs1' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:37: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:26: note: 'int' and 'IADTypedef' can suffer implicit conversions between one another{{$}}
+
+void twoTypedefs2(double d, IADTypedef iadt) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 2 adjacent arguments for 'twoTypedefs2' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:40: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:29: note: 'double' and 'IADTypedef' can suffer implicit conversions between one another{{$}}
+
+void twoTypedefs3(unsigned long ul, IADTypedef iadt) {}
+// NO-WARN: Ambiguous constructor call and conversion call, should we take
+// 'int' or 'double' route from 'unsigned long'?
+
+struct Fwd1;
+struct Fwd2;
+void forwards(Fwd1 *f1, Fwd2 *f2) {}
+// NO-WARN and NO-CRASH: Don't try to compare not complete types.
+
+struct Rec {};
+struct FromRec {
+ FromRec(const Rec &);
+};
+struct ToRec {
+ operator Rec() const;
+};
+struct RecBoth {
+ RecBoth(Rec);
+ operator Rec() const;
+};
+
+void record2record_1(Rec r, FromRec fr) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent arguments for 'record2record_1' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:37: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:29: note: 'Rec' can be implicitly converted to 'FromRec'
+
+void record2record_2(Rec r, ToRec tr) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent arguments for 'record2record_2' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:35: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:29: note: 'Rec' can be implicitly converted from 'ToRec'
+
+void record2record_3(Rec r, RecBoth rb) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent arguments for 'record2record_3' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:37: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:29: note: 'Rec' and 'RecBoth' can suffer implicit conversions between one another{{$}}
+
+void record2record_4(Rec r, FromRec fr, ToRec tr, RecBoth rb) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 4 adjacent arguments for 'record2record_4' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:59: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:29: note: 'Rec' can be implicitly converted to 'FromRec'
+// CHECK-MESSAGES: :[[@LINE-4]]:41: note: 'Rec' can be implicitly converted from 'ToRec'
+// CHECK-MESSAGES: :[[@LINE-5]]:51: note: 'Rec' and 'RecBoth' can suffer implicit conversions between one another{{$}}
+
+struct X;
+struct Y;
+struct X {
+ X();
+ X(Y);
+ operator Y();
+};
+struct Y {
+ Y();
+ Y(X);
+ operator X();
+};
+void ambiguous_records(X x, Y y) {}
+// NO-WARN: Ambiguous conversion if the arguments are swapped, which results in
+// compiler error: f3(Y{}, X{})
+
+struct Base {};
+struct Derived1 : Base {};
+struct Derived2 : Base {};
+void upcasting(Base *bp, Derived1 *d1p, Derived2 *d2p) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 3 adjacent arguments for 'upcasting' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:51: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:26: note: 'Base *' can be implicitly converted from 'Derived1 *'
+// CHECK-MESSAGES: :[[@LINE-4]]:41: note: 'Base *' can be implicitly converted from 'Derived2 *'
+
+void upcasting_ref(const Base &br, const Derived1 &d1r, const Derived2 &d2r) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 3 adjacent arguments for 'upcasting_ref' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:73: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: at a call site, 'const Derived1 &' might bind with same force as 'const Base &'
+// CHECK-MESSAGES: :[[@LINE-4]]:36: note: 'const Base &' can be implicitly converted from 'const Derived1 &'
+// CHECK-MESSAGES: :[[@LINE-5]]:57: note: at a call site, 'const Derived2 &' might bind with same force as 'const Base &'
+// CHECK-MESSAGES: :[[@LINE-6]]:57: note: 'const Base &' can be implicitly converted from 'const Derived2 &'
+
+// Reduced case from live crash on OpenCV
+// http://github.com/opencv/opencv/blob/1996ae4a42d7c7cd338fbdd4abbd83b41b448328/modules/core/include/opencv2/core/types.hpp#L173
+template <typename T>
+struct TemplateConversion {
+ TemplateConversion();
+ TemplateConversion(const T &);
+ template <typename T2>
+ operator TemplateConversion<T2>() const;
+};
+typedef TemplateConversion<int> IntConvert;
+typedef TemplateConversion<float> FloatConvert;
+void templated_conversion_to_other_specialisation(FloatConvert fc, IntConvert ic) {}
+// NO-WARN: Template conversion operators are not resolved, we have approximated
+// much of Sema already...
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-implicits.c
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-implicits.c
@@ -0,0 +1,90 @@
+// RUN: %check_clang_tidy %s \
+// RUN: cppcoreguidelines-avoid-adjacent-arguments-of-same-type %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.MinimumLength, value: 2}, \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 0}, \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.ImplicitConversion, value: 1} \
+// RUN: ]}' --
+
+void compare(int a, int b, int c) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 3 adjacent arguments for 'compare' of similar type ('int') are easily swapped by mistake [cppcoreguidelines-avoid-adjacent-arguments-of-same-type]
+// CHECK-MESSAGES: :[[@LINE-2]]:32: note: last argument in the adjacent range here
+
+void b(_Bool b1, _Bool b2, int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 3 adjacent arguments for 'b' of convertible types may be easily swapped by mistake [cppcoreguidelines-avoid-adjacent-arguments-of-same-type]
+// CHECK-MESSAGES: :[[@LINE-2]]:32: note: last argument in the adjacent range here
+// CHECK-MESSAGES: :[[@LINE-3]]:28: note: '_Bool' and 'int' can suffer implicit conversions between one another{{$}}
+
+void integral1(signed char sc, int si) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'integral1' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:36: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:32: note: 'signed char' and 'int' can suffer implicit conversions between one another{{$}}
+
+void integral2(long l, int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'integral2' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:28: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:24: note: 'long' and 'int' can suffer implicit conversions between one another{{$}}
+
+void integral3(int i, long l) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'integral3' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:28: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:23: note: 'int' and 'long' can suffer implicit conversions between one another{{$}}
+
+void integral4(char c, int i, long l) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 3 adjacent arguments for 'integral4' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:36: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:24: note: 'char' and 'int' can suffer implicit conversions between one another{{$}}
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: 'char' and 'long' can suffer implicit conversions between one another{{$}}
+
+void floating1(float f, double d) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'floating1' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:32: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:25: note: 'float' and 'double' can suffer implicit conversions between one another{{$}}
+
+typedef double D;
+void floating2(float f, D d) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 2 adjacent arguments for 'floating2' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:27: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:16: note: after resolving type aliases, type of argument 'f' is 'float'
+// CHECK-MESSAGES: :[[@LINE-4]]:25: note: after resolving type aliases, type of argument 'd' is 'double'
+// CHECK-MESSAGES: :[[@LINE-5]]:25: note: 'float' and 'double' can suffer implicit conversions between one another{{$}}
+
+void floatToInt(float f, int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent arguments for 'floatToInt' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:30: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:26: note: 'float' and 'int' can suffer implicit conversions between one another{{$}}
+
+enum En { A,
+ B };
+
+void unscopedEnumToIntegral(enum En e, int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: 2 adjacent arguments for 'unscopedEnumToIntegral' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:44: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:40: note: 'enum En' and 'int' can suffer implicit conversions between one another{{$}}
+
+void unscopedEnumToIntegral2(enum En e, unsigned long long ull) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: 2 adjacent arguments for 'unscopedEnumToIntegral2' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:60: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:41: note: 'enum En' and 'unsigned long long' can suffer implicit conversions between one another{{$}}
+
+void unscopedEnum3(char c, enum En e, char c2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 3 adjacent arguments for 'unscopedEnum3' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:44: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:28: note: 'char' and 'enum En' can suffer implicit conversions between one another{{$}}
+// CHECK-MESSAGES: :[[@LINE-4]]:39: note: 'enum En' and 'char' can suffer implicit conversions between one another{{$}}
+
+void unscopedEnumToFloating(enum En e, long double ld) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: 2 adjacent arguments for 'unscopedEnumToFloating' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:52: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:40: note: 'enum En' and 'long double' can suffer implicit conversions between one another{{$}}
+
+void unscopedEnumToIntOrFloat(enum En e, int i, float f) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 3 adjacent arguments for 'unscopedEnumToIntOrFloat' of convertible
+// CHECK-MESSAGES: :[[@LINE-2]]:55: note: last argument in the adjacent range
+// CHECK-MESSAGES: :[[@LINE-3]]:42: note: 'enum En' and 'int' can suffer implicit conversions between one another{{$}}
+// CHECK-MESSAGES: :[[@LINE-4]]:49: note: 'enum En' and 'float' can suffer implicit conversions between one another{{$}}
+// CHECK-MESSAGES: :[[@LINE-5]]:49: note: 'int' and 'float' can suffer implicit conversions between one another{{$}}
+
+void ptr(int *i, long *l) {}
+// NO-WARN: Though 'int' and 'long' can be converted between one-another
+// implicitly, mixing such pointers is diagnosed by compiler warnings.
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-default.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-default.cpp
@@ -2,7 +2,8 @@
// RUN: cppcoreguidelines-avoid-adjacent-arguments-of-same-type %t \
// RUN: -config='{CheckOptions: [ \
// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.MinimumLength, value: 3}, \
-// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 0} \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 0}, \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.ImplicitConversion, value: 0} \
// RUN: ]}' --
void library(void *vp, void *vp2, void *vp3, int n, int m);
@@ -105,6 +106,12 @@
void value_ptr_quals(T *tp, const T *ctp, volatile T *vtp, const volatile T *cvtp, T *const tpc, T *volatile tpv) {}
+void typedef_and_realtype_ptr_const(int *p1, I1 *p2, const I1 *p3) {}
+
+void typedef_ptr_const(I1 *p1, I1 *p2, const I1 *p3) {}
+
+// END of NO-WARN.
+
// NO-WARN: const T* and T* should not mix, even if coming through a typedef.
// (min and max could be mixed up, but length requirement in this test is 3.)
typedef int Numeral;
@@ -112,8 +119,6 @@
const Numeral *const max,
Numeral *const value) {}
-// END of NO-WARN.
-
void protochain(int, int, int);
void protochain(int i, int, int);
void protochain(int, int j, int);
@@ -191,3 +196,20 @@
// CHECK-MESSAGES: :[[@LINE-5]]:25: note: after resolving type aliases, type of argument 'Background' is 'OldSchoolTermColour'
// CHECK-MESSAGES: :[[@LINE-5]]:25: note: after resolving type aliases, type of argument 'Foreground' is 'OldSchoolTermColour'
// CHECK-MESSAGES: :[[@LINE-5]]:25: note: after resolving type aliases, type of argument 'Border' is 'OldSchoolTermColour'
+
+// NO-WARN: Implicit conversions should not warn if the check option is turned off.
+
+void integral1(signed char sc, int si) {}
+
+struct FromInt {
+ FromInt(int);
+};
+void userConv1(int i, FromInt fi) {}
+
+struct Base {};
+struct Derived1 : Base {};
+struct Derived2 : Base {};
+void upcasting(Base *bp, Derived1 *d1p, Derived2 *d2p) {}
+void upcasting_ref(const Base &br, const Derived1 &d1r, const Derived2 &d2r) {}
+
+// END of NO-WARN.
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-cvr-on.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-cvr-on.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-arguments-of-same-type-cvr-on.cpp
@@ -2,7 +2,8 @@
// RUN: cppcoreguidelines-avoid-adjacent-arguments-of-same-type %t \
// RUN: -config='{CheckOptions: [ \
// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.MinimumLength, value: 3}, \
-// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 1} \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.CVRMixPossible, value: 1}, \
+// RUN: {key: cppcoreguidelines-avoid-adjacent-arguments-of-same-type.ImplicitConversion, value: 0} \
// RUN: ]}' --
struct T {};
@@ -51,6 +52,4 @@
Numeral *const value) {}
// CHECK-MESSAGES: :[[@LINE-3]]:17: warning: 3 adjacent arguments for 'set_lerped' of similar type are
// CHECK-MESSAGES: :[[@LINE-2]]:32: note: last argument in the adjacent range
-// CHECK-MESSAGES: :[[@LINE-5]]:17: note: after resolving type aliases, type of argument 'min' is 'const int * const'
-// CHECK-MESSAGES: :[[@LINE-4]]:17: note: after resolving type aliases, type of argument 'value' is 'int * const'
-// CHECK-MESSAGES: :[[@LINE-5]]:17: note: at a call site, 'int * const' might bind with same force as 'const Numeral *const'
+// CHECK-MESSAGES: :[[@LINE-3]]:17: note: at a call site, 'int * const' might bind with same force as 'const Numeral *const'
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.rst
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.rst
@@ -3,9 +3,9 @@
cppcoreguidelines-avoid-adjacent-arguments-of-same-type
=======================================================
-Finds function definitions where arguments of the same type follow each other
-directly, making call sites prone to calling the function with swapped or badly
-ordered arguments.
+Finds function definitions where arguments of the essentially similar type
+follow each other directly, making call sites prone to calling the function
+with swapped or badly ordered arguments.
The rule is part of the "Interfaces" profile of the C++ Core Guidelines, see
`I.24 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#i24-avoid-adjacent-unrelated-parameters-of-the-same-type>`_.
@@ -76,6 +76,41 @@
struct T {};
void f(T *tp, const T *ctp) {}
+.. option:: ImplicitConversion
+
+ Whether to allow the approximation of some usual
+ `implicit conversions <https://en.cppreference.com/w/cpp/language/implicit_conversion>`_
+ when considering which type might be potentially mixed up with another.
+ If `0` (default value), the check does not consider **any** implicit
+ conversions.
+ A non-zero value turns this **on**.
+ A non-zero value will in almost all cases produce a **significantly broader**
+ set of results.
+
+ .. warning::
+ Turning the modelling of implicit conversion sequences on
+ relaxes the constraints for "type convertibility" significantly,
+ however, it also applies a generous performance hit on the check's cost.
+ The check will have to explore a **polynomially more** possibilities:
+ O(n\ :sup:`2`\ ) instead of O(n) for each function's ``n`` arguments.
+ The emitted diagnostics will also be more verbose, which might take more
+ time to stringify.
+
+ It is advised to normally leave this option *off*.
+
+ For details on what is matched by this option, see
+ `Implicit conversion modelling`_.
+
+ The following examples will not produce a diagnostic unless
+ *ImplicitConversion* is set to a non-zero value.
+
+ .. code-block:: c++
+
+ void f(char c, int i, double d) {}
+
+ enum E { Ea, Eb };
+ void f(E e, int i, double d) {}
+
Limitations
-----------
@@ -125,3 +160,36 @@
const vector<T> & Vector,
typename vector<T>::const_reference_type RightEnd,
const typename vector<T>::element_type & LeftBegin) { /* ... */ }
+
+
+Implicit conversion modelling
+-----------------------------
+
+Given a function ``void f(T1 t, T2 u)``, unless ``T1`` and ``T2`` are the same,
+the check won't warn in *"default"* mode. If ``ImplicitConversion`` is turned on
+(see Options_), and either ``T1``
+`can be converted to <https://en.cppreference.com/w/cpp/language/implicit_conversion>`_
+``T2``, or vica versa, diagnostics will be made.
+The "adjacent argument mixup" is considered even if the conversion is asymmetric,
+which is natural in most cases.
+
+In the case of the following function, the numeric arguments can be
+converted between one another, and the ``SomeEnum`` is convertible to any
+numeric type.
+Thus, an "adjacent argument range" of **5** is diagnosed here.
+
+.. code-blocK:: c++
+
+ enum SomeEnum { /* ... */ };
+ void SomeFunction(int, float, double, unsigned long, SomeEnum) {}
+
+Currently, the following implicit conversions are modelled:
+
+ - *standard conversion* sequences:
+ - numeric promotion and conversion between integer (including ``enum``) and
+ floating types, considered essentially the same under the umbrella term
+ "conversion"
+ - For **C** projects, the numeric conversion rules are relaxed to conform
+ to C rules.
+ - *user defined conversions*: non-``explicit`` converting constructors and
+ conversion operators.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,9 +115,9 @@
<clang-tidy/checks/cppcoreguidelines-avoid-adjacent-arguments-of-same-type>`
check.
- Finds function definitions where arguments of the same type follow each other
- directly, making call sites prone to calling the function with swapped or badly
- ordered arguments.
+ Finds function definitions where arguments of the essentially similar type
+ follow each other directly, making call sites prone to calling the function
+ with swapped or badly ordered arguments.
- New :doc:`cppcoreguidelines-init-variables
<clang-tidy/checks/cppcoreguidelines-init-variables>` check.
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.h
@@ -15,9 +15,9 @@
namespace tidy {
namespace cppcoreguidelines {
-/// Finds function definitions where arguments of the same type follow each
-/// other directly, making call sites prone to calling the function with swapped
-/// or badly ordered arguments.
+/// Finds function definitions where arguments of the essentially similar type
+/// 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/cppcoreguidelines-avoid-adjacent-arguments-of-same-type.html
@@ -36,6 +36,10 @@
/// Whether to consider 'T' and 'const T'/'volatile T'/etc. arguments to be
/// possible mixup.
const bool CVRMixPossible;
+
+ /// Whether to consider implicit conversion possibilities as a potential match
+ /// for adjacency.
+ const bool ImplicitConversion;
};
} // namespace cppcoreguidelines
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.cpp
@@ -10,6 +10,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "llvm/ADT/BitmaskEnum.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
#include <string>
@@ -20,6 +21,28 @@
#define DEBUG_TYPE "AdjacentArguments"
#include "llvm/Support/Debug.h"
// clang-format on
+template <std::size_t Width> inline std::string format_bytes_do(std::size_t N) {
+ static_assert(std::is_unsigned<decltype(N)>::value,
+ "size_t is not unsigned?!");
+ constexpr std::size_t WidthWithPadding = Width + (Width / 4);
+ char C[WidthWithPadding];
+ for (std::size_t CurPos = 0; CurPos < WidthWithPadding; ++CurPos) {
+ if (CurPos % 5 == 0) {
+ // Group digits by 4.
+ C[CurPos] = ' ';
+ continue;
+ }
+
+ C[CurPos] = N % 2 ? '1' : '0';
+ N >>= 1;
+ }
+
+ return std::string{std::rbegin(C), std::rend(C)};
+}
+
+template <typename T> inline std::string format_bytes(T &&t) {
+ return format_bytes_do<sizeof(T) * 8>(t);
+}
using namespace clang::ast_matchers;
@@ -29,64 +52,160 @@
namespace {
-/// Annotates how a mixup might happen. This is a flag enumeration.
+// Set the bit at index N to 1 as the enum constant. N = 0 is invalid.
+#define FBIT(Name, N) MIXUP_##Name = (1ull << (N##ull - 1ull))
+/// Annotates how a mixup might happen. This is a bit flag.
enum MixupTag : unsigned char {
MIXUP_Invalid = 0, //< Sentinel 0 bit pattern value for masking. DO NOT USE!
- MIXUP_None = 1, //< Mixup is not possible.
- MIXUP_Trivial = 2, //< No extra information needed.
- MIXUP_Typedef = 4, //< Argument of a typedef which resolves to an
+ FBIT(None, 1), //< Mixup is not possible.
+ FBIT(Trivial, 2), //< No extra information needed.
+ FBIT(Typedef, 3), //< Argument of a typedef which resolves to an
//< effective desugared type same as the other arg.
- MIXUP_RefBind = 8, //< Argument mixes with another due to reference binding.
- MIXUP_CVR = 16, //< Argument mixes with another through implicit
+ FBIT(RefBind, 4), //< Argument mixes with another due to reference binding.
+ FBIT(CVR, 5), //< Argument mixes with another through implicit
//< qualification.
+ FBIT(Implicit, 6), //< An implicit conversion may happen.
- LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ MIXUP_CVR)
+ LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ MIXUP_Implicit)
};
+#undef FBIT
LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
-using AdjacentParamsMixup =
- llvm::SmallVector<std::pair<const ParmVarDecl *, MixupTag>, 4>;
+/// Implicit conversion sequence steps resulting in types referred here.
+struct ConversionSequence {
+ /// Type of the intermediate value after the first standard conversion.
+ const Type *StdPre;
+ /// Type of the intermediate value after executing the user-defined conversion
+ /// which, in case of constructors, the user type, in case of conversion
+ /// operators, the result of the operator.
+ const Type *UserType;
+ /// Type of the intermediate value after the second standard conversion.
+ const Type *StdPost;
+
+ ConversionSequence() : StdPre(nullptr), UserType(nullptr), StdPost(nullptr) {}
+
+ explicit operator bool() const { return StdPre || UserType || StdPost; }
+
+ /// Whether the conversion sequence is single-step only.
+ bool single() const {
+ return ((bool)StdPre ^ (bool)UserType ^ (bool)StdPost) &&
+ !(StdPre && UserType && StdPost);
+ }
+};
-} // namespace
+struct MixupData {
+ MixupTag Flag;
+ ConversionSequence ConvLTR, ConvRTL;
-/// Attempts to strip away as many qualification, sugar and spice from the
-/// type as possible to reach the innermost purest the user eventually uses.
-/// This method attempts to discard everything as much as possible.
-static const Type *stripType(const Type *T);
+ MixupData(MixupTag Flags) : Flag(Flags) {}
+ MixupData(MixupTag Flags, const ConversionSequence &Seq)
+ : Flag(Flags), ConvLTR(Seq), ConvRTL(Seq) {}
+ MixupData(MixupTag Flags, const ConversionSequence <R,
+ const ConversionSequence &RTL)
+ : Flag(Flags), ConvLTR(LTR), ConvRTL(RTL) {}
-/// Returns whether an lvalue reference refers to the same type as T.
-static MixupTag refBindsToSameType(const LValueReferenceType *LRef,
- const Type *T, const bool CVRMixPossible);
+ MixupData operator|(MixupTag EnableFlag) const {
+ return {Flag | EnableFlag, ConvLTR, ConvRTL};
+ }
+ MixupData &operator|=(MixupTag EnableFlag) {
+ Flag |= EnableFlag;
+ return *this;
+ }
-/// Sanitises the MixupTag so it doesn't contain contradictory bits set.
-static MixupTag sanitiseMixup(MixupTag Tag) {
- assert(Tag != MIXUP_Invalid && "Mixup tag had full zero bit pattern value!");
+ /// Sanitises the mixup so it doesn't contain contradictory bits set.
+ void sanitise() {
+ assert(Flag != MIXUP_Invalid &&
+ "Mixup tag had full zero bit pattern value!");
- if (Tag & MIXUP_None)
- // If at any point the check recursion marks the mixup impossible, it is
- // just impossible.
- return MIXUP_None;
+ if (Flag & MIXUP_None) {
+ // If at any point the check recursion marks the mixup impossible, it is
+ // just impossible.
+ Flag = MIXUP_None;
+ return;
+ }
- if (Tag == MIXUP_Trivial)
- return Tag;
+ if (Flag == MIXUP_Trivial)
+ return;
- if (Tag ^ MIXUP_Trivial)
- // If any other bits than Trivial is set, unset Trivial, so only the
- // annotation bits warranting extra diagnostic are set.
- return Tag & ~MIXUP_Trivial;
+ if (Flag ^ MIXUP_Trivial)
+ // If any other bits than Trivial is set, unset Trivial, so only the
+ // annotation bits warranting extra diagnostic are set.
+ Flag &= ~MIXUP_Trivial;
- return Tag;
-}
+ // Set LTR and RTL implicity according to the members being set.
+ if (ConvLTR || ConvRTL)
+ Flag |= MIXUP_Implicit;
+ else
+ Flag &= ~MIXUP_Implicit;
+ }
+};
+
+/// Named tuple that contains that the types of the arguments From and To
+/// are mixable with the given flags in a particular fashion.
+struct Mixup {
+ const ParmVarDecl *From, *To;
+ MixupData Data;
+};
+static_assert(std::is_trivially_copyable<Mixup>::value,
+ "keep Mixup and components trivially copyable!");
+
+struct AdjacentParamsMixup {
+ std::size_t NumParamsChecked = 1;
+ llvm::SmallVector<Mixup, 8> Mixups;
+
+ const ParmVarDecl *getFirstParm() const {
+ // The first element added to the mixup vector has the left hand
+ // side set to the first argument in the range, if any is added.
+ assert(!Mixups.empty());
+ return Mixups.front().From;
+ }
-/// Returns whether LType and RType refer to the same type in a sense that at a
-/// call site it is possible to mix the types up if the actual arguments are
-/// specified in opposite order.
-/// \returns MixupTag indicating how a mixup between the arguments happens.
-/// The final output of this potentially recursive function must be sanitised.
-static MixupTag howPossibleToMixUpAtCallSite(const QualType LType,
- const QualType RType,
- const ASTContext &Ctx,
- const bool CVRMixPossible) {
+ const ParmVarDecl *getLastParm() const {
+ // There builder function paramEqualTypeRange breaks building instance of
+ // this type if it finds something that can not be mixed up, by going
+ // *forward* in the list of arguments. So at the moment of break, the right
+ // hand side of the last conversion in the vector is the last argument in
+ // the adjacency range.
+ assert(!Mixups.empty());
+ return Mixups.back().To;
+ }
+};
+
+} // namespace
+
+/// Attempts to strip away as many qualification, sugar and spice from the
+/// type as possible to reach the innermost purest the user eventually uses.
+/// This method attempts to discard everything as much as possible.
+static const Type *stripType(const Type *T);
+
+/// Returns how an lvalue reference refers to the same type as T.
+static MixupData refBindsToSameType(const LValueReferenceType *LRef,
+ const Type *T, const ASTContext &Ctx,
+ const bool IsRefRightType,
+ const bool CVRMixPossible,
+ const bool ImplicitConversion);
+
+/// Returns whether the left side type is convertible to the right side type,
+/// by attempting to approximate implicit conversion sequences.
+/// \param AllowUserDefined If false, only standard conversions will be
+/// approximated.
+/// \note The result of this operation is not symmetric!
+static MixupData howConvertible(const Type *LT, const Type *RT,
+ const LangOptions &LOpts,
+ bool AllowUserDefined = true);
+
+/// Returns how LType and RType may essentially refer to the same type - in a
+/// sense that at a call site it is possible to mix the arguments up if
+/// specified in the opposite order.
+/// \returns MixupData indicating how a mixup between the arguments happens.
+/// \note The final output of this potentially recursive function must be
+/// sanitised by 'sanitiseMixup' before it could be used, to ensure only the
+/// proper bits are set!
+static MixupData howPossibleToMixUpAtCallSite(const QualType LType,
+ const QualType RType,
+ const ASTContext &Ctx,
+ const bool CVRMixPossible,
+ const bool ImplicitConversion) {
LLVM_DEBUG(llvm::dbgs() << "isPossibleToMixUpAtCallSite?\n Left:";
LType.dump(llvm::dbgs()); llvm::dbgs() << "\n\t and \n Right: ";
RType.dump(llvm::dbgs()); llvm::dbgs() << '\n';);
@@ -97,12 +216,69 @@
}
// Remove certain sugars that don't affect mixability from the types.
- if (dyn_cast<const ParenType>(LType.getTypePtr()))
+ if (dyn_cast<const ParenType>(LType.getTypePtr())) {
+ LLVM_DEBUG(llvm::dbgs() << "Left is some sugared Parens()\n");
return howPossibleToMixUpAtCallSite(LType.getSingleStepDesugaredType(Ctx),
- RType, Ctx, CVRMixPossible);
- if (dyn_cast<const ParenType>(RType.getTypePtr()))
+ RType, Ctx, CVRMixPossible,
+ ImplicitConversion);
+ }
+ if (dyn_cast<const ParenType>(RType.getTypePtr())) {
+ LLVM_DEBUG(llvm::dbgs() << "Right is some sugared Parens()\n");
return howPossibleToMixUpAtCallSite(
- LType, RType.getSingleStepDesugaredType(Ctx), Ctx, CVRMixPossible);
+ LType, RType.getSingleStepDesugaredType(Ctx), Ctx, CVRMixPossible,
+ ImplicitConversion);
+ }
+
+ // An argument of type 'T' and 'const T &' may bind with the same power.
+ // (Note this is a different case, as 'const T &' is a '&' on the top level,
+ // and only then a const.)
+ if (LType->isLValueReferenceType() || RType->isLValueReferenceType()) {
+ // (If both is the same reference type, earlier a return happened.)
+
+ if (LType->isLValueReferenceType()) {
+ LLVM_DEBUG(llvm::dbgs() << "!!! Left is an lvalue reference type\n";);
+ // Return value of function call may or may not have given us a tag (e.g.
+ // reference was to a typedef) via a recursive chain back to this
+ // function. Apply the "bind power" tag here to indicate a reference
+ // binding happened. (If RefBind was MIXUP_None, a later sanitise step
+ // will undo every bit except for None.)
+ return refBindsToSameType(LType->getAs<LValueReferenceType>(),
+ RType.getTypePtr(), Ctx,
+ /* IsRefRightType =*/false, CVRMixPossible,
+ ImplicitConversion) |
+ MIXUP_RefBind;
+ }
+ if (RType->isLValueReferenceType()) {
+ LLVM_DEBUG(llvm::dbgs() << "!!! Right is an lvalue reference type.\n";);
+ return refBindsToSameType(RType->getAs<LValueReferenceType>(),
+ LType.getTypePtr(), Ctx,
+ /* IsRefRightType =*/true, CVRMixPossible,
+ ImplicitConversion) |
+ MIXUP_RefBind;
+ }
+ }
+
+ // An argument of type 'T' and 'const T' may bind with the same power.
+ // Case for both types being const qualified (for the same type) is handled
+ // by LType == RType.
+ if (LType.getLocalCVRQualifiers() || RType.getLocalCVRQualifiers()) {
+ LLVM_DEBUG(
+ llvm::dbgs()
+ << "!!! Left or right is locally CVR: Left: "
+ << format_bytes(
+ (unsigned char)LType.getLocalQualifiers().getCVRQualifiers())
+ << ", Right: "
+ << format_bytes(
+ (unsigned char)RType.getLocalQualifiers().getCVRQualifiers())
+ << '\n');
+ if (!CVRMixPossible)
+ return MIXUP_None;
+
+ return howPossibleToMixUpAtCallSite(LType.getUnqualifiedType(),
+ RType.getUnqualifiedType(), Ctx,
+ CVRMixPossible, ImplicitConversion) |
+ MIXUP_CVR;
+ }
{
const auto *LTypedef = LType->getAs<TypedefType>();
@@ -110,94 +286,87 @@
if (LTypedef && RTypedef) {
LLVM_DEBUG(llvm::dbgs()
<< "!!! Both Left and Right are typedef types.\n";);
- return MIXUP_Typedef | howPossibleToMixUpAtCallSite(LTypedef->desugar(),
- RTypedef->desugar(),
- Ctx, CVRMixPossible);
+ return howPossibleToMixUpAtCallSite(LTypedef->desugar(),
+ RTypedef->desugar(), Ctx,
+ CVRMixPossible, ImplicitConversion) |
+ MIXUP_Typedef;
}
if (LTypedef) {
LLVM_DEBUG(llvm::dbgs() << "!!! Left is typedef type.\n";);
- return MIXUP_Typedef |
- howPossibleToMixUpAtCallSite(LTypedef->desugar(), RType, Ctx,
- CVRMixPossible);
+ return howPossibleToMixUpAtCallSite(LTypedef->desugar(), RType, Ctx,
+ CVRMixPossible, ImplicitConversion) |
+ MIXUP_Typedef;
}
if (RTypedef) {
LLVM_DEBUG(llvm::dbgs() << "!!! Right is typedef type.\n";);
- return MIXUP_Typedef |
- howPossibleToMixUpAtCallSite(LType, RTypedef->desugar(), Ctx,
- CVRMixPossible);
+ return howPossibleToMixUpAtCallSite(LType, RTypedef->desugar(), Ctx,
+ CVRMixPossible, ImplicitConversion) |
+ MIXUP_Typedef;
}
}
if (LType->isPointerType() && RType->isPointerType()) {
// (Both types being the exact same pointer is handled by LType == RType.)
+ // The implicit conversion between any T* and U* possible in C is ignored,
+ // as virtually all compilers emit a warning if the conversion is done at a
+ // call site.
LLVM_DEBUG(llvm::dbgs() << "!!! Both Left and Right are pointer types.\n";);
- MixupTag Mixup = howPossibleToMixUpAtCallSite(
- LType->getPointeeType(), RType->getPointeeType(), Ctx, CVRMixPossible);
-
- if (LType.isLocalConstQualified() || LType.isLocalVolatileQualified() ||
- LType.isLocalRestrictQualified() || RType.isLocalConstQualified() ||
- RType.isLocalVolatileQualified() || RType.isLocalRestrictQualified()) {
- // A qualified ptr might be mixed up with an unqualified one at call, but
- // this is up to user configuration to report about.
- if (CVRMixPossible)
- Mixup |= MIXUP_CVR;
- else
- Mixup |= MIXUP_None;
- }
- return Mixup;
- }
+ // Implicit conversions of two pointers should be diagnosed if they point to
+ // C++ records to find Derived-To-Base implicit casts.
+ // For non-user-types, do not check - giving an unrelated, e.g. "long *" in
+ // place of an "int *" is diagnosed anyways by warnings or errors.
+ bool ShouldDiagnoseImplicitBehindPtr = ImplicitConversion &&
+ LType->getPointeeCXXRecordDecl() &&
+ RType->getPointeeCXXRecordDecl();
+ LLVM_DEBUG(
+ llvm::dbgs()
+ << "Checking mixup between pointed types. Allowing implicit modelling? "
+ << ShouldDiagnoseImplicitBehindPtr << "\nOriginally the user specified "
+ << ImplicitConversion << ".\n");
- // An argument of type 'T' and 'const T' may bind with the same power.
- // Case for both types being const qualified (for the same type) is handled
- // by LType == RType.
- if (CVRMixPossible &&
- (LType.isLocalConstQualified() || LType.isLocalVolatileQualified())) {
- LLVM_DEBUG(llvm::dbgs() << "!!! Left is CV qualified.";
- LType.dump(llvm::dbgs()); llvm::dbgs() << '\n';);
- return MIXUP_CVR | howPossibleToMixUpAtCallSite(LType.getUnqualifiedType(),
- RType, Ctx, CVRMixPossible);
- }
- if (CVRMixPossible &&
- (RType.isLocalConstQualified() || RType.isLocalVolatileQualified())) {
- LLVM_DEBUG(llvm::dbgs() << "!!! Right is CV qualified\n";
- RType.dump(llvm::dbgs()); llvm::dbgs() << '\n';);
- return MIXUP_CVR |
- howPossibleToMixUpAtCallSite(LType, RType.getUnqualifiedType(), Ctx,
- CVRMixPossible);
+ return howPossibleToMixUpAtCallSite(
+ LType->getPointeeType(), RType->getPointeeType(), Ctx, CVRMixPossible,
+ ShouldDiagnoseImplicitBehindPtr);
}
- // An argument of type 'T' and 'const T &' may bind with the same power.
- // (Note this is a different case, as 'const T &' is a '&' on the top level,
- // and only then a const.)
- if (LType->isLValueReferenceType() || RType->isLValueReferenceType()) {
- // (If both is the same reference type, earlier a return happened.)
-
- if (LType->isLValueReferenceType()) {
- LLVM_DEBUG(llvm::dbgs() << "!!! Left is an lvalue reference type\n";);
- MixupTag RefBind = refBindsToSameType(LType->getAs<LValueReferenceType>(),
- RType.getTypePtr(), CVRMixPossible);
- // RefBind may or may not have given us a tag (e.g. reference was to a
- // typedef) via a recursive chain back to this function. Apply the
- // "bind power" tag here to indicate a reference binding happened.
- // (If RefBind was MIXUP_None, a later sanitise step will undo every bit
- // except for None.)
- return RefBind | MIXUP_RefBind;
- }
- if (RType->isLValueReferenceType()) {
- LLVM_DEBUG(llvm::dbgs() << "!!! Right is an lvalue reference type.\n";);
- MixupTag RefBind = refBindsToSameType(RType->getAs<LValueReferenceType>(),
- LType.getTypePtr(), CVRMixPossible);
- return RefBind | MIXUP_RefBind;
- }
+ if (ImplicitConversion) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "Implicit conversion check has been enabled! Checking...\n");
+
+ const Type *LT = LType.getTypePtr();
+ const Type *RT = RType.getTypePtr();
+
+ // Try approximating an implicit conversion sequence.
+ LLVM_DEBUG(llvm::dbgs() << "?????????? Check Left->Right ..............\n");
+ MixupData LTR =
+ howConvertible(LT, RT, Ctx.getLangOpts(), /* AllowUserDefined =*/true);
+ LLVM_DEBUG(llvm::dbgs()
+ << "Left->Right flags: " << format_bytes(LTR.Flag) << '\n');
+ if (LTR.Flag != MIXUP_None)
+ LLVM_DEBUG(llvm::dbgs() << "Left -> Right mixup\n");
+
+ LLVM_DEBUG(llvm::dbgs() << "?????????? Check Right->Left ..............\n");
+ MixupData RTL =
+ howConvertible(RT, LT, Ctx.getLangOpts(), /* AllowUserDefined =*/true);
+ LLVM_DEBUG(llvm::dbgs()
+ << "Left<-Right flags: " << format_bytes(RTL.Flag) << '\n');
+ if (RTL.Flag != MIXUP_None)
+ LLVM_DEBUG(llvm::dbgs() << "Left <- Right mixup\n");
+
+ if (LTR.ConvLTR || RTL.ConvRTL)
+ return {MIXUP_Implicit, LTR.ConvLTR, RTL.ConvRTL};
}
- LLVM_DEBUG(llvm::dbgs() << "<<< End of logic reached, types don't match.";);
+ LLVM_DEBUG(llvm::dbgs() << "<<< End of logic reached, types don't match.\n";);
return MIXUP_None;
}
-static MixupTag refBindsToSameType(const LValueReferenceType *LRef,
- const Type *T, const bool CVRMixPossible) {
+static MixupData refBindsToSameType(const LValueReferenceType *LRef,
+ const Type *T, const ASTContext &Ctx,
+ const bool IsRefRightType,
+ const bool CVRMixPossible,
+ const bool ImplicitConversion) {
LLVM_DEBUG(llvm::dbgs() << "refBindsToSameType?\n"; LRef->dump(llvm::dbgs());
llvm::dbgs() << "\n\t and \n"; T->dump(llvm::dbgs());
llvm::dbgs() << '\n';);
@@ -212,18 +381,379 @@
return MIXUP_None;
}
- if (const auto *TypedefTy = ReferredType.getTypePtr()->getAs<TypedefType>()) {
+ if (const auto *TypedefTy = ReferredType->getAs<TypedefType>()) {
// If the referred type is a typedef, try checking the mixup-chance on the
// desugared type.
LLVM_DEBUG(llvm::dbgs() << "Reference to a typedef, calling back...\n";);
- return howPossibleToMixUpAtCallSite(TypedefTy->desugar(), QualType{T, 0},
- TypedefTy->getDecl()->getASTContext(),
- CVRMixPossible);
+ if (!IsRefRightType)
+ return howPossibleToMixUpAtCallSite(TypedefTy->desugar(), QualType{T, 0},
+ Ctx, CVRMixPossible,
+ ImplicitConversion);
+ return howPossibleToMixUpAtCallSite(QualType{T, 0}, TypedefTy->desugar(),
+ Ctx, CVRMixPossible,
+ ImplicitConversion);
}
LLVM_DEBUG(llvm::dbgs() << "is referred type type_ptr() and T the same? "
<< (ReferredType.getTypePtr() == T) << '\n';);
- return ReferredType.getTypePtr() == T ? MIXUP_Trivial : MIXUP_None;
+ if (ReferredType.getTypePtr() == T)
+ return MIXUP_Trivial;
+ if (ImplicitConversion) {
+ // Try to see if the reference can be bound through an implicit conversion.
+ if (!IsRefRightType)
+ return howPossibleToMixUpAtCallSite(ReferredType.getUnqualifiedType(),
+ QualType{T, 0}, Ctx, CVRMixPossible,
+ ImplicitConversion);
+ return howPossibleToMixUpAtCallSite(QualType{T, 0},
+ ReferredType.getUnqualifiedType(), Ctx,
+ CVRMixPossible, ImplicitConversion);
+ }
+ return MIXUP_None;
+}
+
+static inline bool isNumericConvertible(const Type *LT, const Type *RT) {
+ LLVM_DEBUG(llvm::dbgs() << "isNumericConvertible(" << LT << ", " << RT
+ << ")\n");
+ if (!LT || !RT)
+ return false;
+
+ bool LI = LT->isIntegerType();
+ bool LF = LT->isFloatingType();
+ bool RI = RT->isIntegerType();
+ bool RF = RT->isFloatingType();
+
+ // Through promotions and conversions, there is free passage between
+ // "integer types" and "floating types".
+ // Promotions don't change value, conversions may result in value or precision
+ // loss, but for the sake of easy understanding, we put the two under
+ // one umbrella.
+ return (LI && RI) || (LF && RF) || (LI && RF) || (LF && RI);
+}
+
+/// Returns a Mixup indicating if the specified converting constructor can be
+/// called with LT or after applying a standard conversion sequence on LT.
+static MixupData tryConvertingConstructor(const Type *LT,
+ const CXXRecordDecl *RD,
+ const CXXConstructorDecl *Conv,
+ const LangOptions &LOpts) {
+ const ParmVarDecl *ConArg = Conv->getParamDecl(0);
+ MixupData Mix = howPossibleToMixUpAtCallSite(
+ QualType{LT, 0}, ConArg->getType(), Conv->getASTContext(),
+ // Model qualifier mixing generously.
+ /* CVRMixPossible =*/true,
+ // Do not allow any further implicit conversions.
+ /* ImplicitConversion =*/false);
+ LLVM_DEBUG(llvm::dbgs() << "Conv no-std (pre sanitise): "
+ << format_bytes(Mix.Flag) << '\n');
+ Mix.sanitise();
+ LLVM_DEBUG(llvm::dbgs() << "Conv no-std (post sanitise): "
+ << format_bytes(Mix.Flag) << '\n');
+ if (Mix.Flag != MIXUP_None) {
+ // If simply applying the user-defined constructor works as is, this is
+ // the right constructor.
+ LLVM_DEBUG(llvm::dbgs() << "Conversion constructor matches exactly.\n");
+ ConversionSequence Seq;
+ Seq.UserType = RD->getTypeForDecl();
+ Mix.ConvLTR = Seq;
+ return Mix;
+ }
+
+ // Otherwise, see if LT and the converting constructor's argument can be
+ // matched via a standard conversion before. This will take care of converting
+ // an int to another type.
+ ConversionSequence Seq;
+ MixupData Pre = howConvertible(LT, ConArg->getType().getTypePtr(), LOpts,
+ /* AllowUserDefined =*/false);
+ LLVM_DEBUG(llvm::dbgs() << "Pre-UD: " << format_bytes(Pre.Flag)
+ << " to type \n";
+ Pre.ConvLTR.StdPre->dump(llvm::dbgs()); llvm::dbgs() << '\n');
+ if (Pre.Flag == MIXUP_None || Pre.Flag == MIXUP_Trivial)
+ // If not possible to mix, don't check. If trivial, previous case should
+ // have returned already.
+ return MIXUP_None;
+ Seq.StdPre = Pre.ConvLTR.StdPre;
+
+ LLVM_DEBUG(llvm::dbgs()
+ << "Conversion constructor matches after standard conversion.\n");
+
+ Mix = howPossibleToMixUpAtCallSite(
+ QualType{Seq.StdPre, 0}, ConArg->getType(), Conv->getASTContext(),
+ // Model qualifier mixing generously.
+ /* CVRMixPossible =*/true,
+ // Do not allow any further implicit conversions.
+ /* ImplicitConversion =*/false);
+ LLVM_DEBUG(llvm::dbgs() << "Conv (pre sanitise): " << format_bytes(Mix.Flag)
+ << '\n');
+ Mix.sanitise();
+ LLVM_DEBUG(llvm::dbgs() << "Conv (post sanitise): " << format_bytes(Mix.Flag)
+ << '\n');
+ if (Mix.Flag != MIXUP_None) {
+ LLVM_DEBUG(llvm::dbgs() << "Conversion constructor matches!\n");
+ Seq.UserType = RD->getTypeForDecl();
+ Mix.ConvLTR = Seq;
+ return Mix;
+ }
+
+ return MIXUP_None;
+}
+
+static MixupData tryConversionOperator(const CXXConversionDecl *Conv,
+ const Type *RT,
+ const LangOptions &LOpts) {
+ MixupData Mix = howPossibleToMixUpAtCallSite(
+ Conv->getConversionType(), QualType{RT, 0}, Conv->getASTContext(),
+ // Model qualifier mixing generously.
+ /* CVRMixPossible =*/true,
+ // Do not allow any further implicit conversions.
+ /* ImplicitConversion =*/false);
+ LLVM_DEBUG(llvm::dbgs() << "Conv no-std (pre sanitise): "
+ << format_bytes(Mix.Flag) << '\n');
+ Mix.sanitise();
+ LLVM_DEBUG(llvm::dbgs() << "Conv no-std (post sanitise): "
+ << format_bytes(Mix.Flag) << '\n');
+ if (Mix.Flag != MIXUP_None) {
+ // If simply applying the user-defined operator works as is, this is
+ // the right conversion operator.
+ ConversionSequence Seq;
+ Seq.UserType = RT;
+ Mix.ConvLTR = Seq;
+ return Mix;
+ }
+
+ // Otherwise, see if the converting operator's result and RT can be
+ // matched via a standard conversion after. This will take care of converting
+ // a result of float to another type.
+ ConversionSequence Seq;
+ MixupData Post = howConvertible(Conv->getConversionType().getTypePtr(), RT,
+ LOpts, /* AllowUserDefined =*/false);
+ LLVM_DEBUG(llvm::dbgs() << "Post-UD: " << format_bytes(Post.Flag)
+ << " to type \n";
+ Post.ConvLTR.StdPre->dump(llvm::dbgs()); llvm::dbgs() << '\n');
+ if (Post.Flag == MIXUP_None || Post.Flag == MIXUP_Trivial)
+ // If not possible to mix, don't check. If trivial, previous case should
+ // have returned already.
+ return MIXUP_None;
+ // Save the result type of the user-defined conversion operator.
+ Seq.UserType = Conv->getConversionType().getTypePtr();
+
+ // The unqualified version of the conversion operator's return type can be
+ // converted to the "needed" Right-Type (output type wanted by the caller
+ // of this method). Now see if between we lost any qualifiers.
+ Mix = howPossibleToMixUpAtCallSite(
+ Conv->getConversionType(),
+ QualType{Conv->getConversionType().getTypePtr(), 0},
+ Conv->getASTContext(),
+ // Model qualifier mixing generously.
+ /* CVRMixPossible =*/true,
+ // Do not allow any further implicit conversions.
+ /* ImplicitConversion =*/false);
+ LLVM_DEBUG(llvm::dbgs() << "Conv (pre sanitise): " << format_bytes(Mix.Flag)
+ << '\n');
+ Mix.sanitise();
+ LLVM_DEBUG(llvm::dbgs() << "Conv (post sanitise): " << format_bytes(Mix.Flag)
+ << '\n');
+ if (Mix.Flag != MIXUP_None) {
+ LLVM_DEBUG(llvm::dbgs() << "Conversion operator matches!\n");
+ Seq.StdPost = RT;
+ Mix.ConvLTR = Seq;
+ return Mix;
+ }
+
+ return MIXUP_None;
+}
+
+static MixupData howConvertible(const Type *LT, const Type *RT,
+ const LangOptions &LOpts,
+ bool AllowUserDefined) {
+ LLVM_DEBUG(llvm::dbgs() << ">>>>> how convertible? \n";
+ LT->dump(llvm::dbgs()); llvm::dbgs() << '\n';
+ RT->dump(llvm::dbgs()); llvm::dbgs() << '\n');
+ if (LT == RT) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "Left and right is the same, returning Trivial.\n");
+ return MIXUP_Trivial;
+ }
+
+ ConversionSequence Seq;
+
+ const auto *LBT = LT->getAs<BuiltinType>();
+ const auto *RBT = RT->getAs<BuiltinType>();
+ if (LBT && RBT && LBT == RBT)
+ return MIXUP_Trivial;
+ if (isNumericConvertible(LBT, RBT)) {
+ // Builtin numerical types are back-and-forth convertible.
+ LLVM_DEBUG(llvm::dbgs() << "Builtin conversion between numerics.\n");
+ Seq.StdPre = RBT;
+ return {MIXUP_Implicit, Seq};
+ }
+
+ const auto *LET = LT->getAs<EnumType>();
+ const auto *RET = RT->getAs<EnumType>();
+ if (LET && !LET->isScopedEnumeralType() && RBT &&
+ (RBT->isIntegerType() || RBT->isFloatingType())) {
+ // Enum can convert to underlying integer type.
+ LLVM_DEBUG(llvm::dbgs() << "Enum -> integer/float.\n");
+ Seq.StdPre = RBT;
+ return {MIXUP_Implicit, Seq};
+ }
+ if (LBT && (LBT->isFloatingType() || LBT->isIntegerType()) && RET) {
+ // Enum cannot be constructed from any builtin type (neither int, nor
+ // float).
+ if (LOpts.CPlusPlus) {
+ LLVM_DEBUG(llvm::dbgs() << "Numeric -/-> Enum NOT POSSIBLE (C++).\n");
+ return MIXUP_None;
+ }
+ // In C, you can go back and forth between numeric types.
+ Seq.StdPre = RET;
+ return {MIXUP_Implicit, Seq};
+ }
+
+ if (LOpts.CPlusPlus) {
+ // We are checking Left -> Right mixup, in which case Left <: Right should
+ // hold for DerivedToBase implicit cast. This is a standard implicit
+ // conversion.
+ const auto *LCXXRec = LT->getAsCXXRecordDecl();
+ const auto *RCXXRec = RT->getAsCXXRecordDecl();
+ LLVM_DEBUG(llvm::dbgs() << "Left as record: " << LCXXRec
+ << ", Right as record: " << RCXXRec << '\n');
+ if (LCXXRec && RCXXRec && LCXXRec->isCompleteDefinition() &&
+ RCXXRec->isCompleteDefinition() &&
+ (LCXXRec->isDerivedFrom(RCXXRec) ||
+ LCXXRec->isVirtuallyDerivedFrom(RCXXRec))) {
+ LLVM_DEBUG(llvm::dbgs() << "Left <: Right, giving standard DerivedToBase "
+ "cast Left->Right.\n");
+ Seq.StdPre = RT;
+ return {MIXUP_Implicit, Seq};
+ }
+ }
+
+ LLVM_DEBUG(llvm::dbgs() << "Before checking user defined...\n");
+
+ if (!LOpts.CPlusPlus || !AllowUserDefined)
+ // User-defined conversions are only sensible in C++ mode.
+ return MIXUP_None;
+ assert(!Seq && "Non-user defined conversion check should've returned.");
+
+ LLVM_DEBUG(llvm::dbgs() << "Checking for user-defined conversions...\n");
+ MixupTag UserMixup = MIXUP_Invalid;
+ bool FoundConvertingCtor = false, FoundConversionOperator = false;
+ bool FoundMultipleMatches = false;
+ // An implicit conversion sequence may only contain at most *one* user-defined
+ // conversion.
+ if (const auto *RRT = RT->getAs<RecordType>()) {
+ LLVM_DEBUG(llvm::dbgs() << "Right hand side is a record type.\n");
+ const auto *RD = RRT->getAsCXXRecordDecl();
+ if (!RD)
+ // Initialisation of C-style record types from an unrelated type is not
+ // possible.
+ return MIXUP_None;
+ if (!RD->isCompleteDefinition())
+ // Incomplete definition means we don't know anything about members.
+ return MIXUP_None;
+
+ for (const CXXConstructorDecl *Con : RD->ctors()) {
+ if (Con->isCopyOrMoveConstructor() ||
+ !Con->isConvertingConstructor(/* AllowExplicit =*/false))
+ continue;
+ LLVM_DEBUG(llvm::dbgs() << "Found a converting constructor? \n";
+ Con->dump(llvm::dbgs()); llvm::dbgs() << '\n');
+
+ MixupData ConvertingResult = tryConvertingConstructor(LT, RD, Con, LOpts);
+ if (ConvertingResult.Flag == MIXUP_None)
+ continue;
+
+ LLVM_DEBUG(llvm::dbgs() << "Came back with a positive result.\n");
+ FoundConvertingCtor = true;
+
+ if (!ConvertingResult.ConvLTR.StdPre) {
+ // A match without a pre-conversion was found for the converting ctor.
+ LLVM_DEBUG(llvm::dbgs() << "! EXACT MATCH.\n");
+ UserMixup |= ConvertingResult.Flag;
+ Seq = ConvertingResult.ConvLTR;
+ FoundMultipleMatches = false;
+ break;
+ }
+
+ if (!FoundMultipleMatches && Seq) {
+ // If the loop executes multiple times, it's possible to find multiple
+ // converting constructors with different, ambiguous conversion
+ // sequences leading up to them. This is an error, but we need to check
+ // all constructors first for a possible exact match.
+ FoundMultipleMatches = true;
+ continue;
+ }
+
+ // Register the match.
+ UserMixup |= ConvertingResult.Flag;
+ Seq = ConvertingResult.ConvLTR;
+ }
+ }
+
+ if (const auto *LRT = LT->getAs<RecordType>()) {
+ LLVM_DEBUG(llvm::dbgs() << "Left hand side is a record type.\n");
+ const auto *RD = LRT->getAsCXXRecordDecl();
+ if (!RD)
+ // Initialisation of things from an unrelated C-style record type is not
+ // possible.
+ return MIXUP_None;
+ if (!RD->isCompleteDefinition())
+ // Against getVisibleConversionFunctions() assert.
+ return MIXUP_None;
+
+ for (const NamedDecl *Method : RD->getVisibleConversionFunctions()) {
+ LLVM_DEBUG(llvm::dbgs() << "Found visible conv operator: \n";
+ Method->dump(llvm::dbgs()); llvm::dbgs() << '\n';);
+
+ const auto *Con = dyn_cast<CXXConversionDecl>(Method);
+ if (!Con || Con->isExplicit())
+ continue;
+ LLVM_DEBUG(llvm::dbgs() << "Found a conversion method? \n";
+ Con->dump(llvm::dbgs()); llvm::dbgs() << '\n');
+
+ MixupData ConvertingResult = tryConversionOperator(Con, RT, LOpts);
+ if (ConvertingResult.Flag == MIXUP_None)
+ continue;
+
+ LLVM_DEBUG(llvm::dbgs() << "Came back with a positive result.\n");
+ FoundConversionOperator = true;
+
+ if (!ConvertingResult.ConvLTR.StdPost) {
+ // A match without a pre-conversion was found for the converting oper.
+ LLVM_DEBUG(llvm::dbgs() << "! EXACT MATCH.\n");
+ UserMixup |= ConvertingResult.Flag;
+ Seq = ConvertingResult.ConvLTR;
+ FoundMultipleMatches = false;
+ break;
+ }
+
+ if (!FoundMultipleMatches && Seq) {
+ // If the loop executes multiple times, it's possible to find multiple
+ // conversion operators with different, ambiguous conversion
+ // sequences coming from them. This is an error, but we need to check
+ // all constructors first for a possible exact match.
+ FoundMultipleMatches = true;
+ continue;
+ }
+
+ // Register the match.
+ UserMixup |= ConvertingResult.Flag;
+ Seq = ConvertingResult.ConvLTR;
+ }
+ }
+
+ if (FoundMultipleMatches) {
+ LLVM_DEBUG(llvm::dbgs() << "Multiple conversion sequences found! :(\n");
+ return MIXUP_None;
+ }
+ if (FoundConvertingCtor && FoundConversionOperator) {
+ LLVM_DEBUG(llvm::dbgs() << "Left<->Right ambiguous as both can be "
+ "converted to each other...\n");
+ return MIXUP_None;
+ }
+
+ LLVM_DEBUG(llvm::dbgs() << "Flags at the end of 'howConvertible?': "
+ << format_bytes(UserMixup) << '\n');
+ return Seq ? MixupData{UserMixup, Seq} : MixupData{MIXUP_None};
}
/// Gets the type-equal range of the parameters of F starting with the param
@@ -233,46 +763,77 @@
/// reasoning behind mixing up the nth parameter with the first in the adjacent
/// range.
static AdjacentParamsMixup paramEqualTypeRange(const FunctionDecl *F,
- unsigned int i,
- const bool CVRMixPossible) {
- assert(i <= F->getNumParams() && "invalid index given");
-
+ unsigned int StartIdx,
+ const bool CVRMixPossible,
+ const bool ImplicitConversion) {
AdjacentParamsMixup APR;
- const ParmVarDecl *First = F->getParamDecl(i);
- LLVM_DEBUG(llvm::dbgs() << "Start " << i << "th param:\n";
- First->dump(llvm::dbgs()); llvm::dbgs() << '\n';);
- APR.emplace_back(First, MIXUP_Trivial);
- const ASTContext &Ctx = F->getASTContext();
const unsigned int ParamCount = F->getNumParams();
+ assert(StartIdx < ParamCount && "invalid start index given!");
+ const ASTContext &Ctx = F->getASTContext();
+
+ const auto CalcMixupBetween = [&Ctx, CVRMixPossible,
+ ImplicitConversion](const ParmVarDecl *A,
+ const ParmVarDecl *B) {
+ MixupData MD = howPossibleToMixUpAtCallSite(
+ A->getType(), B->getType(), Ctx, CVRMixPossible, ImplicitConversion);
+ LLVM_DEBUG(llvm::dbgs() << "\tMixup (before sanitise)? "
+ << format_bytes(MD.Flag) << "\n";);
+ MD.sanitise();
+ LLVM_DEBUG(llvm::dbgs() << "\tMixup (after sanitise)? "
+ << format_bytes(MD.Flag) << "\n";);
+ assert(MD.Flag != MIXUP_Invalid &&
+ "Bits fell off, result is sentinel value.");
+
+ return Mixup{A, B, MD};
+ };
+
+ const ParmVarDecl *First = F->getParamDecl(StartIdx);
+ LLVM_DEBUG(llvm::dbgs() << "Start at " << StartIdx << "th param:\n";
+ First->dump(llvm::dbgs()); llvm::dbgs() << '\n';);
+
// FIXME: Make this method get the minimum length required and do not build
// ranges that are shorter than requirement - they'd be discarded anyways.
- for (++i; i < ParamCount; ++i) {
+ for (unsigned int i = StartIdx + 1; i < ParamCount; ++i) {
const ParmVarDecl *Ith = F->getParamDecl(i);
LLVM_DEBUG(llvm::dbgs() << "Check " << i << "th param:\n";
Ith->dump(llvm::dbgs()); llvm::dbgs() << '\n';);
- MixupTag Mixup = howPossibleToMixUpAtCallSite(
- First->getType(), Ith->getType(), Ctx, CVRMixPossible);
- LLVM_DEBUG(llvm::dbgs() << "\tMixup? " << static_cast<int>(Mixup) << "\n";);
- Mixup = sanitiseMixup(Mixup);
- LLVM_DEBUG(llvm::dbgs() << "\tMixup (after sanitise)? "
- << static_cast<int>(Mixup) << "\n";);
- if (Mixup == MIXUP_None)
- break;
+ if (!ImplicitConversion) {
+ // If implicit conversion check is not enabled, the mixup logic is easy,
+ // as we only need to check each subsequent argument against the first one
+ // of the range - mixes between elements of the range can be inferred
+ // from a mix with the first one, as only references, qualifiers, and
+ // typedefs are resolved.
+ Mixup M = CalcMixupBetween(First, Ith);
+ if (M.Data.Flag == MIXUP_None)
+ // If no mixup is possible, we are at the end of the current range.
+ break;
+ APR.Mixups.emplace_back(M);
+ } else {
+ // Calculate mixes with *each* argument in the range. Mixup is calculated
+ // bidirectional, so only consider the prefix of the currently built
+ // range.
+ bool AnyMixupStored = false;
+ for (unsigned int j = StartIdx; j < i; ++j) {
+ const ParmVarDecl *Jth = F->getParamDecl(j);
+ LLVM_DEBUG(llvm::dbgs() << "Check " << j << "th param:\n";
+ Jth->dump(llvm::dbgs()); llvm::dbgs() << '\n';);
+ Mixup M = CalcMixupBetween(Jth, Ith);
+ if (M.Data.Flag != MIXUP_None) {
+ AnyMixupStored = true;
+ APR.Mixups.emplace_back(M);
+ }
+ }
- APR.emplace_back(Ith, Mixup);
- }
+ if (!AnyMixupStored)
+ // If there is no any "new" mixup possibility for the Ith param, it
+ // signifies the end of range.
+ break;
+ }
- if (APR.size() > 1 &&
- llvm::any_of(APR, [](const AdjacentParamsMixup::value_type &E) {
- return E.second & MIXUP_Typedef;
- })) {
- // If any of the mixups for argument pairs (1, 2), (1, 3), ... involve a
- // typedef match, the first argument might be a typedef too. Set a bit to
- // emit a "type alias resolution" diagnostic for the first argument of the
- // range.
- APR.front().second = MIXUP_Typedef;
+ // If the loop did not break earlier, another param was found to be mixable.
+ ++APR.NumParamsChecked;
}
return APR;
@@ -400,6 +961,21 @@
}
}
+static std::string typeNameAsString(const QualType QT,
+ const PrintingPolicy &PP) {
+ std::string Ret;
+
+ {
+ llvm::raw_string_ostream OS{Ret};
+ putTypeName(QT, OS, PP);
+ }
+
+ if (Ret.empty())
+ Ret = "<unprintable type?>";
+
+ return Ret;
+}
+
// For convenience, ignore a few well-known parameter variable and type names
// which usually show up with repeated parameters.
// FIXME: Make this configurable from the checker options.
@@ -487,12 +1063,14 @@
: ClangTidyCheck(Name, Context),
MinimumLength(
clampMinimumLength(Options.get("MinimumLength", DefaultMinLength))),
- CVRMixPossible(Options.get("CVRMixPossible", false)) {}
+ CVRMixPossible(Options.get("CVRMixPossible", false)),
+ ImplicitConversion(Options.get("ImplicitConversion", false)) {}
void AdjacentArgumentsOfSameTypeCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "MinimumLength", MinimumLength);
Options.store(Opts, "CVRMixPossible", CVRMixPossible);
+ Options.store(Opts, "ImplicitConversion", ImplicitConversion);
}
void AdjacentArgumentsOfSameTypeCheck::registerMatchers(MatchFinder *Finder) {
@@ -536,10 +1114,10 @@
continue;
}
- AdjacentParamsMixup APR =
- paramEqualTypeRange(Fun, ParamEqRangeStartIdx, CVRMixPossible);
- ParamEqRangeStartIdx += APR.size();
- if (APR.size() < MinimumLength)
+ AdjacentParamsMixup APR = paramEqualTypeRange(
+ Fun, ParamEqRangeStartIdx, CVRMixPossible, ImplicitConversion);
+ ParamEqRangeStartIdx += APR.NumParamsChecked;
+ if (APR.NumParamsChecked < MinimumLength)
continue;
// A function with an adjacent argument range of sufficient length found.
@@ -548,64 +1126,162 @@
FunName = "<unknown>";
const PrintingPolicy &PP{Fun->getASTContext().getLangOpts()};
- std::string FirstArgType = APR.front().first->getType().getAsString(PP);
-
- const auto HasMixupTagWarrantingTypePrint =
- [](const AdjacentParamsMixup::value_type &E) {
- return E.second & (MIXUP_Typedef | MIXUP_RefBind | MIXUP_CVR);
- };
-
- if (llvm::any_of(APR, HasMixupTagWarrantingTypePrint)) {
- // If any of the arguments in the range is annotated with a tag that will
- // warrant printing type names, the primary diagnostic shouldn't have
- // a type name printed.
- diag(APR.front().first->getOuterLocStart(),
- "%0 adjacent arguments for '%1' of similar type are easily swapped "
- "by mistake")
- << static_cast<unsigned>(APR.size()) << FunName;
- } else {
- diag(APR.front().first->getOuterLocStart(),
- "%0 adjacent arguments for '%1' of similar type ('%2') are easily "
- "swapped by mistake")
- << static_cast<unsigned>(APR.size()) << FunName << FirstArgType;
+ std::string FirstArgType = APR.getFirstParm()->getType().getAsString(PP);
+ llvm::SmallPtrSet<const ParmVarDecl *, 4> TypePrintedForParm;
+
+ const auto TypePrintFinder = [](const Mixup &E) {
+ return E.Data.Flag & (MIXUP_Typedef | MIXUP_RefBind | MIXUP_CVR);
+ };
+ bool HasAnyTypePrint = llvm::any_of(APR.Mixups, TypePrintFinder);
+ const auto ImplicitConvFinder = [](const Mixup &E) {
+ return E.Data.Flag & MIXUP_Implicit;
+ };
+ bool HasAnyImplicits = llvm::any_of(APR.Mixups, ImplicitConvFinder);
+
+ StringRef MainDiagnostic;
+ if (HasAnyImplicits)
+ MainDiagnostic = "%0 adjacent arguments for '%1' of convertible types "
+ "may be easily swapped "
+ "by mistake";
+ else if (HasAnyTypePrint)
+ MainDiagnostic =
+ "%0 adjacent arguments for '%1' of similar type are easily swapped "
+ "by mistake";
+ else
+ MainDiagnostic =
+ "%0 adjacent arguments for '%1' of similar type ('%2') are easily "
+ "swapped by mistake";
+
+ {
+ auto Diag = diag(APR.getFirstParm()->getOuterLocStart(), MainDiagnostic)
+ << static_cast<unsigned>(APR.NumParamsChecked) << FunName;
+ if (!HasAnyImplicits && !HasAnyTypePrint)
+ Diag << FirstArgType;
}
// FIXME: ClangTidy doesn't seem to support emitting a custom source range
// highlight without a fixit. This check can't produce useful automatic
// fixits, but highlighting the 'CharSourceRange' would be good to have.
- diag(APR.back().first->getLocation(),
+ diag(APR.getLastParm()->getLocation(),
"last argument in the adjacent range here", DiagnosticIDs::Note);
- for (const auto &ParmPair : APR) {
- if (ParmPair.second < MIXUP_Trivial)
- llvm_unreachable("Invalid mixup type in result when emitting diags.");
- // MIXUP_Trivial: no extra diagnostics required.
-
- if (HasMixupTagWarrantingTypePrint(ParmPair)) {
- std::string ParmType;
- {
- llvm::raw_string_ostream OS{ParmType};
- LLVM_DEBUG(llvm::dbgs()
- << "\n\nGenerating type name for diagnostic...\n\n";);
- putTypeName(ParmPair.first->getType(), OS, PP);
- }
+ for (const Mixup &M : APR.Mixups) {
+ assert(M.Data.Flag >= MIXUP_Trivial && "Too low bits in mixup type.");
+ // For MIXUP_Trivial no extra diagnostics required.
+
+ std::string FromParmType, ToParmType;
+ if (TypePrintFinder(M) || ImplicitConvFinder(M)) {
+ // Typedefs, reference binds, and implicit conversions might result in
+ // the type of a variable printed in the diagnostic, so we have to
+ // prepare it.
LLVM_DEBUG(llvm::dbgs()
- << "Type name generated: " << ParmType << '\n';);
+ << "\n\nGenerating type names for diagnostic...\n\n";);
+ FromParmType = typeNameAsString(M.From->getType(), PP);
+ ToParmType = typeNameAsString(M.To->getType(), PP);
+ LLVM_DEBUG(llvm::dbgs() << "Type names generated: " << FromParmType
+ << " >> and << " << ToParmType << '\n';);
+ }
- if (ParmPair.second & MIXUP_Typedef) {
+ if (TypePrintFinder(M)) {
+ if (M.Data.Flag & MIXUP_Typedef) {
// FIXME: Don't emit one side of the mix in question did not contain a
// typedef.
- diag(ParmPair.first->getOuterLocStart(),
- "after resolving type aliases, type of argument '%0' is '%1'",
- DiagnosticIDs::Note)
- << ParmPair.first->getName() << ParmType;
+ if (!TypePrintedForParm.count(M.From)) {
+ diag(M.From->getOuterLocStart(),
+ "after resolving type aliases, type of argument '%0' is '%1'",
+ DiagnosticIDs::Note)
+ << M.From->getName() << FromParmType;
+ TypePrintedForParm.insert(M.From);
+ }
+
+ if (!TypePrintedForParm.count(M.To)) {
+ diag(M.To->getOuterLocStart(),
+ "after resolving type aliases, type of argument '%0' is '%1'",
+ DiagnosticIDs::Note)
+ << M.To->getName() << ToParmType;
+ TypePrintedForParm.insert(M.To);
+ }
}
- if (ParmPair.second & (MIXUP_RefBind | MIXUP_CVR)) {
- diag(ParmPair.first->getOuterLocStart(),
+ if (M.Data.Flag & (MIXUP_RefBind | MIXUP_CVR)) {
+ diag(M.To->getOuterLocStart(),
"at a call site, '%0' might bind with same force as '%1'",
DiagnosticIDs::Note)
- << ParmType << FirstArgType;
+ << ToParmType << FirstArgType;
+ }
+ }
+
+ if (ImplicitConvFinder(M)) {
+ const ConversionSequence <R = M.Data.ConvLTR, &RTL = M.Data.ConvRTL;
+
+ const auto DiagnoseSequence =
+ [&M, &PP](const std::string &StartType,
+ const ConversionSequence &Seq) -> std::string {
+ assert((&Seq == &M.Data.ConvLTR || &Seq == &M.Data.ConvRTL) &&
+ "sequence should be a sequence from the capture.");
+ llvm::SmallString<256> SS;
+ llvm::raw_svector_ostream OS{SS};
+
+ OS << '\'' << StartType << '\'';
+ if (Seq.StdPre)
+ OS << " -> '" << typeNameAsString(QualType{Seq.StdPre, 0}, PP)
+ << '\'';
+ if (Seq.UserType)
+ OS << " -> '" << typeNameAsString(QualType{Seq.UserType, 0}, PP)
+ << '\'';
+ if (Seq.StdPost)
+ OS << " -> '" << typeNameAsString(QualType{Seq.StdPost, 0}, PP)
+ << '\'';
+
+ return OS.str().str();
+ };
+
+ if (LTR && RTL) {
+ std::string ConvMsg =
+ "'%0' and '%1' can suffer implicit conversions between one "
+ "another";
+ if (!LTR.single() && !RTL.single())
+ ConvMsg.append(": %2 and %3");
+ else if (!LTR.single())
+ ConvMsg.append(": %2 and trivially");
+ else if (!RTL.single())
+ ConvMsg.append(": trivially and %2");
+
+ auto Diag =
+ diag(M.To->getOuterLocStart(), ConvMsg, DiagnosticIDs::Note)
+ << FromParmType << ToParmType;
+ if (!LTR.single() && !RTL.single())
+ Diag << DiagnoseSequence(FromParmType, LTR)
+ << DiagnoseSequence(ToParmType, RTL);
+ else if (!LTR.single())
+ Diag << DiagnoseSequence(FromParmType, LTR);
+ else if (!RTL.single())
+ Diag << DiagnoseSequence(ToParmType, RTL);
+ } else {
+ std::string SeqDetails;
+ StringRef ConvMsg;
+ if (LTR) {
+ if (LTR.single())
+ ConvMsg = "'%0' can be implicitly converted to '%1'";
+ else {
+ ConvMsg = "'%0' can be implicitly converted to '%1': %2";
+ SeqDetails = DiagnoseSequence(FromParmType, LTR);
+ }
+ } else if (RTL) {
+ if (RTL.single())
+ ConvMsg = "'%0' can be implicitly converted from '%1'";
+ else {
+ ConvMsg = "'%0' can be implicitly converted from '%1': %2";
+ SeqDetails = DiagnoseSequence(ToParmType, RTL);
+ }
+ }
+ assert(!ConvMsg.empty() && "implicit bit set but we arrived here?");
+
+ auto Diag =
+ diag(M.To->getOuterLocStart(), ConvMsg, DiagnosticIDs::Note)
+ << FromParmType << ToParmType;
+ if (!SeqDetails.empty())
+ Diag << SeqDetails;
}
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
