[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity updated this revision to Diff 353979.
whisperity added a comment.

**NFC**: Rebase & update.

While this patch conceptually only depends on D69560 
, the diff itself has become linear during the 
code review and subsequent updates, and as such, must be applied on top of 
D78652 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicit-qualifiers.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -5,7 +5,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \
 // RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
 // RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0} \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' -- -x c
 
 #define bool _Bool
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
@@ -5,7 +5,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
 // RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
 // RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1} \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' --
 
 namespace std {
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
@@ -5,7 +5,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
 // RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
 // RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1} \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' -- -x c
 
 int myprint();
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
==

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity updated this revision to Diff 354229.
whisperity added a comment.

**NC** Rebase and buildbot trigger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicit-qualifiers.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -5,7 +5,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \
 // RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
 // RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0} \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' -- -x c
 
 #define bool _Bool
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
@@ -5,7 +5,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
 // RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
 // RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1} \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' --
 
 namespace std {
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
@@ -5,7 +5,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
 // RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
 // RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1} \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' -- -x c
 
 int myprint();
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cp

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity created this revision.
whisperity added reviewers: aaron.ballman, alexfh, hokein, JonasToth.
whisperity added projects: clang, clang-tools-extra.
Herald added subscribers: gamesh411, Szelethus, rnkovacs, xazax.hun.
whisperity requested review of this revision.

While the original check's (D69560 ) purpose 
is to identify potentially dangerous functions based on the parameter types (as 
identifier names do not mean //anything// when it comes to the language rules), 
unfortunately, such a plain interface check rule can be incredibly noisy. While 
the previous //"filtering heuristic"// in D78652 
 is able to find many similar usages, there is 
an entire class of parameters that should not be warned about very easily mixed 
by that check: parameters that have a name and their name follows a pattern, 
e.g. `text1, text2, text3, ...`. [1]

This patch implements a simple, but powerful rule, that allows us to detect 
such cases and ensure that no warnings are emitted for parameter sequences that 
follow a pattern, even if their types allow for them to be potentially mixed at 
a call site.

Given a threshold `k`, warnings about two parameters are filtered from the 
result set if the names of the parameters are either prefixes or suffixes of 
each other, with at most `k` letters difference on the non-common end. 
(Assuming that the names themselves are at least `k` long.)

The above `text1, text2` is an example of this. (Live finding from Xerces.)
`LHS` and `RHS` are also fitting the bill here. (Live finding from... virtually 
any project.)
So does `Qmat, Tmat, Rmat`. (Live finding from I think OpenCV.)

On average, **15%** reduction in findings were achieved with this feature 
turned on.

Similarly to the other filtering patch on the "relatedness modelling", this 
option is implemented as a check option, 
`NamePrefixSuffixSilenceDissimilarityTreshold`. If `0`, the heuristic is turned 
off. **Defaults to `1`**, i.e. a single letter difference ("pattern") on either 
end of the parameter name suffice for silence. This is the most reasonable 
default.



[1] It has been identified before by other research that such patterned names 
carry no viable information and such names are useless for a name-based 
analysis where argument vs. parameter names are contrasted with one another. 
[Rice2017]

//[Rice2017]//: Andrew Rice, et al.: Detecting argument selection defects. In: 
Proceedings of the ACM on Programming Languages, 1, pp. 104:1-104:23, 2017.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97297

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -2,7 +2,8 @@
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
-// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"} \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' -- -x c
 
 #define bool _Bool
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
+// RUN: {key: bugprone-easily-swappa

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity updated this revision to Diff 325799.
whisperity added a comment.

- **[NFC]** Realised I forgot to add the new option to the check's 
documentation. This is fixed now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -2,7 +2,8 @@
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
-// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"} \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' -- -x c
 
 #define bool _Bool
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 1} \
+// RUN:  ]}' --
+
+namespace std {
+struct string {};
+} // namespace std
+class Matrix {};
+
+void test1(int Foo, int Bar) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent parameters of 'test1' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:16: note: the first parameter in the range is 'Foo'
+// CHECK-MESSAGES: :[[@LINE-3]]:25: note: the last parameter in the range is 'Bar'
+
+void test2(int A, int B) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent parameters of 'test2' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:16: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-3]]:23: note: the last parameter in the range is 'B'
+
+void test3(int Val1, int Val2) {} // NO-WARN.
+
+void test4(int ValA, int Valb) {} // NO-WARN.
+
+void test5(int Val1, int ValZ) {} // NO-WARN.
+
+void test6(int PValue, int QValue) {} // NO-WARN.
+
+void test7(std::string Astr, std::string Bstr) {} // NO-WARN.
+
+void test8(int Aladdin, int Alabaster) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent parameters of 'test8' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:16: note: the first parameter in the range is 'Aladdin'
+// CHECK-MESSAGES: :[[@LINE-3]]:29: note: the last parameter in the range is 'Alabaster'
+
+void test9(Matrix Qmat, Matrix Rmat, Matrix Tmat) {} // NO-WARN.
+
+void test10(int Something, int Other, int Foo, int Bar1, int Bar2, int Baz, int Qux) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 4 adjacent parameters of 'test10' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-2]]:17: note: the first parameter in the range is 'Something'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'Bar1'
+//
+// CHECK-MESSAGES: :[[@LINE-5]]:58: warning: 3 adjacent parameters of 'test10' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-6]]:62: note: the first parameter in the range is 'Bar2'
+// CHECK-MESSAGES: :[[@LINE-7]]:81: note: the last parameter in the range is 'Qux'
+
+void test11(int Foobar, int Foo) {}
+// CHECK-MESSAGES: :[[@LINE-

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please mention new option in Release Notes.




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst:89
+warning about the two parameters are silenced.
+Defaults to ``1``.
+Might be any positive integer.

