[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-06-28 Thread Whisperity via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG26d864b44b9d: [clang-tidy] Extend 
bugprone-easily-swappable-parameters with `typedef` and… (authored 
by whisperity).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95736

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

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
@@ -115,20 +115,38 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
 // CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
 
-void throughTypedef(int I, MyInt1 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
-// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+void typedefMultiple(MyInt1 I1, MyInt2 I2x, MyInt2 I2y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 adjacent parameters of 'typedefMultiple' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'I2y'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+void throughTypedef1(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'throughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:26: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+
+void betweenTypedef2(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'betweenTypedef2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+typedef MyInt2 MyInt2b;
 
-void betweenTypedef(MyInt1 I, MyInt2 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'betweenTypedef' of similar type ('MyInt1')
-// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:38: note: the last parameter in the range is 'J'
+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'
+// CHECK-MESSAGES: :[[@LINE-3]]:58: note: the last parameter in the range is 'MI2b'
+// CHECK-MESSAGES: :[[@LINE-4]]:19: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+// 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: Not the same type.
+void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: int and long.
 
 void qualified1(int I, const int CI) {} // NO-WARN: Not the same type.
 
@@ -142,18 +160,73 @@
 
 void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Not the same type.
 
-void qualifiedThroughTypedef2(CInt CI1, const int CI2) {} // NO-WARN: Not the same type.
-// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type ('CInt')
+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'
-
-void reference1(int I, int ) {} // NO-WARN: Not the same type.
-
-void reference2(int I, const int ) {} // NO-WARN: Not the same type.
-
-void reference3(int I, int &) {} // NO-WARN: Not the same type.
-
-void reference4(int I,

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-06-24 Thread Whisperity via Phabricator via cfe-commits 
whisperity updated this revision to Diff 354183.
whisperity added a comment.

**NFC** Fix lint.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95736

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

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
@@ -115,20 +115,38 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
 // CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
 
-void throughTypedef(int I, MyInt1 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
-// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+void typedefMultiple(MyInt1 I1, MyInt2 I2x, MyInt2 I2y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 adjacent parameters of 'typedefMultiple' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'I2y'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+void throughTypedef1(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'throughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:26: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+
+void betweenTypedef2(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'betweenTypedef2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+typedef MyInt2 MyInt2b;
 
-void betweenTypedef(MyInt1 I, MyInt2 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'betweenTypedef' of similar type ('MyInt1')
-// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:38: note: the last parameter in the range is 'J'
+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'
+// CHECK-MESSAGES: :[[@LINE-3]]:58: note: the last parameter in the range is 'MI2b'
+// CHECK-MESSAGES: :[[@LINE-4]]:19: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+// 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: Not the same type.
+void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: int and long.
 
 void qualified1(int I, const int CI) {} // NO-WARN: Not the same type.
 
@@ -142,18 +160,73 @@
 
 void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Not the same type.
 
-void qualifiedThroughTypedef2(CInt CI1, const int CI2) {} // NO-WARN: Not the same type.
-// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type ('CInt')
+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'
-
-void reference1(int I, int ) {} // NO-WARN: Not the same type.
-
-void reference2(int I, const int ) {} // NO-WARN: Not the same type.
-
-void reference3(int I, int &) {} // NO-WARN: Not the same type.
-
-void reference4(int I, const int &) {} // NO-WARN: Not the same type.
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-06-23 Thread Whisperity via Phabricator via cfe-commits 
whisperity updated this revision to Diff 353972.
whisperity added a comment.

**NFC**: Rebase & update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95736

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

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
@@ -111,20 +111,38 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
 // CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
 
-void throughTypedef(int I, MyInt1 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
-// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+void typedefMultiple(MyInt1 I1, MyInt2 I2x, MyInt2 I2y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 adjacent parameters of 'typedefMultiple' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'I2y'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+void throughTypedef1(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'throughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:26: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+
+void betweenTypedef2(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'betweenTypedef2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+typedef MyInt2 MyInt2b;
 
-void betweenTypedef(MyInt1 I, MyInt2 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'betweenTypedef' of similar type ('MyInt1')
-// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:38: note: the last parameter in the range is 'J'
+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'
+// CHECK-MESSAGES: :[[@LINE-3]]:58: note: the last parameter in the range is 'MI2b'
+// CHECK-MESSAGES: :[[@LINE-4]]:19: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+// 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: Not the same type.
+void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: int and long.
 
 void qualified1(int I, const int CI) {} // NO-WARN: Not the same type.
 
@@ -138,18 +156,73 @@
 
 void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Not the same type.
 
-void qualifiedThroughTypedef2(CInt CI1, const int CI2) {} // NO-WARN: Not the same type.
-// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type ('CInt')
+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'
-
-void reference1(int I, int ) {} // NO-WARN: Not the same type.
-
-void reference2(int I, const int ) {} // NO-WARN: Not the same type.
-
-void reference3(int I, int &) {} // NO-WARN: Not the same type.
-
-void reference4(int I, const int &) {} // NO-WARN: Not the same type.
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-04-10 Thread Whisperity via Phabricator via cfe-commits 
whisperity updated this revision to Diff 336619.
whisperity added a comment.

**[NFC]** Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95736

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

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
@@ -111,20 +111,38 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
 // CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
 
-void throughTypedef(int I, MyInt1 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
-// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+void typedefMultiple(MyInt1 I1, MyInt2 I2x, MyInt2 I2y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 adjacent parameters of 'typedefMultiple' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'I2y'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+void throughTypedef1(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'throughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:26: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+
+void betweenTypedef2(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'betweenTypedef2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+typedef MyInt2 MyInt2b;
 
-void betweenTypedef(MyInt1 I, MyInt2 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'betweenTypedef' of similar type ('MyInt1')
-// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:38: note: the last parameter in the range is 'J'
+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'
+// CHECK-MESSAGES: :[[@LINE-3]]:58: note: the last parameter in the range is 'MI2b'
+// CHECK-MESSAGES: :[[@LINE-4]]:19: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+// 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: Not the same type.
+void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: int and long.
 
 void qualified1(int I, const int CI) {} // NO-WARN: Not the same type.
 
@@ -138,18 +156,73 @@
 
 void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Not the same type.
 
-void qualifiedThroughTypedef2(CInt CI1, const int CI2) {} // NO-WARN: Not the same type.
-// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type ('CInt')
+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'
-
-void reference1(int I, int ) {} // NO-WARN: Not the same type.
-
-void reference2(int I, const int ) {} // NO-WARN: Not the same type.
-
-void reference3(int I, int &) {} // NO-WARN: Not the same type.
-
-void reference4(int I, const int &) {} // NO-WARN: Not the same type.
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-03-18 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


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

https://reviews.llvm.org/D95736

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-03-17 Thread Whisperity via Phabricator via cfe-commits 
whisperity updated this revision to Diff 331242.
whisperity marked 4 inline comments as done.
whisperity added a comment.

- Made the diagnostic message about "binding power" easier to understand.


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

https://reviews.llvm.org/D95736

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

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
@@ -111,20 +111,38 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
 // CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
 
-void throughTypedef(int I, MyInt1 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
-// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+void typedefMultiple(MyInt1 I1, MyInt2 I2x, MyInt2 I2y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 adjacent parameters of 'typedefMultiple' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'I2y'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+void throughTypedef1(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'throughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:26: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+
+void betweenTypedef2(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'betweenTypedef2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+typedef MyInt2 MyInt2b;
 
-void betweenTypedef(MyInt1 I, MyInt2 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'betweenTypedef' of similar type ('MyInt1')
-// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:38: note: the last parameter in the range is 'J'
+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'
+// CHECK-MESSAGES: :[[@LINE-3]]:58: note: the last parameter in the range is 'MI2b'
+// CHECK-MESSAGES: :[[@LINE-4]]:19: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+// 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: Not the same type.
+void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: int and long.
 
 void qualified1(int I, const int CI) {} // NO-WARN: Not the same type.
 
@@ -138,18 +156,73 @@
 
 void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Not the same type.
 
-void qualifiedThroughTypedef2(CInt CI1, const int CI2) {} // NO-WARN: Not the same type.
-// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type ('CInt')
+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'
-
-void reference1(int I, int ) {} // NO-WARN: Not the same type.
-
-void reference2(int I, const int ) {} // NO-WARN: Not the same type.
-
-void reference3(int I, int &) {} // NO-WARN: Not the same type.
-
-void reference4(int I, const int &) {} // NO-WARN: Not the same type.
+//

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-03-16 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:706
+UniqueBindPower({LType, RType})) {
+  // FIXME: Don't emit multiple combinations here either.
+  StringRef DiagText = "a call site binds an expression to '%0' and "

whisperity wrote:
> This is a stale FIXME, the uniqueing of the emission is in the line right 
> above...
Heh, I was sort of wondering about that. Please remove the FIXME before 
committing.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:707-708
+  // FIXME: Don't emit multiple combinations here either.
+  StringRef DiagText = "a call site binds an expression to '%0' and "
+   "'%1' with the same force";
+  diag(M.Second->getOuterLocStart(), DiagText, DiagnosticIDs::Note)

whisperity wrote:
> aaron.ballman wrote:
> > I think "with the same force" is going to be hard for users to make sense 
> > of. I'm at a bit of a loss for how to word it though. The issue is that `T` 
> > and `const T&` parameters *might* be easily swapped, so maybe it's best to 
> > call it out that way?
> This is a note tag to explain the reason behind the mix. The warning itself 
> has been emitted before. How about
> 
> > a `T` and `const T&` parameter accepts all values of `T`
> 
> ?
I think that'd be significantly more clear, yes!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95736

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-03-16 Thread Whisperity via Phabricator via cfe-commits 
whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:706
+UniqueBindPower({LType, RType})) {
+  // FIXME: Don't emit multiple combinations here either.
+  StringRef DiagText = "a call site binds an expression to '%0' and "

This is a stale FIXME, the uniqueing of the emission is in the line right 
above...



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:707-708
+  // FIXME: Don't emit multiple combinations here either.
+  StringRef DiagText = "a call site binds an expression to '%0' and "
+   "'%1' with the same force";
+  diag(M.Second->getOuterLocStart(), DiagText, DiagnosticIDs::Note)

aaron.ballman wrote:
> I think "with the same force" is going to be hard for users to make sense of. 
> I'm at a bit of a loss for how to word it though. The issue is that `T` and 
> `const T&` parameters *might* be easily swapped, so maybe it's best to call 
> it out that way?
This is a note tag to explain the reason behind the mix. The warning itself has 
been emitted before. How about

> a `T` and `const T&` parameter accepts all values of `T`

?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95736

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-03-16 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:707-708
+  // FIXME: Don't emit multiple combinations here either.
+  StringRef DiagText = "a call site binds an expression to '%0' and "
+   "'%1' with the same force";
+  diag(M.Second->getOuterLocStart(), DiagText, DiagnosticIDs::Note)

I think "with the same force" is going to be hard for users to make sense of. 
I'm at a bit of a loss for how to word it though. The issue is that `T` and 
`const T&` parameters *might* be easily swapped, so maybe it's best to call it 
out that way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95736

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-02-25 Thread Whisperity via Phabricator via cfe-commits 
whisperity updated this revision to Diff 326440.
whisperity added a comment.

- **[NFC]** Reformat the documentation so option values are in single backticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95736

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

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
@@ -106,20 +106,38 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
 // CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
 
-void throughTypedef(int I, MyInt1 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
-// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+void typedefMultiple(MyInt1 I1, MyInt2 I2x, MyInt2 I2y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 adjacent parameters of 'typedefMultiple' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'I2y'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+void throughTypedef1(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'throughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:26: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+
+void betweenTypedef2(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'betweenTypedef2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+typedef MyInt2 MyInt2b;
 
-void betweenTypedef(MyInt1 I, MyInt2 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'betweenTypedef' of similar type ('MyInt1')
-// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:38: note: the last parameter in the range is 'J'
+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'
+// CHECK-MESSAGES: :[[@LINE-3]]:58: note: the last parameter in the range is 'MI2b'
+// CHECK-MESSAGES: :[[@LINE-4]]:19: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+// 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: Not the same type.
+void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: int and long.
 
 void qualified1(int I, const int CI) {} // NO-WARN: Not the same type.
 
@@ -133,18 +151,73 @@
 
 void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Not the same type.
 
-void qualifiedThroughTypedef2(CInt CI1, const int CI2) {} // NO-WARN: Not the same type.
-// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type ('CInt')
+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'
-
-void reference1(int I, int ) {} // NO-WARN: Not the same type.
-
-void reference2(int I, const int ) {} // NO-WARN: Not the same type.
-
-void reference3(int I, int &) {} // NO-WARN: Not the same type.
-
-void reference4(int I, const int &) {} // NO-WARN: Not the same type.
+//

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-02-09 Thread Whisperity via Phabricator via cfe-commits 
whisperity updated this revision to Diff 322431.
whisperity marked 6 inline comments as done.
whisperity added a comment.
Herald added a subscriber: nullptr.cpp.

- **NFC** Code style, formatting, wording, etc. changes as per review comments.
- **NFC** Removed a stale FIXME


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95736

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

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
@@ -86,20 +86,38 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
 // CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
 
-void throughTypedef(int I, MyInt1 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
-// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+void typedefMultiple(MyInt1 I1, MyInt2 I2x, MyInt2 I2y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 adjacent parameters of 'typedefMultiple' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'I2y'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+void throughTypedef1(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'throughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:26: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+
+void betweenTypedef2(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'betweenTypedef2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+typedef MyInt2 MyInt2b;
 
-void betweenTypedef(MyInt1 I, MyInt2 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'betweenTypedef' of similar type ('MyInt1')
-// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:38: note: the last parameter in the range is 'J'
+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'
+// CHECK-MESSAGES: :[[@LINE-3]]:58: note: the last parameter in the range is 'MI2b'
+// CHECK-MESSAGES: :[[@LINE-4]]:19: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+// 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: Not the same type.
+void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: int and long.
 
 void qualified1(int I, const int CI) {} // NO-WARN: Not the same type.
 
@@ -113,18 +131,73 @@
 
 void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Not the same type.
 
-void qualifiedThroughTypedef2(CInt CI1, const int CI2) {} // NO-WARN: Not the same type.
-// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type ('CInt')
+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'
-
-void reference1(int I, int ) {} // NO-WARN: Not the same type.
-
-void reference2(int I, const int ) {} // NO-WARN: Not the same type.
-
-void reference3(int I, int

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-02-02 Thread Whisperity via Phabricator via cfe-commits 
whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:135
   void sanitize() {
-assert(Flags != MIX_Invalid && "sanitize() called on invalid bitvec");
-// TODO: There will be statements here in further extensions of the check.
+assert(Flags != MIX_Invalid && "sanitise() called on sentinel bitvec");
+

aaron.ballman wrote:
> 
How in the... Okay, I think I need to up my rebase game because I know for a 
fact that I fixed BrE to AmE in the parent patch and that patch was merged into 
the branch this patch was created from and this patch is created against the 
parent, obviously.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:240
+  // user to understand.
+  if (isa(LType.getTypePtr())) {
+LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. LHS is ParenType.\n");

aaron.ballman wrote:
> 
Is `const` respected properly in this case, will it not try to cast it away? 
Wow... I did not know this!



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:289-290
 
+/// Calculates if the reference binds an expression of the given type. This is
+/// true iff 'LRef' is some 'const T &' type, and the 'Ty' is 'T' or 'const T'.
+static MixData isLRefEquallyBindingToType(const TheCheck ,

aaron.ballman wrote:
> I think this is reasonable but it does raise a question about implicit 
> conversions, which I suppose is more of a generic question about the check. 
> Should the check consider these to be easily swappable parameter types?
> ```
> struct S {
>   S(int);
>   operator int() const;
> };
> 
> void func(S s, int i);
> ```
> given that you can call `func()` like:
> ```
> S s;
> int i;
> 
> func(i, s);
> func(s, i);
> ```
> (Also, does the answer change if `S` has more types it can convert to/from? 
> Or does the answer change if `S` can only convert from a type (or convert to 
> a type) rather than convert both ways?)
Yes, yes, yes and yes! That is the whole idea, that is one of the things in 
which we are "better" than the existing works... and one of the reasons behind 
this whole "argument swapping" deal is much worse in C++, than let's say Java. 
(A lot of existing literature deals with Java, or deals with C++ but only in 
terms of type equivalence, or not considering types -- only arg and param names 
-- at all.) That one shall be diagnosed, but the whole implicit conversion 
"mess" is coming in a later patch!
In fact, the patch for implicit conversions is already up, but in a //will be 
reworked// state.

Unfortunately, I can't seem to find a way to link a particular line of a 
particular diff, so let's do it manually. The `...-implicits.cpp` test file in 
D75041 [[ http://reviews.llvm.org/D75041?id=315384 | diff 315384 ]] starting 
from line 95 shows this being dealt with.

Originally, implicit conversions were warned about when they were only 
unidirectional (i.e. `int -> S` possible, `S -> int` not possible, `fn(int{}, 
S{})` warned). However, this created **a lot of** false positives even in the 
two or three projects I went over manually, so... for the time being, I 
reworked the patch (only changing like 2 lines at most!) to only deal with 
//bidirectional// implicit conversions. Because in case you say `fn(5, 5)` it 
still accepts, but if you have a //valid `s`//, `fn(s, 5)` is a compile error.
(You can check the [[ http://reviews.llvm.org/D75041?id=259324 | earlier diff 
]] at the same location for this case.)

This becomes a deep rabbit hole way too fast, because one layer of the analysis 
is //"warn about `f(T1, T2)` if `f(T2, T1)` is also possible"//. That's the 
idea I'm trying to sell right now. Saying //"warn about `f(T1, T2)` if there 
exists some type `Tx` with which `f(Tx, Tx)` is possible"// is a whooole lotta 
harder to handle. Maybe not impossible, but definitely harder, and with most 
likely introducing a plethora of weird false positives.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:289-290
 
+/// Calculates if the reference binds an expression of the given type. This is
+/// true iff 'LRef' is some 'const T &' type, and the 'Ty' is 'T' or 'const T'.
+static MixData isLRefEquallyBindingToType(const TheCheck ,

whisperity wrote:
> aaron.ballman wrote:
> > I think this is reasonable but it does raise a question about implicit 
> > conversions, which I suppose is more of a generic question about the check. 
> > Should the check consider these to be easily swappable parameter types?
> > ```
> > struct S {
> >   S(int);
> >   operator int() const;
> > };
> > 
> > void func(S s, int i);
> > ```
> > given that you can call `func()` like:
> > ```
> > S s;
> > int i;
> > 
> > func(i, s);
> > func(s, i);
> > ```
> > (Also, does the answer change if `S` has more types it can

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-02-02 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:135
   void sanitize() {
-assert(Flags != MIX_Invalid && "sanitize() called on invalid bitvec");
-// TODO: There will be statements here in further extensions of the check.
+assert(Flags != MIX_Invalid && "sanitise() called on sentinel bitvec");
+





Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:237
 
-  // TODO: Implement more elaborate logic, such as typedef, implicit
-  // conversions, etc.
+  // Dissolve certain type sugars that do not affect the mixability of one type
+  // with the other, and also do not require any sort of elaboration for the

"Dissolve certain type sugars" has to be one of my new favorite comments. :-D



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:240
+  // user to understand.
+  if (isa(LType.getTypePtr())) {
+LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. LHS is ParenType.\n");





Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:245
+  }
+  if (isa(RType.getTypePtr())) {
+LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. RHS is ParenType.\n");





Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:289-290
 
+/// Calculates if the reference binds an expression of the given type. This is
+/// true iff 'LRef' is some 'const T &' type, and the 'Ty' is 'T' or 'const T'.
+static MixData isLRefEquallyBindingToType(const TheCheck ,

I think this is reasonable but it does raise a question about implicit 
conversions, which I suppose is more of a generic question about the check. 
Should the check consider these to be easily swappable parameter types?
```
struct S {
  S(int);
  operator int() const;
};

void func(S s, int i);
```
given that you can call `func()` like:
```
S s;
int i;

func(i, s);
func(s, i);
```
(Also, does the answer change if `S` has more types it can convert to/from? Or 
does the answer change if `S` can only convert from a type (or convert to a 
type) rather than convert both ways?)



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:306
+
+  QualType UnqualifiedReferred = ReferredType;
+  UnqualifiedReferred.removeLocalConst();

It's not really unqualified since it could still have `volatile` (etc).



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:467-468
+  bool operator()(E El) {
+auto Insert = CalledWith.insert(std::move(El));
+return Insert.second;
+  }





Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:473-474
+
+private:
+  llvm::SmallSet CalledWith;
+};

Might as well hoist this to be above the `public` access specifier (and can 
drop the `private` access specifier entirely at that point).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95736

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95736: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with `typedef` and `const &` diagnostics

2021-01-30 Thread Whisperity via Phabricator via cfe-commits 
whisperity created this revision.
whisperity added a reviewer: aaron.ballman.
whisperity added projects: clang, clang-tools-extra.
Herald added subscribers: martong, gamesh411, Szelethus, rnkovacs, xazax.hun.
whisperity requested review of this revision.

The base patch only deals with strict (canonical) type equality, which is 
merely a subset of all the dangerous function interfaces that we intend to 
find. In addition, in the base patch, canonical type equivalence is not 
diagnosed in a way that is immediately apparent to the user.

This patch extends the check with two features:

1. Proper `typedef` diagnostics and explanations to the user.
2. //"Reference bind power"// matching.

**Case 1** will tell the user //the common type of "SomeIntegerType" and 
"SomeOtherIntegerType" is "int"// or //"my_int" and "int" are the same//. This 
makes the (otherwise, through `CanonicalType` desugaring already handled and 
diagnosed, but not //explained//) typedef case obvious to the user without 
having to invoke code comprehension tools.

**Case 2** is a necessary addition because in every case someone encounters a 
function `f(T t, const T& tr)`, any expression that might be passed to either 
can be passed to both. Thus, such adjacent parameter sequences should be 
matched. This is //specifically// modelled for `const&` only, as any other pair 
combination from "value, ref, qualified ref, rref" have //some// broad value 
category in which some actual swap of the arguments at a call would be a 
compiler error. (E.g. `f(int, int&)`  and `f(2, Val)` breaks if swapped, 
similarly how rrefs cannot bind lvalues, etc.) This case **broadens** the 
result set, but only with the most dangerous and "Please think about this and 
refactor..." case.

(Note, that //Case 2// does not model things like `f(int, 
std::reference_wrapper)`.)

Due to the cases above always being a problem (and there isn't a good enough 
reason to say //typedefs should not be diagnosed//, everyone hates the weak 
typedef...), I decided **not** to make matching these configurable.

This patch also serves as a proof-of-concept on how I wish the check to evolve 
over some more subsequent patches, which all aim to introduce user-toggleable 
modelling.

> Originally, the implementation of this feature was part of the very first 
> patch related to this check, D69560  (diff 
> 259320 ). It has been factored out 
> for clarity and to allow more on-point discussion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95736

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp

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
@@ -80,20 +80,38 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
 // CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
 
-void throughTypedef(int I, MyInt1 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
-// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+void typedefMultiple(MyInt1 I1, MyInt2 I2x, MyInt2 I2y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 adjacent parameters of 'typedefMultiple' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'I2y'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+void throughTypedef1(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'throughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:26: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+
+void betweenTypedef2(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'betweenTypedef2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases,