[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG73e4b5cfa8ea: [clang-tidy] Add

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

aaron.ballman accepted this revision. aaron.ballman added a comment. In D20689#2874473 , @whisperity wrote: > Bump.  I would prefer this check to be able to go into Clang-Tidy 13 if it's > okay, together with the other check I implemented. @alexfh has

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity added a comment. Bump.  I would prefer this check to be able to go into Clang-Tidy 13 if it's okay, together with the other check I implemented. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D20689/new/ https://reviews.llvm.org/D20689

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

aaron.ballman added a comment. I think this looks good to me. @alexfh, you had raised questions about this meeting the quality bar. I think those concerns are valid in the abstract (swapped argument checking requires heuristics), but I'd argue that most existing code bases that actually have

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity updated this revision to Diff 354867. whisperity added a comment. **NFC** Fix `ReleaseNotes` entry getting out of place during rebases earlier. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D20689/new/ https://reviews.llvm.org/D20689

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity updated this revision to Diff 354856. whisperity marked 2 inline comments as done. whisperity added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. - Fix comments and code according to comments - Turned `Substring` into a case-insensitive heuristic. -

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity marked an inline comment as done. whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:694-695 + + for (std::size_t I = 0, E = CalleeFuncDecl->getNumParams(); I != E; ++I) { +if (const auto *Param

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity marked 12 inline comments as done. whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:408-409 + + bool IsParamContinuouslyConst = + !IsParamReference ||

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:185 +for (std::size_t J = 0; J < Param.size(); ++J) { + if (Arg[I] == Param[J]) { +if (I == 0 || J == 0) Should this be a

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity updated this revision to Diff 354467. whisperity added a comment. **NFC** Rebase and fix things broken by D104819 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D20689/new/

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity updated this revision to Diff 339625. whisperity added a comment. **NFC** Fix the header guard name lint again, hopefully to the proper value this time. I forgot the `CHECK` out of it... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity updated this revision to Diff 339533. whisperity edited the summary of this revision. whisperity added a comment. - **NFC** Fix lint of header guard nomenclature - **NFC** Tear out obsolete `LLVM_DEBUG` calls from the implementation Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity added a comment. @aaron.ballman @alexfh Bump! How shall we move further? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D20689/new/ https://reviews.llvm.org/D20689 ___ cfe-commits mailing list

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity updated this revision to Diff 336625. whisperity edited the summary of this revision. whisperity added a comment. Rebase over D98635 . Highlighting only the parameter's name, not the entire range of the parameter. Repository: rG LLVM Github

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:347 +// Checks whether ArgType converts implicitly to ParamType. +static bool areTypesCompatible(QualType ArgType, QualType ParamType, +

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity updated this revision to Diff 331560. whisperity marked 2 inline comments as done. whisperity added a comment. - **NFC** Fixed some nits - Added a new check option, //`MinimumIdentifierNameLength`// instead of a hardcoded `3` value. Defaults to `3`. - Fixed an issue with heuristics

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity marked 12 inline comments as done. whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:657-660 +} else { + ParamTypes.push_back(QualType()); + ParamNames.push_back(StringRef()); +}

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:347-349 +static bool areTypesCompatible(QualType ArgType, QualType ParamType, + const ASTContext ) { + if (ArgType.isNull()

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity marked an inline comment as done. whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:657-660 +} else { + ParamTypes.push_back(QualType()); + ParamNames.push_back(StringRef()); +}

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:51 +{true, 40, 50}, // Substring. +{true, 50, 66}, // Levenshtein. +{true, 75, 85}, // Jaro-Winkler. varjujan

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:31-32 + + char DissimilarBelow; + char SimilarAbove; + aaron.ballman wrote: > `signed char` since we're doing `> -1` below? Or better yet,

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity updated this revision to Diff 331532. whisperity marked 3 inline comments as done. whisperity added a comment. **NFC** Made the code more legible, updated and clarified some comments, fixed grammar issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

varjujan added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:51 +{true, 40, 50}, // Substring. +{true, 50, 66}, // Levenshtein. +{true, 75, 85}, // Jaro-Winkler. whisperity wrote: >

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity marked 8 inline comments as done. whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:51 +{true, 40, 50}, // Substring. +{true, 50, 66}, // Levenshtein. +{true, 75, 85}, //

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

aaron.ballman added a comment. Pinging @alexfh for opinions about the check (especially any concerns about true or false positive rates). I continue to think this is a good check that's worth moving forward on. Comment at:

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity updated this revision to Diff 329666. whisperity edited the summary of this revision. whisperity added reviewers: hokein, njames93. whisperity added a comment. - Massively refactored and modernised the implementation - Removed spaghetti code related to the check options and made their

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity added a comment. (Typing this in here so it is not forgotten.) @aaron.ballman suggested in http://reviews.llvm.org/D69560#inline-893813 that a case of argument and parameter swaps could be discovered between forward declarations and the definitions, i.e. void fn(int x, int y);

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

aaron.ballman added a comment. In D20689#2457808 , @whisperity wrote: > Right, let's bump. Thank you for all of the detailed information on the performance of the check! I worked on a similar check in a recent past life and my intuition is that over a

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity added a comment. But of course, after having written all of that, I forgot to upload the results themselves...  F14738535: SuspiciousCall.sqlite Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity added a reviewer: aaron.ballman. whisperity added a comment. Right, let's bump. I've checked the output of the checker on a set of test projects (our usual testbed, I should say) which contains around 15 projects, half of which was C and the other half C++. All projects were analysed

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity commandeered this revision. whisperity added a reviewer: barancsuk. whisperity added a comment. Herald added subscribers: martong, Charusso, gamesh411, Szelethus. Assuming direct control. Previous colleagues and university mates departed for snowier pastures, time to try to do

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

whisperity updated this revision to Diff 259508. whisperity retitled this revision from "[clang-tidy] Suspicious Call Argument checker" to "[clang-tidy] Add 'readability-suspicious-call-argument' check". whisperity edited the summary of this revision. whisperity removed reviewers: varjujan,