Please use single back-ticks for option values. Same below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D97297#2583391 , @Eugene.Zelenko 
wrote:

> Please mention new option in Release Notes.

The base check is not yet accepted or in. I plan to land the entire thing in 
one go once everything is accepted because a partial landing would result in a 
useless check (in one way or another).




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst:89
+warning about the two parameters are silenced.
+Defaults to ``1``.
+Might be any positive integer.

Eugene.Zelenko wrote:
> Please use single back-ticks for option values. Same below.
**All** values (including above the ``_Bool`` and such, or only the numeric 
constants? The ``LHS`` and ``RHS`` below are examples of source code, not a 
valid value for the option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst:89
+warning about the two parameters are silenced.
+Defaults to ``1``.
+Might be any positive integer.

whisperity wrote:
> Eugene.Zelenko wrote:
> > Please use single back-ticks for option values. Same below.
> **All** values (including above the ``_Bool`` and such, or only the numeric 
> constants? The ``LHS`` and ``RHS`` below are examples of source code, not a 
> valid value for the option.
All option values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity updated this revision to Diff 326442.
whisperity marked 2 inline comments as done.
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/D97297/new/

https://reviews.llvm.org/D97297

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

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -2,7 +2,8 @@
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
-// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"} \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' -- -x c
 
 #define bool _Bool
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 1} \
+// RUN:  ]}' --
+
+namespace std {
+struct string {};
+} // namespace std
+class Matrix {};
+
+void test1(int Foo, int Bar) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent parameters of 'test1' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:16: note: the first parameter in the range is 'Foo'
+// CHECK-MESSAGES: :[[@LINE-3]]:25: note: the last parameter in the range is 'Bar'
+
+void test2(int A, int B) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent parameters of 'test2' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:16: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-3]]:23: note: the last parameter in the range is 'B'
+
+void test3(int Val1, int Val2) {} // NO-WARN.
+
+void test4(int ValA, int Valb) {} // NO-WARN.
+
+void test5(int Val1, int ValZ) {} // NO-WARN.
+
+void test6(int PValue, int QValue) {} // NO-WARN.
+
+void test7(std::string Astr, std::string Bstr) {} // NO-WARN.
+
+void test8(int Aladdin, int Alabaster) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent parameters of 'test8' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:16: note: the first parameter in the range is 'Aladdin'
+// CHECK-MESSAGES: :[[@LINE-3]]:29: note: the last parameter in the range is 'Alabaster'
+
+void test9(Matrix Qmat, Matrix Rmat, Matrix Tmat) {} // NO-WARN.
+
+void test10(int Something, int Other, int Foo, int Bar1, int Bar2, int Baz, int Qux) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 4 adjacent parameters of 'test10' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-2]]:17: note: the first parameter in the range is 'Something'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'Bar1'
+//
+// CHECK-MESSAGES: :[[@LINE-5]]:58: warning: 3 adjacent parameters of 'test10' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-6]]:62: note: the first parameter in the range is 'Bar2'
+// CHECK-MESSAGES: :[[@LINE-7]]:81: note: the last parameter in the range is 'Qux'
+
+void test11(int Foobar, int Foo) {}
+// C

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0fba450b9756: [clang-tidy] Suppress reports to patternedly 
named parameters in 'bugprone… (authored by whisperity).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicit-qualifiers.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -5,7 +5,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \
 // RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
 // RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0} \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' -- -x c
 
 #define bool _Bool
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
@@ -5,7 +5,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
 // RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
 // RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1} \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' --
 
 namespace std {
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
@@ -5,7 +5,8 @@
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
 // RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
 // RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1} \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' -- -x c
 
 int myprint();
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
+++ cla

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:364-365
+StringRef S2) {
+  StringRef S1Prefix = S1.take_front(S1.size() - N),
+S2Prefix = S2.take_front(S2.size() - N);
+  return S1Prefix == S2Prefix && !S1Prefix.empty();

Should we be checking that `.size() - N` doesn't cause wraparound?



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:371
+StringRef S2) {
+  StringRef S1Suffix = S1.take_back(S1.size() - N),
+S2Suffix = S2.take_back(S2.size() - N);

Same question here as above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:364-365
+StringRef S2) {
+  StringRef S1Prefix = S1.take_front(S1.size() - N),
+S2Prefix = S2.take_front(S2.size() - N);
+  return S1Prefix == S2Prefix && !S1Prefix.empty();

aaron.ballman wrote:
> Should we be checking that `.size() - N` doesn't cause wraparound?
Wraparound as in underflow? Like `2 - 8` becoming multiple millions? The 
incoming strings are padded to the common length.

`take_xxx` won't let you go out of bounds, even if the parameter is greater 
than the string's length. It will just silently return the entire string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[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!




Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:364-365
+StringRef S2) {
+  StringRef S1Prefix = S1.take_front(S1.size() - N),
+S2Prefix = S2.take_front(S2.size() - N);
+  return S1Prefix == S2Prefix && !S1Prefix.empty();

whisperity wrote:
> aaron.ballman wrote:
> > Should we be checking that `.size() - N` doesn't cause wraparound?
> Wraparound as in underflow? Like `2 - 8` becoming multiple millions? The 
> incoming strings are padded to the common length.
> 
> `take_xxx` won't let you go out of bounds, even if the parameter is greater 
> than the string's length. It will just silently return the entire string.
> Wraparound as in underflow? Like 2 - 8 becoming multiple millions? The 
> incoming strings are padded to the common length.

They are, but I didn't see anything checking that `Threshhold` (which gets 
passed as `N`) isn't larger than the common string length.

> take_xxx won't let you go out of bounds, even if the parameter is greater 
> than the string's length. It will just silently return the entire string.

Aha, that's the important bit, thanks for verifying that this is safe!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity marked an inline comment as not done.
whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:364-365
+StringRef S2) {
+  StringRef S1Prefix = S1.take_front(S1.size() - N),
+S2Prefix = S2.take_front(S2.size() - N);
+  return S1Prefix == S2Prefix && !S1Prefix.empty();

aaron.ballman wrote:
> whisperity wrote:
> > aaron.ballman wrote:
> > > Should we be checking that `.size() - N` doesn't cause wraparound?
> > Wraparound as in underflow? Like `2 - 8` becoming multiple millions? The 
> > incoming strings are padded to the common length.
> > 
> > `take_xxx` won't let you go out of bounds, even if the parameter is greater 
> > than the string's length. It will just silently return the entire string.
> > Wraparound as in underflow? Like 2 - 8 becoming multiple millions? The 
> > incoming strings are padded to the common length.
> 
> They are, but I didn't see anything checking that `Threshhold` (which gets 
> passed as `N`) isn't larger than the common string length.
> 
> > take_xxx won't let you go out of bounds, even if the parameter is greater 
> > than the string's length. It will just silently return the entire string.
> 
> Aha, that's the important bit, thanks for verifying that this is safe!
Nevertheless I'll clarify this, specifically because I have to make sure (I was 
thinking for a few minutes then realised the `!S1Prefix.empty()` is actually 
making sure of this...) that if you got parameter names of length 1 (let's say) 
and a threshold of 1, it really shouldn't say that `A` and `B` are "related" 
because the common prefix is `""` and the non-common end differs in one 
character, `A` vs. `B`.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:384
+  // Pad the two strings to the longer length.
+  std::size_t BiggerLength = std::max(Str1.size(), Str2.size());
+  SmallString<32> S1PadE{Str1}, S2PadE{Str2};

@aaron.ballman I think I'll do something like `if (BiggerLength < Threshold) 
return false;`, how does that sound? The comparison is moot in that case, 
imagine having strings of 2 and 4, padded to 4, with a threshold of 6, for 
example.

That way even if someone accidentally makes `StringRef` overflow the buffer, 
we'll be safe on the call paths we have here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:384
+  // Pad the two strings to the longer length.
+  std::size_t BiggerLength = std::max(Str1.size(), Str2.size());
+  SmallString<32> S1PadE{Str1}, S2PadE{Str2};

whisperity wrote:
> @aaron.ballman I think I'll do something like `if (BiggerLength < Threshold) 
> return false;`, how does that sound? The comparison is moot in that case, 
> imagine having strings of 2 and 4, padded to 4, with a threshold of 6, for 
> example.
> 
> That way even if someone accidentally makes `StringRef` overflow the buffer, 
> we'll be safe on the call paths we have here.
I think that'd make the behavior much more obvious, but should that be `<=`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:384
+  // Pad the two strings to the longer length.
+  std::size_t BiggerLength = std::max(Str1.size(), Str2.size());
+  SmallString<32> S1PadE{Str1}, S2PadE{Str2};

aaron.ballman wrote:
> whisperity wrote:
> > @aaron.ballman I think I'll do something like `if (BiggerLength < 
> > Threshold) return false;`, how does that sound? The comparison is moot in 
> > that case, imagine having strings of 2 and 4, padded to 4, with a threshold 
> > of 6, for example.
> > 
> > That way even if someone accidentally makes `StringRef` overflow the 
> > buffer, we'll be safe on the call paths we have here.
> I think that'd make the behavior much more obvious, but should that be `<=`?
No, because being = to the threshold means that the common prefix/suffix is 
empty string. It'd make variables such as "A" and "X" deemed related. Generally 
not something we want, because that's too broad of an assumption that they are 
"meant to be used together". (Generally you shouldn't name variables like so, 
but still...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:384
+  // Pad the two strings to the longer length.
+  std::size_t BiggerLength = std::max(Str1.size(), Str2.size());
+  SmallString<32> S1PadE{Str1}, S2PadE{Str2};

whisperity wrote:
> aaron.ballman wrote:
> > whisperity wrote:
> > > @aaron.ballman I think I'll do something like `if (BiggerLength < 
> > > Threshold) return false;`, how does that sound? The comparison is moot in 
> > > that case, imagine having strings of 2 and 4, padded to 4, with a 
> > > threshold of 6, for example.
> > > 
> > > That way even if someone accidentally makes `StringRef` overflow the 
> > > buffer, we'll be safe on the call paths we have here.
> > I think that'd make the behavior much more obvious, but should that be `<=`?
> No, because being = to the threshold means that the common prefix/suffix is 
> empty string. It'd make variables such as "A" and "X" deemed related. 
> Generally not something we want, because that's too broad of an assumption 
> that they are "meant to be used together". (Generally you shouldn't name 
> variables like so, but still...)
Ah, that's a good point, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity updated this revision to Diff 331245.
whisperity marked 5 inline comments as done.
whisperity added a comment.

- Made a precondition check explicit, and added an early `return` to not 
evaluate a case where it would return `false` anyways...


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

https://reviews.llvm.org/D97297

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

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -2,7 +2,8 @@
 // RUN:   -config='{CheckOptions: [ \
 // RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
 // RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
-// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"} \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 0} \
 // RUN:  ]}' -- -x c
 
 #define bool _Bool
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-prefixsuffixname.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.NamePrefixSuffixSilenceDissimilarityTreshold, value: 1} \
+// RUN:  ]}' --
+
+namespace std {
+struct string {};
+} // namespace std
+class Matrix {};
+
+void test1(int Foo, int Bar) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent parameters of 'test1' of similar type ('int') are easily swapped by mistake [bugprone-easily-swappable-parameters]
+// CHECK-MESSAGES: :[[@LINE-2]]:16: note: the first parameter in the range is 'Foo'
+// CHECK-MESSAGES: :[[@LINE-3]]:25: note: the last parameter in the range is 'Bar'
+
+void test2(int A, int B) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent parameters of 'test2' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:16: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-3]]:23: note: the last parameter in the range is 'B'
+
+void test3(int Val1, int Val2) {} // NO-WARN.
+
+void test4(int ValA, int Valb) {} // NO-WARN.
+
+void test5(int Val1, int ValZ) {} // NO-WARN.
+
+void test6(int PValue, int QValue) {} // NO-WARN.
+
+void test7(std::string Astr, std::string Bstr) {} // NO-WARN.
+
+void test8(int Aladdin, int Alabaster) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 2 adjacent parameters of 'test8' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:16: note: the first parameter in the range is 'Aladdin'
+// CHECK-MESSAGES: :[[@LINE-3]]:29: note: the last parameter in the range is 'Alabaster'
+
+void test9(Matrix Qmat, Matrix Rmat, Matrix Tmat) {} // NO-WARN.
+
+void test10(int Something, int Other, int Foo, int Bar1, int Bar2, int Baz, int Qux) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: 4 adjacent parameters of 'test10' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-2]]:17: note: the first parameter in the range is 'Something'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'Bar1'
+//
+// CHECK-MESSAGES: :[[@LINE-5]]:58: warning: 3 adjacent parameters of 'test10' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-6]]:62: note: the first parameter in the range is 'Bar2'
+// CHECK-MESSAGES: :[[@LINE-7]]:81: note: the last parameter in the range is 'Qux'
+
+void test11(int Foobar, int Fo