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

[whisperity]

Summary

Find function calls where the call arguments might be provided in an incorrect order.
The check works by comparing the name of the arguments with the name of the parameters in the visible function declaration called.
A diagnostic is emitted if an argument name is similar to another parameter more than the one it is passed to currently, while also being dissimilar enough from the current one.

Several string metrics are implemented, and each has thresholds configurable by the user.

As this is a heuristics-based check, no FixIts are generated, on purpose.
False-positive (in the sense that the diagnostic isn't indicating an actual swap that was done) warnings from this check are still useful for developers as the findings indicate potential bad naming conventions used for variables and parameters.

---

Originally implemented by @varjujan as his Master's Thesis work.
The check was subsequently taken over by @barancsuk who added type conformity checks to silence false positive matches.

[alexfh]

Thank you for the new check!

Before starting with the review, I'd like to clarify one important thing. It's not immediately obvious that the pattern the check detects is actually a good indicator of a programming mistake. Did you try to run the check on a large enough codebase (at least, on the whole LLVM project) and analyze the results? http://clang.llvm.org/extra/clang-tidy/#running-clang-tidy-on-llvm describes the recommended way to run clang-tidy on LLVM.

[varjujan]

Yes, I did. The results from running the checker on LLVM are in the attached file. Sadly, I could'nt find any real mistakes but as I wrote in the summary, false positives can still indicate bad naming convention for some variables.

result.txt66 KBDownload

[alexfh]

In D20689#443266, @varjujan wrote:

Yes, I did. The results from running the checker on LLVM are in the attached file. Sadly, I could'nt find any real mistakes but as I wrote in the summary, false positives can still indicate bad naming convention for some variables.

result.txt66 KBDownload

It looks like the check doesn't meet the quality bar, at least, in its current form. "Bad naming convention" is a very arguable thing and I'm not sure we can claim that the pattern detected by the check is somehow an indication of a bad naming convention.

[varjujan]

I have implemented some more heuristics to achieve better results.

[varjujan]

I ran the check on multiple projects and tried to categorize the warnings: real errors, false positives, naming errors and coincidences. The results are attached. I got no warnings on LLVM.

postgres19 KBDownload

linuxKernel514 BDownload

xerces2 KBDownload

libreOffice487 BDownload

[alexfh]

In D20689#633889, @varjujan wrote:

I ran the check on multiple projects and tried to categorize the warnings: real errors, false positives, naming errors and coincidences. The results are attached. I got no warnings on LLVM.

I didn't find any "real errors" in the files you posted, which means that either all of these projects are of extreme code quality or that the error is rather unlikely to happen.

Another concern is that the check seems to treat argument types in a rather primitive way, e.g. it doesn't consider any type conversions or promotions.

[xazax.hun]

I think this might be better as a readability checker to find misleading variable or parameter names.

It would also be great to consider types. Unfortunately it probably means reimplementing some of the logic from Sema, since that information is not available at this point.

@varjujan
Do you actually use all of the heuristics that are implemented?

[varjujan]

@xazax.hun
Yes I do. Obviously some of them seem to be better than the others so I can remove a couple if needed.

[barancsuk]

Since the previous author, @varjujan is not available anymore, I take over the role of the author of this revision.

[barancsuk]

Major changes that have been made since the last update are as follows:

• The checker is moved from the module misc to readability
• It is checked, whether implicit type conversion is possible from the argument to the other parameter. The following conversion rules are considered:
  • Arithmetic type conversions
  • Pointer to pointer conversion (for multilevel pointers and also base/derived class pointers)
  • Array to pointer conversion
  • Function to function-pointer conversion
  • CV-qualifier compatibility is checked as well.

• The calculation of a heuristic`s result and the comparison of this result with the corresponding threshold value are performed by the same method, the heuristic`s function.
• The heuristic`s function is called only if the heuristic itself is turned on by the configuration settings.

Remark:
Implicit conversion rules of C are not checked separately, because, as I experienced when testing the checker on a larger code base, deeming all pointers convertible results in several false positives.

[barancsuk]

Check if argument and parameter numbers differ, add test cases for functions with default parameters

[barancsuk]

@alexfh, would you mind taking a look at the changes that have been introduced in the new patch?

The main improvements are:

• The checker has been shifted to the module readability.
• It is checked, whether implicit type conversion is possible from the argument to the parameter.

I have run the modified checker on some large code bases with the following results:
(The errors were categorized subjectively.)

• PostgreSQL: 32 renaming opportunities of 39 warnings
• Cpython: 10 renaming opportunities of 15 warnings
• Xerces: 6 renaming opportunities of 8 warnings
• FFmpeg: 5 renaming opportunities of 9 warnings
• OpenSSL: 3 renaming opportunities of 4 warnings
• LLVM: 20 renaming opportunities of 44 warnings

This article provides some evidence to support the feasibility of the checker as well.

icse2016-names.pdf426 KBDownload

The authors have proven that argument names are generally very similar to the corresponding parameters' names.
The presented empirical evidence also shows, that argument and parameter name dissimilarities are strong indicators of incorrect argument usages, or they identify renaming opportunities to improve code readability.
Moreover, the authors have even found 3 existing bugs in open source projects.

[alexfh]

In D20689#871947, @barancsuk wrote:

@alexfh, would you mind taking a look at the changes that have been introduced in the new patch?

The main improvements are:

• The checker has been shifted to the module readability.
• It is checked, whether implicit type conversion is possible from the argument to the parameter.

I have run the modified checker on some large code bases with the following results:
(The errors were categorized subjectively.)

• PostgreSQL: 32 renaming opportunities of 39 warnings
• Cpython: 10 renaming opportunities of 15 warnings
• Xerces: 6 renaming opportunities of 8 warnings
• FFmpeg: 5 renaming opportunities of 9 warnings
• OpenSSL: 3 renaming opportunities of 4 warnings
• LLVM: 20 renaming opportunities of 44 warnings

Is there a list of all the warnings? I'd like to take a closer look.

This article provides some evidence to support the feasibility of the checker as well.

icse2016-names.pdf426 KBDownload

The authors have proven that argument names are generally very similar to the corresponding parameters' names.
The presented empirical evidence also shows, that argument and parameter name dissimilarities are strong indicators of incorrect argument usages, or they identify renaming opportunities to improve code readability.
Moreover, the authors have even found 3 existing bugs in open source projects.

[barancsuk]

I attached the results of the tests.
The warnings are categorized into false positives and renaming opportunities.

PostgreSQL6 KBDownload

FFmpeg2 KBDownload

LLVM9 KBDownload

OpenSSL1 KBDownload

Xerces2 KBDownload

Cpython3 KBDownload

[barancsuk]

@alexfh, have you had a chance to look at the results yet?

[alexfh]

Sorry, I lost this patch. I've looked at the results and it still seems that the signal-to-noise ratio is quite low. There's definitely potential in using parameter name and argument spelling to detect possibly swapped arguments, and there's a recent research on this topic, where authors claim to have reached the true positive rate of 85% with decent recall. See https://research.google.com/pubs/pub46317.html. If you're interested in working on this check, I would suggest at least looking at the techniques described in that paper.

[whisperity]

I have developed a related check in D69560. That one considers types, but is an interface rule checker, and does not consider (any) potential call sites. Moreover, it does not consider "swaps" that happen across a function call, only, as the name implies, adjacent similar-type ranges.

Maybe one could lift the "is-similar-type", or rather, "is-accidentally-mixable-type" related ruling to some common location, and use type similarity as a precondition gate in the reports of this check?

docs/clang-tidy/checks/misc-redundant-expression.rst

18 ↗

(On Diff #113830)

This seems to be an unrelated diff.

test/clang-tidy/misc-redundant-expression.cpp

20 ↗

(On Diff #113830)

This entire file seems to be unrelated to the discussion at hand, perhaps a rebase went sideways?

[whisperity]

Assuming direct control. Previous colleagues and university mates departed for snowier pastures, time to try to do something with this check.

[whisperity]

First things first, we were 50 thousand (!) patches behind reality. Rebased to master. Fixed it to compile, too. Otherwise, NFC so far.

[whisperity]

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 independently in a virtual machine. All projects were analysed after checking out the latest release tag that was closest to Jan 2019. (I know this sounds dated, but the whole reproduction image was originally constructed for another checker I'm working on, and I did not wish to re-do the whole "What is needed to install this particular version?" pipeline.) The more important thing is that releases were analysed, which should, in theory, under-approximate the findings because releases are generally more polished than in-the-works code. The check was run as-is (sans a minor rebase issue that does not affect functionality) currently in the patch, namely, Diff 259508.

In total, I had received 194 reports (129 unique (1)). From this, I had found 8 (7 unique) true positives (marked Confirmed bug in CodeChecker terms), where something is visibly wrong with the call.
From this, there were 3 in a project where the called function's comment said that "The order of arguments <> and <> does not matter.", however, because this can never be figured out totally from the checker, I regarded the swapped call as a true positive. The fact that they had to seemingly create, inside the called function, some logic to detect and handle the swap shows that something was going wrong with the code's design for a long time.

In addition to these findings, I have identified 122 (75 unique) function call sites where the report about the potential swap (and the similarity to a different parameter name) is justifiable because the understandability of the code (especially to someone who is an outsider from virtually all of the projects analysed (2)) is hindered by the poor choice of argument or parameter names. The conclusions from these cases (marked Intentional in the CodeChecker database) are consistent with those drawn in [Pradel2013] and [Liu2016].

Now, onto the false positives. There were 64 (47 unique) cases. However, these cases can be further broken down into different categories, which I wasn't able to tally easily as CodeChecker only supports 3 unique categories, not infinite ones.

• Some of the false positives are what one would say "borderline": if the person reading the code reads it accurately, the reported "swap" does not fall into the understandability issue category. However, a famous case of this is from Postgres: reloid (IIRC, meaning relation owner id) and roleid (role (user) id) are the names of args/params of some functions. They are not swapped in the calls that exist in the code, but the similarity (swapping only 2 letters) makes it very easy to typo or misread the thing. In Postgres, there were 7 such cases.
• Approximately 5+5 cases are false positives but can be dealt with in heuristics. However, across the 17 projects, they do not account for a sizeable amount of cases.
  • Recursive function calls should be ignored. (This was mentioned in [Rice2017].)
  • Swapped and not-swapped calls appearing close to one another (i.e. constructs like b ? f(x,y) : f(y,x)) should be ignored too. This seems a bit harder to implement.
• There is a bug in the check's current implementation when calls from/to operator() is handled. (3) I'll look into fixing this.
• Binary operators should be ignored from reporting in general. Their parameters tend to have generic names, and the reports created from them is confusing.

Another generic observation is that the check's output is pretty wonky and hard to read at a glance, but this should be easy to fix. In addition, the observation of [Rice2017] about ignoring "patterned names" (i.e. arg1, arg2, ...) seems like a useful thing to add to the implementation, even though I had no findings at all where "ignoring patterned names" would've squelched a false positive report.

Agreeably, this check is limited compared to the previously linked [Rice2017], as it only checks the names in the call, not all variables in the surrounding context.

---

(1): I'm not exactly sure as to what "report uniqueing" in CodeChecker precisely does these days, but basically it uses the context of the bug and the checker message and whatnot to create a hash for the bug - "unique reports" mode shows each report belonging to the same hash only once.
(2): From the set of test projects, I only have some hands-on experience with parts of LLVM and Apache Xerces.
(3): Codes similar in nature to the following example of exact value forwarding to the call operator seems to still trigger the check. I have yet to actually pin down what causes this.

struct S
{
    int operator()(int a, int b, const char* c, double d);
};

struct T
{
    S* s;
    int operator(int a, int b, const char* c, double d)
    {
        return (*s)(a, b, c, d);
    }
};

[Pradel2013]: Michael Pradel, and Thomas R. Gross: Name-based analysis of equally typed method arguments. In: IEEE Transactions on Software Engineering, 39(8), pp. 1127-1143, 2013.
[Liu2016]: Hiu Liu, et al.: Nomen est Omen: Exploring and exploiting similarities between argument and parameter names. In: 38th IEEE International Conference on Software Engineering, pp. 1063-1073, 2016.
[Rice2017]: Andrew Rice, et al.: Detecting argument selection defects. In: Proceedings of the ACM on Programming Languages, 1, pp. 104:1-104:23, 2017.

[whisperity]

But of course, after having written all of that, I forgot to upload the results themselves... 😲

SuspiciousCall.sqlite32 MBDownload

[aaron.ballman]

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 large corpus of code, this will still have quite a bit of false positives and false negatives. However, I think those can likely be handled by post-commit improvements like noticing abbreviations (def vs define) or synonyms (number vs numeral), being smarter about patterned names, etc. I think this check is worth moving forward with, but I'm not certain how @alexfh feels about it.

[whisperity]

(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);

void fn(int y, int x) { ... } // Proparly mistakenly swapped names!

The conclusion was that this check (given this one does string distance, metrics, etc.) should be extended with such functionality. However, we need a better name for the check in that case!

[whisperity]

• Massively refactored and modernised the implementation
• Removed spaghetti code related to the check options and made their storage and defaults clearer
• Fixed modelling issues that caused false positives around lambdas, overloaded call operators, recursive calls, enumconstant arguments
• Made the abbreviation dictionary for the Abbreviations heuristic a check-option
• Made the check message much more legible
• Fixed a severe modelling issue about not respecting or handling record types passed by copy (CXXConstructExpr)
• Wrote the documentation for real, including all heuristics, default values, options, etc. The original documentation was basically empty. For the docs, I used @varjujan's thesis (which subject was this check), which is unfortunately only available in Hungarian, but it helped me understand what some of the enums and the Bound doing on the inside.
• Thanks to having written a proper documentation, I also renamed a few symbols and check options to better tell what they are about.

[aaron.ballman]

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.

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

32–33

signed char since we're doing > -1 below? Or better yet, int8_t because these aren't really characters?

52

We should probably document where all these numbers come from, but 66 definitely jumps out at me as being a bit strange. :-D

94

103–109

272–273

297

302–303

Can re-flow the comments.

342

Elsewhere we're using isPointerType() which is subtly different because it excludes ObjC object pointers. We should be consistent about the usage.

348

It seems like we're doing an awful lot of the same work as ASTContext::typesAreCompatible() and type compatibility rules are pretty complex, so I worry about this implementation being different than the ASTContext implementation. Have you explored whether we can reuse more of the logic from ASTContext here, or are they doing fundamentally different kinds of type compatibility checks?

463

468

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.h

39

The comment is somewhat confusing because the enumerators have the values 0 and 1, which are valid percentages but not likely what the comment means.

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

52

Unfortunately, I have absolutely no idea. All these values are percentages between 0 and 100 (-1 is just saying that "This heuristic doesn't accept percentages"), and this is written in the documentation now. However, the answer to "why 66%?", unless @varjujan can say something, I think is lost to history...

I'll read over his thesis once again, maybe I can find anything with regards to this.

Either way, I've detailed from both the code and the thesis how the percentages are meant. In some cases, the % is calculated as "% of the longer string's length". In the Leventhstein's case, it's actually inverted:

Dist = (1 - Dist / LongerLength) * 100;

So what this says is that if the current arg1-param1 arg2-param2 pairing has less than the inverse of 50% (which is more than 50%) of the longer string's edit distance, but the arg2-param1 and arg1-param2 (the suggested swapped order) has more than the inverse of 66% (which is less than 33%), then the swap will be suggested.

Originally these values were called LowerBound and UpperBound, respectively, which was saying even less about what they mean...

272–273

Good catch! I believe this is what happens when you write LaTeX and code at the same time? I didn't notice this when I was tidying up the code...

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

348

No, I didn't know that function even existed. This check must be older than that function.

[varjujan]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

52

Sadly, I think there isn't any scientific reason behind these numbers. They just looked ok after a couple of test runs. (Maybe they make sense for shorter arg names, like the 66 for 3 char long names.)

[whisperity]

NFC Made the code more legible, updated and clarified some comments, fixed grammar issues.

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

32–33

Oh right, I always forget char isn't guaranteed. int8_t seems like a better idea anyways (until we have int6_t...)

52

In [Rice2017], the inflexion point of the precision/recall plot is at around 0.55-ish threshold. They express this threshold as the distance of distances, i.e. 0 would mean the (a1, p1) - (a2, p2) pair is good as it is, and 1 would mean that it should definitely be (a2, p1) - (a1, p2) instead.

348

Actually, no, that function is pretty old... However, that function, and all the function it subsequently calls, require a non-const ASTContext. I have changed ASTContext:

╰─ git diff --cached --stat
 clang/include/clang/AST/ASTContext.h                                     | 67 ++++++++++++++++++++++++++++++++++++-------------------------------
 clang/lib/AST/ASTContext.cpp                                             | 72 +++++++++++++++++++++++++++++++++++++-----------------------------------
 2 files changed, 73 insertions(+), 66 deletions(-)

making related member functions and internal static functions take const ASTContext &/*.

This, by itself, did not break any of the tests of check-clang check-clang-unit check-clang-tools check-clang-extra-unit!

348–350

@aaron.ballman Changing the function to be

return Ctx.typesAreCompatible(ArgType, ParamType);

will make the checker miss the test case about T/const T& mixup.

void value_const_reference(int llllll, const int& kkkkkk);
void const_ref_value_swapped() {
  const int& kkkkkk = 42;
  const int& llllll = 42;
  value_const_reference(kkkkkk, llllll);
  // error: CHECK-MESSAGES: expected string not found in input:
  // warning: 1st argument 'kkkkkk' (passed to 'llllll') looks like it might be swapped with the 2nd, 'llllll' (passed to 'kkkkkk')
}

---

Setting bool CompareUnqualified = true (3rd argument to typesAreCompatible) doesn't help either.
Which is valid from typesAreCompatible's perspective... that function answer the question, applied to the context of the above test: "Is llllll = kkkkkk; valid?", which is obviously false as both are const T&s.

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.h

39

I'll rename it to BoundKind and update the comments.

[aaron.ballman]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

52

Thank you for the explanations! I'm fine with the values as they are (they're defaults that can be changed anyway).

165

348

Doubtful -- typesAreCompatible() is critical for checking the semantics of assignment in C, overloading in C++, etc. It may have simply been overlooked when writing this check. Given the complexities of type checking, my intuition is that we should be leaning on ASTContext for as much of this functionality as we can get away with. That will also get us nice "extras" like caring about address spaces, ARC, etc which are what got me worried when I started looking at this implementation.

625

TIL about %ordinal in diagnostics, thanks for that! :-D

633–635

645

653–657

658–661

Can this case happen?

708–709

Should the length of what's considered "too short" be a configuration option? I think 3 is a good default.

740

Otherwise it looks like we could read an uninitalized value later (if GotBound was None).

740–742

There's some type confusion going on here between unsigned char and char -- I think all of these uses should switch to uint8_t or int8_t (or possibly just int given that these values all wind up being promoted to int anyway).

744–745

Any reason not to move this below the switch and use = instead of |= within the cases? (Or return from the cases directly?)

clang-tools-extra/docs/clang-tidy/checks/readability-suspicious-call-argument.rst

62–69

I wonder how Hungarian notation impacts this heuristic -- I would imagine a lot of similar prefixes in such a code base, and things like lpstr as a prefix could be a pretty large chunk of some identifiers.

76

Similar to above, I wonder how numeric digits impact this heuristic -- do the defaults consider this to be a swap?

void foo(int frobble1, int frobble2);
foo(frobble2, frobble1); // Hopefully identified as a swap
foo(bar2, bar1); // How about this?

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

658–661

Oops... It seems I posted the updated patch right where you were writing more comments and we got into a data race. Which case are you referring to? It's now affixed to a diag( call for me...

clang-tools-extra/docs/clang-tidy/checks/readability-suspicious-call-argument.rst

62–69

The switch is only warned if it would be type-safe. If the HN prefix is in both the same way, then it could be ignored. Thus, given f(const char* lpszFoo, const char* lpszBar, uint16_t psnzXXX) {}, if I do a f(lpszX, lpszA, ...);, it should consider in both cases that the prefix is common and matches. Note that to produce a diagnostic, two things has to be proven: first, that the current ordering is dissimilar (below threshold A), and second, that the potential swapped ordering is more similar (above threshold B).

76

Currently, neither of these are matched. I have to look into why the first isn't... it really should, based on the "equality" heuristic. It's too trivial.

The second... well... that's trickier. I would say it shouldn't match, because if it did, we would be swamped with false positives. The suffix is only 1 character, and we need 25/30% based on the string's length.

[aaron.ballman]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

348–350

Yeah, I expect there to be a delta between the work this check is doing and the existing work done by typesAreCompatible(). However, given the complexity of type compatibility checking, I'd say it's better for us to try to refactor the ASTContext functionality so that we can share as much of the implementation as plausible rather than duplicate some really difficult logic in the tidy check.

658–661

Hehe, it's "fun" when the comments move around like this, isn't it? :-D I meant the else clause in:

for (std::size_t I = 0, E = CalleeFuncDecl->getNumParams(); I != E; ++I) {
  if (const ParmVarDecl *Param = CalleeFuncDecl->getParamDecl(I)) {
    ParamTypes.push_back(Param->getType());

    if (IdentifierInfo *II = Param->getIdentifier()) {
      ParamNames.push_back(II->getName());
    } else {
      ParamNames.push_back(StringRef());
    }
  } else { // This seems like it should be impossible, no?
    ParamTypes.push_back(QualType());
    ParamNames.push_back(StringRef());
  }
}

clang-tools-extra/docs/clang-tidy/checks/readability-suspicious-call-argument.rst

76

I agree that the first one should be caught and I also agree that the second one is tricky but that matching it would likely increase false positives.

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

658–661

Oh, nevermind, there is a button that shows me the older diff where it's aligned properly.

And yeah, it seems it can't, getParamDecl always returns a ParmVarDecl. Weird issues might arise when the vectors that are built here get out of sync (such as the issue we had with operator() calls before I fixed it!), so I understood the reason behind keeping the two functions parallel with each other in terms of pure visuals, even.

744–745

Returning from the cases directly is a bad idea because we want to try all heuristics and only say false if none of them matches.
But this break in a very bad location, I agree.

[whisperity]

• NFC Fixed some nits
• Added a new check option, MinimumIdentifierNameLength instead of a hardcoded 3 value. Defaults to 3.
• Fixed an issue with heuristics matching only in one direction but not in the other direction silenced a warning that clearly should have been there.

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

348

I talked with @aaron.ballman in private about this a little, and unfortunately, the route to call typesAreCompatible() is no dice. First things first, in C++ mode, that function just early returns (essentially) == on the type. Otherwise, it would call QualType ASTContext::mergeTypes(QualType, QualType, ...). Okay, let's delve into this thing. Now, first things first, somewhere down the line if you happen to give it something C++-specific (e.g. LValueReferenceType), it will just assert into your face. (Because C++ things are only compatible if the types are equal, most likely. In C++ mode, mergeType shouldn't be, and doesn't get, called.)

But there are even more issues with this function. Put simply, it does not perform any "implicit conversions". Sic!, I'm putting it in between quotes, but the fact is that if one type is const and the other isn't, it will just say "Nope, these are not compatible.".
So modelling the whole idea of "is passing an argument of type T acceptable to a parameter of type U?" is not possible with these functions.

So even though typesAreCompatible()'s documentation comment says:

Compatibility predicates used to check assignment expressions.

by far and large, we are not having the right kinds of types in our hands (in terms of this check) to fall back to this logic.

I've done a skim of the codebase to find call sites for these functions and try to explore my way back and forth from there to see how this function should be used, and the results are daunting.

The image is from lib/Sema/SemaExpr.cpp somewhere along the lines of checking address spaces (for Obj-C, the vast majority of this function seems to only deal with Obj-C-specific things!), from checkConditionalPointerCompatibility. The call to mergeTypes() is in the bottom left, the right pane is the continuation below on the result here is used.

It seems to me (and many other call sites to this function tend to behave the same way) that Sema does a lot of casting-away-things (like constness) before it can reasonably call this function, and if the result comes back valid, this function (and many others) then just re-apply manually the things that were originally cast off.

Thus, I see that if we would call this function in the check, a lot (if not all, or maybe even more than now!) of the logic that is currently here would still have to remain to prefix and suffix the call to mergeTypes().

Just for the sake of trying it all out, I put together a version where I leave the handling of the "C++ stuff" (like references) to the checker, but the rest back to the ASTContext, but it didn't fall into place:

ParamType = PointerType 0x5627819d0930 'const int *'
`-QualType 0x5627819cfef1 'const int' const
  `-BuiltinType 0x5627819cfef0 'int'
ArgType = PointerType 0x562781a49290 'int *'
`-BuiltinType 0x5627819cfef0 'int'
mergeType(Param, Arg) = <<<NULL>>>
mergeType(Arg, Param) = <<<NULL>>>

348

╰─ git diff --cached --stat
 clang/include/clang/AST/ASTContext.h                                     | 67 ++++++++++++++++++++++++++++++++++++-------------------------------
 clang/lib/AST/ASTContext.cpp                                             | 72 +++++++++++++++++++++++++++++++++++++-----------------------------------
 2 files changed, 73 insertions(+), 66 deletions(-)

ASTContext_typesAreCompatible_const.patch17 KBDownload

[whisperity]

Rebase over D98635. Highlighting only the parameter's name, not the entire range of the parameter.

[whisperity]

@aaron.ballman @alexfh Bump! How shall we move further?

[whisperity]

• NFC Fix lint of header guard nomenclature
• NFC Tear out obsolete LLVM_DEBUG calls from the implementation

[whisperity]

NFC Fix the header guard name lint again, hopefully to the proper value this time. I forgot the CHECK out of it...

[whisperity]

NFC Rebase and fix things broken by D104819.

[aaron.ballman]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

186

Should this be a case insensitive comparison?

208

269

300

372

404

409–410

Should this move down closer to where it's used?

426–428

438

454

468

It would be good to have a test case involving private inheritance.

580

589

695–696

Range-based for loop over CalledFuncDecl->params()?

802

This looks pretty reachable to me in the case where there's no bound.

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

409–410

Sure!

802

I'm not sure if that is the case. I added the llvm_unreachable so we don't get a warning about the function not having a return value on every code path. The switch covers all potential heuristics that are in the check right now, but if we add a new heuristic (to the enum) and forget to write it in, we will get a -Wswitch warning here. A default case doesn't apply here, further developers should be encouraged to wire new heuristics in properly.

[whisperity]

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

695–696

(Note: it's parameters() now, not params()!)

[whisperity]

• Fix comments and code according to comments
• Turned Substring into a case-insensitive heuristic.
• Added some missing tests.

[whisperity]

NFC Fix ReleaseNotes entry getting out of place during rebases earlier.

[aaron.ballman]

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 swapped args finds those bugs in production, so the rate of true positives on existing code is often small. I see the value from this check coming from checking new code as it's written, as part of a CI pipeline for instance. With that in mind, do you still have concerns about this check?

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

802

Oops, ignore my think-o! I somehow read the switch as switching over Threshold and not H and confused myself.

[whisperity]

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.

[aaron.ballman]

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 had a few weeks to respond, so I'm going to give my explicit LG just in case he isn't available to perform a review. Please give Alex until the end of the week to comment on here before landing, but given the considerable review and discussion on this already, I think we can address remaining concerns post-commit if he's unavailable for the moment.

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

add_clang_library(clangTidyReadabilityModule
   RedundantSmartptrGetCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantStringInitCheck.cpp
   SimplifyBooleanExprCheck.cpp
   SimplifySubscriptExprCheck.cpp
   StaticAccessedThroughInstanceCheck.cpp
   StaticDefinitionInAnonymousNamespaceCheck.cpp
   StringCompareCheck.cpp
+  SuspiciousCallArgumentCheck.cpp
   UniqueptrDeleteReleaseCheck.cpp
   UppercaseLiteralSuffixCheck.cpp
   UseAnyOfAllOfCheck.cpp
 
   LINK_LIBS
   clangTidy
   clangTidyUtils
 

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "RedundantStringInitCheck.h"
 #include "SimplifyBooleanExprCheck.h"
 #include "SimplifySubscriptExprCheck.h"
 #include "StaticAccessedThroughInstanceCheck.h"
 #include "StaticDefinitionInAnonymousNamespaceCheck.h"
 #include "StringCompareCheck.h"
+#include "SuspiciousCallArgumentCheck.h"
 #include "UniqueptrDeleteReleaseCheck.h"
 #include "UppercaseLiteralSuffixCheck.h"
 #include "UseAnyOfAllOfCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<RedundantSmartptrGetCheck>(
         "readability-redundant-smartptr-get");
     CheckFactories.registerCheck<RedundantStringCStrCheck>(
         "readability-redundant-string-cstr");
     CheckFactories.registerCheck<RedundantStringInitCheck>(
         "readability-redundant-string-init");
     CheckFactories.registerCheck<SimplifyBooleanExprCheck>(
         "readability-simplify-boolean-expr");
+    CheckFactories.registerCheck<SuspiciousCallArgumentCheck>(
+        "readability-suspicious-call-argument");
     CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
         "readability-uniqueptr-delete-release");
     CheckFactories.registerCheck<UppercaseLiteralSuffixCheck>(
         "readability-uppercase-literal-suffix");
     CheckFactories.registerCheck<UseAnyOfAllOfCheck>(
         "readability-use-anyofallof");
   }
 };

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.h

+//===--- SuspiciousCallArgumentCheck.h - clang-tidy -------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SUSPICIOUSCALLARGUMENTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SUSPICIOUSCALLARGUMENTCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "llvm/ADT/StringSet.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds function calls where the arguments passed are provided out of order,
+/// based on the difference between the argument name and the parameter names
+/// of the function.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-suspicious-call-argument.html
+class SuspiciousCallArgumentCheck : public ClangTidyCheck {
+  enum class Heuristic {
+    Equality,
+    Abbreviation,
+    Prefix,
+    Suffix,
+    Substring,
+    Levenshtein,
+    JaroWinkler,
+    Dice
+  };
+
+  /// When applying a heuristic, the value of this enum decides which kind of
+  /// bound will be selected from the bounds configured for the heuristic.
+  /// This only applies to heuristics that can take bounds.
+  enum class BoundKind {
+    /// Check for dissimilarity of the names. Names are deemed dissimilar if
+    /// the similarity measurement is **below** the configured threshold.
+    DissimilarBelow,
+
+    /// Check for similarity of the names. Names are deemed similar if the
+    /// similarity measurement (the result of heuristic) is **above** the
+    /// configured threshold.
+    SimilarAbove
+  };
+
+public:
+  static constexpr std::size_t SmallVectorSize = 8;
+  static constexpr std::size_t HeuristicCount =
+      static_cast<std::size_t>(Heuristic::Dice) + 1;
+
+  SuspiciousCallArgumentCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const std::size_t MinimumIdentifierNameLength;
+
+  /// The configuration for which heuristics were enabled.
+  SmallVector<Heuristic, HeuristicCount> AppliedHeuristics;
+
+  /// The lower and upper bounds for each heuristic, as configured by the user.
+  SmallVector<std::pair<int8_t, int8_t>, HeuristicCount> ConfiguredBounds;
+
+  /// The abbreviation-to-abbreviated map for the Abbreviation heuristic.
+  llvm::StringMap<std::string> AbbreviationDictionary;
+
+  bool isHeuristicEnabled(Heuristic H) const;
+  Optional<int8_t> getBound(Heuristic H, BoundKind BK) const;
+
+  // Runtime information of the currently analyzed function call.
+  SmallVector<QualType, SmallVectorSize> ArgTypes;
+  SmallVector<StringRef, SmallVectorSize> ArgNames;
+  SmallVector<QualType, SmallVectorSize> ParamTypes;
+  SmallVector<StringRef, SmallVectorSize> ParamNames;
+
+  void setParamNamesAndTypes(const FunctionDecl *CalleeFuncDecl);
+
+  void setArgNamesAndTypes(const CallExpr *MatchedCallExpr,
+                           std::size_t InitialArgIndex);
+
+  bool areParamAndArgComparable(std::size_t Position1, std::size_t Position2,
+                                const ASTContext &Ctx) const;
+
+  bool areArgsSwapped(std::size_t Position1, std::size_t Position2) const;
+
+  bool areNamesSimilar(StringRef Arg, StringRef Param, Heuristic H,
+                       BoundKind BK) const;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_SUSPICIOUSCALLARGUMENTCHECK_H

clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp

+//===--- SuspiciousCallArgumentCheck.cpp - clang-tidy ---------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "SuspiciousCallArgumentCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Type.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <sstream>
+
+using namespace clang::ast_matchers;
+namespace optutils = clang::tidy::utils::options;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+struct DefaultHeuristicConfiguration {
+  /// Whether the heuristic is to be enabled by default.
+  const bool Enabled;
+
+  /// The upper bound of % of similarity the two strings might have to be
+  /// considered dissimilar.
+  /// (For purposes of configuration, -1 if the heuristic is not configurable
+  /// with bounds.)
+  const int8_t DissimilarBelow;
+
+  /// The lower bound of % of similarity the two string must have to be
+  /// considered similar.
+  /// (For purposes of configuration, -1 if the heuristic is not configurable
+  /// with bounds.)
+  const int8_t SimilarAbove;
+
+  /// Can the heuristic be configured with bounds?
+  bool hasBounds() const { return DissimilarBelow > -1 && SimilarAbove > -1; }
+};
+} // namespace
+
+static constexpr std::size_t DefaultMinimumIdentifierNameLength = 3;
+
+static constexpr StringRef HeuristicToString[] = {
+    "Equality",  "Abbreviation", "Prefix",      "Suffix",
+    "Substring", "Levenshtein",  "JaroWinkler", "Dice"};
+
+static constexpr DefaultHeuristicConfiguration Defaults[] = {
+    {true, -1, -1}, // Equality.
+    {true, -1, -1}, // Abbreviation.
+    {true, 25, 30}, // Prefix.
+    {true, 25, 30}, // Suffix.
+    {true, 40, 50}, // Substring.
+    {true, 50, 66}, // Levenshtein.
+    {true, 75, 85}, // Jaro-Winkler.
+    {true, 60, 70}, // Dice.
+};
+
+static_assert(
+    sizeof(HeuristicToString) / sizeof(HeuristicToString[0]) ==
+        SuspiciousCallArgumentCheck::HeuristicCount,
+    "Ensure that every heuristic has a corresponding stringified name");
+static_assert(sizeof(Defaults) / sizeof(Defaults[0]) ==
+                  SuspiciousCallArgumentCheck::HeuristicCount,
+              "Ensure that every heuristic has a default configuration.");
+
+namespace {
+template <std::size_t I> struct HasWellConfiguredBounds {
+  static constexpr bool Value =
+      !((Defaults[I].DissimilarBelow == -1) ^ (Defaults[I].SimilarAbove == -1));
+  static_assert(Value, "A heuristic must either have a dissimilarity and "
+                       "similarity bound, or neither!");
+};
+
+template <std::size_t I> struct HasWellConfiguredBoundsFold {
+  static constexpr bool Value = HasWellConfiguredBounds<I>::Value &&
+                                HasWellConfiguredBoundsFold<I - 1>::Value;
+};
+
+template <> struct HasWellConfiguredBoundsFold<0> {
+  static constexpr bool Value = HasWellConfiguredBounds<0>::Value;
+};
+
+struct AllHeuristicsBoundsWellConfigured {
+  static constexpr bool Value =
+      HasWellConfiguredBoundsFold<SuspiciousCallArgumentCheck::HeuristicCount -
+                                  1>::Value;
+};
+
+static_assert(AllHeuristicsBoundsWellConfigured::Value, "");
+} // namespace
+
+static const std::string DefaultAbbreviations =
+    optutils::serializeStringList({"addr=address",
+                                   "arr=array",
+                                   "attr=attribute",
+                                   "buf=buffer",
+                                   "cl=client",
+                                   "cnt=count",
+                                   "col=column",
+                                   "cpy=copy",
+                                   "dest=destination",
+                                   "dist=distance"
+                                   "dst=distance",
+                                   "elem=element",
+                                   "hght=height",
+                                   "i=index",
+                                   "idx=index",
+                                   "len=length",
+                                   "ln=line",
+                                   "lst=list",
+                                   "nr=number",
+                                   "num=number",
+                                   "pos=position",
+                                   "ptr=pointer",
+                                   "ref=reference",
+                                   "src=source",
+                                   "srv=server",
+                                   "stmt=statement",
+                                   "str=string",
+                                   "val=value",
+                                   "var=variable",
+                                   "vec=vector",
+                                   "wdth=width"});
+
+static constexpr std::size_t SmallVectorSize =
+    SuspiciousCallArgumentCheck::SmallVectorSize;
+
+/// Returns how many % X is of Y.
+static inline double percentage(double X, double Y) { return X / Y * 100.0; }
+
+static bool applyEqualityHeuristic(StringRef Arg, StringRef Param) {
+  return Arg.equals_insensitive(Param);
+}
+
+static bool applyAbbreviationHeuristic(
+    const llvm::StringMap<std::string> &AbbreviationDictionary, StringRef Arg,
+    StringRef Param) {
+  if (AbbreviationDictionary.find(Arg) != AbbreviationDictionary.end() &&
+      Param.equals(AbbreviationDictionary.lookup(Arg)))
+    return true;
+
+  if (AbbreviationDictionary.find(Param) != AbbreviationDictionary.end() &&
+      Arg.equals(AbbreviationDictionary.lookup(Param)))
+    return true;
+
+  return false;
+}
+
+/// Check whether the shorter String is a prefix of the longer String.
+static bool applyPrefixHeuristic(StringRef Arg, StringRef Param,
+                                 int8_t Threshold) {
+  StringRef Shorter = Arg.size() < Param.size() ? Arg : Param;
+  StringRef Longer = Arg.size() >= Param.size() ? Arg : Param;
+
+  if (Longer.startswith_insensitive(Shorter))
+    return percentage(Shorter.size(), Longer.size()) > Threshold;
+
+  return false;
+}
+
+/// Check whether the shorter String is a suffix of the longer String.
+static bool applySuffixHeuristic(StringRef Arg, StringRef Param,
+                                 int8_t Threshold) {
+  StringRef Shorter = Arg.size() < Param.size() ? Arg : Param;
+  StringRef Longer = Arg.size() >= Param.size() ? Arg : Param;
+
+  if (Longer.endswith_insensitive(Shorter))
+    return percentage(Shorter.size(), Longer.size()) > Threshold;
+
+  return false;
+}
+
+static bool applySubstringHeuristic(StringRef Arg, StringRef Param,
+                                    int8_t Threshold) {
+
+  std::size_t MaxLength = 0;
+  SmallVector<std::size_t, SmallVectorSize> Current(Param.size());
+  SmallVector<std::size_t, SmallVectorSize> Previous(Param.size());
+  std::string ArgLower = Arg.lower();
+  std::string ParamLower = Param.lower();
+
+  for (std::size_t I = 0; I < Arg.size(); ++I) {
+    for (std::size_t J = 0; J < Param.size(); ++J) {
+      if (ArgLower[I] == ParamLower[J]) {
+        if (I == 0 || J == 0)
+          Current[J] = 1;
+        else
+          Current[J] = 1 + Previous[J - 1];
+
+        MaxLength = std::max(MaxLength, Current[J]);
+      } else
+        Current[J] = 0;
+    }
+
+    Current.swap(Previous);
+  }
+
+  size_t LongerLength = std::max(Arg.size(), Param.size());
+  return percentage(MaxLength, LongerLength) > Threshold;
+}
+
+static bool applyLevenshteinHeuristic(StringRef Arg, StringRef Param,
+                                      int8_t Threshold) {
+  std::size_t LongerLength = std::max(Arg.size(), Param.size());
+  double Dist = Arg.edit_distance(Param);
+  Dist = (1.0 - Dist / LongerLength) * 100.0;
+  return Dist > Threshold;
+}
+
+// Based on http://en.wikipedia.org/wiki/Jaro–Winkler_distance.
+static bool applyJaroWinklerHeuristic(StringRef Arg, StringRef Param,
+                                      int8_t Threshold) {
+  std::size_t Match = 0, Transpos = 0;
+  std::ptrdiff_t ArgLen = Arg.size();
+  std::ptrdiff_t ParamLen = Param.size();
+  SmallVector<int, SmallVectorSize> ArgFlags(ArgLen);
+  SmallVector<int, SmallVectorSize> ParamFlags(ParamLen);
+  std::ptrdiff_t Range =
+      std::max(std::ptrdiff_t{0}, std::max(ArgLen, ParamLen) / 2 - 1);
+
+  // Calculate matching characters.
+  for (std::ptrdiff_t I = 0; I < ParamLen; ++I)
+    for (std::ptrdiff_t J = std::max(I - Range, std::ptrdiff_t{0}),
+                        L = std::min(I + Range + 1, ArgLen);
+         J < L; ++J)
+      if (tolower(Param[I]) == tolower(Arg[J]) && !ArgFlags[J]) {
+        ArgFlags[J] = 1;
+        ParamFlags[I] = 1;
+        ++Match;
+        break;
+      }
+
+  if (!Match)
+    return false;
+
+  // Calculate character transpositions.
+  std::ptrdiff_t L = 0;
+  for (std::ptrdiff_t I = 0; I < ParamLen; ++I) {
+    if (ParamFlags[I] == 1) {
+      std::ptrdiff_t J;
+      for (J = L; J < ArgLen; ++J)
+        if (ArgFlags[J] == 1) {
+          L = J + 1;
+          break;
+        }
+
+      if (tolower(Param[I]) != tolower(Arg[J]))
+        ++Transpos;
+    }
+  }
+  Transpos /= 2;
+
+  // Jaro distance.
+  double MatchD = Match;
+  double Dist = ((MatchD / ArgLen) + (MatchD / ParamLen) +
+                 ((MatchD - Transpos) / Match)) /
+                3.0;
+
+  // Calculate common string prefix up to 4 chars.
+  L = 0;
+  for (std::ptrdiff_t I = 0;
+       I < std::min(std::min(ArgLen, ParamLen), std::ptrdiff_t{4}); ++I)
+    if (tolower(Arg[I]) == tolower(Param[I]))
+      ++L;
+
+  // Jaro-Winkler distance.
+  Dist = (Dist + (L * 0.1 * (1.0 - Dist))) * 100.0;
+  return Dist > Threshold;
+}
+
+// Based on http://en.wikipedia.org/wiki/Sørensen–Dice_coefficient
+static bool applyDiceHeuristic(StringRef Arg, StringRef Param,
+                               int8_t Threshold) {
+  llvm::StringSet<> ArgBigrams;
+  llvm::StringSet<> ParamBigrams;
+
+  // Extract character bigrams from Arg.
+  for (std::ptrdiff_t I = 0; I < static_cast<std::ptrdiff_t>(Arg.size()) - 1;
+       ++I)
+    ArgBigrams.insert(Arg.substr(I, 2).lower());
+
+  // Extract character bigrams from Param.
+  for (std::ptrdiff_t I = 0; I < static_cast<std::ptrdiff_t>(Param.size()) - 1;
+       ++I)
+    ParamBigrams.insert(Param.substr(I, 2).lower());
+
+  std::size_t Intersection = 0;
+
+  // Find the intersection between the two sets.
+  for (auto IT = ParamBigrams.begin(); IT != ParamBigrams.end(); ++IT)
+    Intersection += ArgBigrams.count((IT->getKey()));
+
+  // Calculate Dice coefficient.
+  return percentage(Intersection * 2.0,
+                    ArgBigrams.size() + ParamBigrams.size()) > Threshold;
+}
+
+/// Checks if ArgType binds to ParamType regarding reference-ness and
+/// cv-qualifiers.
+static bool areRefAndQualCompatible(QualType ArgType, QualType ParamType) {
+  return !ParamType->isReferenceType() ||
+         ParamType.getNonReferenceType().isAtLeastAsQualifiedAs(
+             ArgType.getNonReferenceType());
+}
+
+static bool isPointerOrArray(QualType TypeToCheck) {
+  return TypeToCheck->isPointerType() || TypeToCheck->isArrayType();
+}
+
+/// Checks whether ArgType is an array type identical to ParamType's array type.
+/// Enforces array elements' qualifier compatibility as well.
+static bool isCompatibleWithArrayReference(QualType ArgType,
+                                           QualType ParamType) {
+  if (!ArgType->isArrayType())
+    return false;
+  // Here, qualifiers belong to the elements of the arrays.
+  if (!ParamType.isAtLeastAsQualifiedAs(ArgType))
+    return false;
+
+  return ParamType.getUnqualifiedType() == ArgType.getUnqualifiedType();
+}
+
+static QualType convertToPointeeOrArrayElementQualType(QualType TypeToConvert) {
+  unsigned CVRqualifiers = 0;
+  // Save array element qualifiers, since getElementType() removes qualifiers
+  // from array elements.
+  if (TypeToConvert->isArrayType())
+    CVRqualifiers = TypeToConvert.getLocalQualifiers().getCVRQualifiers();
+  TypeToConvert = TypeToConvert->isPointerType()
+                      ? TypeToConvert->getPointeeType()
+                      : TypeToConvert->getAsArrayTypeUnsafe()->getElementType();
+  TypeToConvert = TypeToConvert.withCVRQualifiers(CVRqualifiers);
+  return TypeToConvert;
+}
+
+/// Checks if multilevel pointers' qualifiers compatibility continues on the
+/// current pointer level. For multilevel pointers, C++ permits conversion, if
+/// every cv-qualifier in ArgType also appears in the corresponding position in
+/// ParamType, and if PramType has a cv-qualifier that's not in ArgType, then
+/// every * in ParamType to the right of that cv-qualifier, except the last
+/// one, must also be const-qualified.
+static bool arePointersStillQualCompatible(QualType ArgType, QualType ParamType,
+                                           bool &IsParamContinuouslyConst) {
+  // The types are compatible, if the parameter is at least as qualified as the
+  // argument, and if it is more qualified, it has to be const on upper pointer
+  // levels.
+  bool AreTypesQualCompatible =
+      ParamType.isAtLeastAsQualifiedAs(ArgType) &&
+      (!ParamType.hasQualifiers() || IsParamContinuouslyConst);
+  // Check whether the parameter's constness continues at the current pointer
+  // level.
+  IsParamContinuouslyConst &= ParamType.isConstQualified();
+
+  return AreTypesQualCompatible;
+}
+
+/// Checks whether multilevel pointers are compatible in terms of levels,
+/// qualifiers and pointee type.
+static bool arePointerTypesCompatible(QualType ArgType, QualType ParamType,
+                                      bool IsParamContinuouslyConst) {
+  if (!arePointersStillQualCompatible(ArgType, ParamType,
+                                      IsParamContinuouslyConst))
+    return false;
+
+  do {
+    // Step down one pointer level.
+    ArgType = convertToPointeeOrArrayElementQualType(ArgType);
+    ParamType = convertToPointeeOrArrayElementQualType(ParamType);
+
+    // Check whether cv-qualifiers permit compatibility on
+    // current level.
+    if (!arePointersStillQualCompatible(ArgType, ParamType,
+                                        IsParamContinuouslyConst))
+      return false;
+
+    if (ParamType.getUnqualifiedType() == ArgType.getUnqualifiedType())
+      return true;
+
+  } while (ParamType->isPointerType() && ArgType->isPointerType());
+  // The final type does not match, or pointer levels differ.
+  return false;
+}
+
+/// Checks whether ArgType converts implicitly to ParamType.
+static bool areTypesCompatible(QualType ArgType, QualType ParamType,
+                               const ASTContext &Ctx) {
+  if (ArgType.isNull() || ParamType.isNull())
+    return false;
+
+  ArgType = ArgType.getCanonicalType();
+  ParamType = ParamType.getCanonicalType();
+
+  if (ArgType == ParamType)
+    return true;
+
+  // Check for constness and reference compatibility.
+  if (!areRefAndQualCompatible(ArgType, ParamType))
+    return false;
+
+  bool IsParamReference = ParamType->isReferenceType();
+
+  // Reference-ness has already been checked and should be removed
+  // before further checking.
+  ArgType = ArgType.getNonReferenceType();
+  ParamType = ParamType.getNonReferenceType();
+
+  if (ParamType.getUnqualifiedType() == ArgType.getUnqualifiedType())
+    return true;
+
+  // Arithmetic types are interconvertible, except scoped enums.
+  if (ParamType->isArithmeticType() && ArgType->isArithmeticType()) {
+    if ((ParamType->isEnumeralType() &&
+         ParamType->getAs<EnumType>()->getDecl()->isScoped()) ||
+        (ArgType->isEnumeralType() &&
+         ArgType->getAs<EnumType>()->getDecl()->isScoped()))
+      return false;
+
+    return true;
+  }
+
+  // Check if the argument and the param are both function types (the parameter
+  // decayed to a function pointer).
+  if (ArgType->isFunctionType() && ParamType->isFunctionPointerType()) {
+    ParamType = ParamType->getPointeeType();
+    return ArgType == ParamType;
+  }
+
+  // Arrays or pointer arguments convert to array or pointer parameters.
+  if (!(isPointerOrArray(ArgType) && isPointerOrArray(ParamType)))
+    return false;
+
+  // When ParamType is an array reference, ArgType has to be of the same-sized
+  // array-type with cv-compatible element type.
+  if (IsParamReference && ParamType->isArrayType())
+    return isCompatibleWithArrayReference(ArgType, ParamType);
+
+  bool IsParamContinuouslyConst =
+      !IsParamReference || ParamType.getNonReferenceType().isConstQualified();
+
+  // Remove the first level of indirection.
+  ArgType = convertToPointeeOrArrayElementQualType(ArgType);
+  ParamType = convertToPointeeOrArrayElementQualType(ParamType);
+
+  // Check qualifier compatibility on the next level.
+  if (!ParamType.isAtLeastAsQualifiedAs(ArgType))
+    return false;
+
+  if (ParamType.getUnqualifiedType() == ArgType.getUnqualifiedType())
+    return true;
+
+  // At this point, all possible C language implicit conversion were checked.
+  if (!Ctx.getLangOpts().CPlusPlus)
+    return false;
+
+  // Check whether ParamType and ArgType were both pointers to a class or a
+  // struct, and check for inheritance.
+  if (ParamType->isStructureOrClassType() &&
+      ArgType->isStructureOrClassType()) {
+    const auto *ArgDecl = ArgType->getAsCXXRecordDecl();
+    const auto *ParamDecl = ParamType->getAsCXXRecordDecl();
+    if (!ArgDecl || !ArgDecl->hasDefinition() || !ParamDecl ||
+        !ParamDecl->hasDefinition())
+      return false;
+
+    return ArgDecl->isDerivedFrom(ParamDecl);
+  }
+
+  // Unless argument and param are both multilevel pointers, the types are not
+  // convertible.
+  if (!(ParamType->isAnyPointerType() && ArgType->isAnyPointerType()))
+    return false;
+
+  return arePointerTypesCompatible(ArgType, ParamType,
+                                   IsParamContinuouslyConst);
+}
+
+static bool isOverloadedUnaryOrBinarySymbolOperator(const FunctionDecl *FD) {
+  switch (FD->getOverloadedOperator()) {
+  case OO_None:
+  case OO_Call:
+  case OO_Subscript:
+  case OO_New:
+  case OO_Delete:
+  case OO_Array_New:
+  case OO_Array_Delete:
+  case OO_Conditional:
+  case OO_Coawait:
+    return false;
+
+  default:
+    return FD->getNumParams() <= 2;
+  }
+}
+
+SuspiciousCallArgumentCheck::SuspiciousCallArgumentCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      MinimumIdentifierNameLength(Options.get(
+          "MinimumIdentifierNameLength", DefaultMinimumIdentifierNameLength)) {
+  auto GetToggleOpt = [this](Heuristic H) -> bool {
+    auto Idx = static_cast<std::size_t>(H);
+    assert(Idx < HeuristicCount);
+    return Options.get(HeuristicToString[Idx], Defaults[Idx].Enabled);
+  };
+  auto GetBoundOpt = [this](Heuristic H, BoundKind BK) -> int8_t {
+    auto Idx = static_cast<std::size_t>(H);
+    assert(Idx < HeuristicCount);
+
+    SmallString<32> Key = HeuristicToString[Idx];
+    Key.append(BK == BoundKind::DissimilarBelow ? "DissimilarBelow"
+                                                : "SimilarAbove");
+    int8_t Default = BK == BoundKind::DissimilarBelow
+                         ? Defaults[Idx].DissimilarBelow
+                         : Defaults[Idx].SimilarAbove;
+    return Options.get(Key, Default);
+  };
+  for (std::size_t Idx = 0; Idx < HeuristicCount; ++Idx) {
+    auto H = static_cast<Heuristic>(Idx);
+    if (GetToggleOpt(H))
+      AppliedHeuristics.emplace_back(H);
+    ConfiguredBounds.emplace_back(
+        std::make_pair(GetBoundOpt(H, BoundKind::DissimilarBelow),
+                       GetBoundOpt(H, BoundKind::SimilarAbove)));
+  }
+
+  for (const std::string &Abbreviation : optutils::parseStringList(
+           Options.get("Abbreviations", DefaultAbbreviations))) {
+    auto KeyAndValue = StringRef{Abbreviation}.split("=");
+    assert(!KeyAndValue.first.empty() && !KeyAndValue.second.empty());
+    AbbreviationDictionary.insert(
+        std::make_pair(KeyAndValue.first.str(), KeyAndValue.second.str()));
+  }
+}
+
+void SuspiciousCallArgumentCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "MinimumIdentifierNameLength",
+                MinimumIdentifierNameLength);
+  const auto &SetToggleOpt = [this, &Opts](Heuristic H) -> void {
+    auto Idx = static_cast<std::size_t>(H);
+    Options.store(Opts, HeuristicToString[Idx], isHeuristicEnabled(H));
+  };
+  const auto &SetBoundOpt = [this, &Opts](Heuristic H, BoundKind BK) -> void {
+    auto Idx = static_cast<std::size_t>(H);
+    assert(Idx < HeuristicCount);
+    if (!Defaults[Idx].hasBounds())
+      return;
+
+    SmallString<32> Key = HeuristicToString[Idx];
+    Key.append(BK == BoundKind::DissimilarBelow ? "DissimilarBelow"
+                                                : "SimilarAbove");
+    Options.store(Opts, Key, getBound(H, BK).getValue());
+  };
+
+  for (std::size_t Idx = 0; Idx < HeuristicCount; ++Idx) {
+    auto H = static_cast<Heuristic>(Idx);
+    SetToggleOpt(H);
+    SetBoundOpt(H, BoundKind::DissimilarBelow);
+    SetBoundOpt(H, BoundKind::SimilarAbove);
+  }
+
+  SmallVector<std::string, 32> Abbreviations;
+  for (const auto &Abbreviation : AbbreviationDictionary) {
+    SmallString<32> EqualSignJoined;
+    EqualSignJoined.append(Abbreviation.first());
+    EqualSignJoined.append("=");
+    EqualSignJoined.append(Abbreviation.second);
+
+    if (!Abbreviation.second.empty())
+      Abbreviations.emplace_back(EqualSignJoined.str());
+  }
+  Options.store(Opts, "Abbreviations",
+                optutils::serializeStringList(Abbreviations));
+}
+
+bool SuspiciousCallArgumentCheck::isHeuristicEnabled(Heuristic H) const {
+  return llvm::is_contained(AppliedHeuristics, H);
+}
+
+Optional<int8_t> SuspiciousCallArgumentCheck::getBound(Heuristic H,
+                                                       BoundKind BK) const {
+  auto Idx = static_cast<std::size_t>(H);
+  assert(Idx < HeuristicCount);
+
+  if (!Defaults[Idx].hasBounds())
+    return None;
+
+  switch (BK) {
+  case BoundKind::DissimilarBelow:
+    return ConfiguredBounds[Idx].first;
+  case BoundKind::SimilarAbove:
+    return ConfiguredBounds[Idx].second;
+  }
+  llvm_unreachable("Unhandled Bound kind.");
+}
+
+void SuspiciousCallArgumentCheck::registerMatchers(MatchFinder *Finder) {
+  // Only match calls with at least 2 arguments.
+  Finder->addMatcher(
+      functionDecl(forEachDescendant(callExpr(unless(anyOf(argumentCountIs(0),
+                                                           argumentCountIs(1))))
+                                         .bind("functionCall")))
+          .bind("callingFunc"),
+      this);
+}
+
+void SuspiciousCallArgumentCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedCallExpr =
+      Result.Nodes.getNodeAs<CallExpr>("functionCall");
+  const auto *Caller = Result.Nodes.getNodeAs<FunctionDecl>("callingFunc");
+  assert(MatchedCallExpr && Caller);
+
+  const Decl *CalleeDecl = MatchedCallExpr->getCalleeDecl();
+  if (!CalleeDecl)
+    return;
+
+  const FunctionDecl *CalleeFuncDecl = CalleeDecl->getAsFunction();
+  if (!CalleeFuncDecl)
+    return;
+  if (CalleeFuncDecl == Caller)
+    // Ignore recursive calls.
+    return;
+  if (isOverloadedUnaryOrBinarySymbolOperator(CalleeFuncDecl))
+    return;
+
+  // Get param attributes.
+  setParamNamesAndTypes(CalleeFuncDecl);
+
+  if (ParamNames.empty())
+    return;
+
+  // Get Arg attributes.
+  std::size_t InitialArgIndex = 0;
+
+  if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(CalleeFuncDecl)) {
+    if (MethodDecl->getParent()->isLambda())
+      // Lambda functions' first Arg are the lambda object.
+      InitialArgIndex = 1;
+    else if (MethodDecl->getOverloadedOperator() == OO_Call)
+      // For custom operator()s, the first Arg is the called object.
+      InitialArgIndex = 1;
+  }
+
+  setArgNamesAndTypes(MatchedCallExpr, InitialArgIndex);
+
+  if (ArgNames.empty())
+    return;
+
+  std::size_t ParamCount = ParamNames.size();
+
+  // Check similarity.
+  for (std::size_t I = 0; I < ParamCount; ++I) {
+    for (std::size_t J = I + 1; J < ParamCount; ++J) {
+      // Do not check if param or arg names are short, or not convertible.
+      if (!areParamAndArgComparable(I, J, *Result.Context))
+        continue;
+      if (!areArgsSwapped(I, J))
+        continue;
+
+      // Warning at the call itself.
+      diag(MatchedCallExpr->getExprLoc(),
+           "%ordinal0 argument '%1' (passed to '%2') looks like it might be "
+           "swapped with the %ordinal3, '%4' (passed to '%5')")
+          << static_cast<unsigned>(I + 1) << ArgNames[I] << ParamNames[I]
+          << static_cast<unsigned>(J + 1) << ArgNames[J] << ParamNames[J]
+          << MatchedCallExpr->getArg(I)->getSourceRange()
+          << MatchedCallExpr->getArg(J)->getSourceRange();
+
+      // Note at the functions declaration.
+      SourceLocation IParNameLoc =
+          CalleeFuncDecl->getParamDecl(I)->getLocation();
+      SourceLocation JParNameLoc =
+          CalleeFuncDecl->getParamDecl(J)->getLocation();
+
+      diag(CalleeFuncDecl->getLocation(), "in the call to %0, declared here",
+           DiagnosticIDs::Note)
+          << CalleeFuncDecl
+          << CharSourceRange::getTokenRange(IParNameLoc, IParNameLoc)
+          << CharSourceRange::getTokenRange(JParNameLoc, JParNameLoc);
+    }
+  }
+}
+
+void SuspiciousCallArgumentCheck::setParamNamesAndTypes(
+    const FunctionDecl *CalleeFuncDecl) {
+  // Reset vectors, and fill them with the currently checked function's
+  // parameters' data.
+  ParamNames.clear();
+  ParamTypes.clear();
+
+  for (const ParmVarDecl *Param : CalleeFuncDecl->parameters()) {
+    ParamTypes.push_back(Param->getType());
+
+    if (IdentifierInfo *II = Param->getIdentifier())
+      ParamNames.push_back(II->getName());
+    else
+      ParamNames.push_back(StringRef());
+  }
+}
+
+void SuspiciousCallArgumentCheck::setArgNamesAndTypes(
+    const CallExpr *MatchedCallExpr, std::size_t InitialArgIndex) {
+  // Reset vectors, and fill them with the currently checked function's
+  // arguments' data.
+  ArgNames.clear();
+  ArgTypes.clear();
+
+  for (std::size_t I = InitialArgIndex, J = MatchedCallExpr->getNumArgs();
+       I < J; ++I) {
+    if (const auto *ArgExpr = dyn_cast<DeclRefExpr>(
+            MatchedCallExpr->getArg(I)->IgnoreUnlessSpelledInSource())) {
+      if (const auto *Var = dyn_cast<VarDecl>(ArgExpr->getDecl())) {
+        ArgTypes.push_back(Var->getType());
+        ArgNames.push_back(Var->getName());
+      } else if (const auto *FCall =
+                     dyn_cast<FunctionDecl>(ArgExpr->getDecl())) {
+        ArgTypes.push_back(FCall->getType());
+        ArgNames.push_back(FCall->getName());
+      } else {
+        ArgTypes.push_back(QualType());
+        ArgNames.push_back(StringRef());
+      }
+    } else {
+      ArgTypes.push_back(QualType());
+      ArgNames.push_back(StringRef());
+    }
+  }
+}
+
+bool SuspiciousCallArgumentCheck::areParamAndArgComparable(
+    std::size_t Position1, std::size_t Position2, const ASTContext &Ctx) const {
+  if (Position1 >= ArgNames.size() || Position2 >= ArgNames.size())
+    return false;
+
+  // Do not report for too short strings.
+  if (ArgNames[Position1].size() < MinimumIdentifierNameLength ||
+      ArgNames[Position2].size() < MinimumIdentifierNameLength ||
+      ParamNames[Position1].size() < MinimumIdentifierNameLength ||
+      ParamNames[Position2].size() < MinimumIdentifierNameLength)
+    return false;
+
+  if (!areTypesCompatible(ArgTypes[Position1], ParamTypes[Position2], Ctx) ||
+      !areTypesCompatible(ArgTypes[Position2], ParamTypes[Position1], Ctx))
+    return false;
+
+  return true;
+}
+
+bool SuspiciousCallArgumentCheck::areArgsSwapped(std::size_t Position1,
+                                                 std::size_t Position2) const {
+  for (Heuristic H : AppliedHeuristics) {
+    bool A1ToP2Similar = areNamesSimilar(
+        ArgNames[Position2], ParamNames[Position1], H, BoundKind::SimilarAbove);
+    bool A2ToP1Similar = areNamesSimilar(
+        ArgNames[Position1], ParamNames[Position2], H, BoundKind::SimilarAbove);
+
+    bool A1ToP1Dissimilar =
+        !areNamesSimilar(ArgNames[Position1], ParamNames[Position1], H,
+                         BoundKind::DissimilarBelow);
+    bool A2ToP2Dissimilar =
+        !areNamesSimilar(ArgNames[Position2], ParamNames[Position2], H,
+                         BoundKind::DissimilarBelow);
+
+    if ((A1ToP2Similar || A2ToP1Similar) && A1ToP1Dissimilar &&
+        A2ToP2Dissimilar)
+      return true;
+  }
+  return false;
+}
+
+bool SuspiciousCallArgumentCheck::areNamesSimilar(StringRef Arg,
+                                                  StringRef Param, Heuristic H,
+                                                  BoundKind BK) const {
+  int8_t Threshold = -1;
+  if (Optional<int8_t> GotBound = getBound(H, BK))
+    Threshold = GotBound.getValue();
+
+  switch (H) {
+  case Heuristic::Equality:
+    return applyEqualityHeuristic(Arg, Param);
+  case Heuristic::Abbreviation:
+    return applyAbbreviationHeuristic(AbbreviationDictionary, Arg, Param);
+  case Heuristic::Prefix:
+    return applyPrefixHeuristic(Arg, Param, Threshold);
+  case Heuristic::Suffix:
+    return applySuffixHeuristic(Arg, Param, Threshold);
+  case Heuristic::Substring:
+    return applySubstringHeuristic(Arg, Param, Threshold);
+  case Heuristic::Levenshtein:
+    return applyLevenshteinHeuristic(Arg, Param, Threshold);
+  case Heuristic::JaroWinkler:
+    return applyJaroWinklerHeuristic(Arg, Param, Threshold);
+  case Heuristic::Dice:
+    return applyDiceHeuristic(Arg, Param, Threshold);
+  }
+  llvm_unreachable("Unhandled heuristic kind");
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`cppcoreguidelines-prefer-member-initializer
   Finds member initializations in the constructor body which can be placed into
   the initialization list instead.
 
 - New :doc:`bugprone-unhandled-exception-at-new
   <clang-tidy/checks/bugprone-unhandled-exception-at-new>` check.
 
   Finds calls to ``new`` with missing exception handler for ``std::bad_alloc``.
 
+- New `readability-suspicious-call-argument
+  <clang-tidy/checks/readability-suspicious-call-argument>`_ check
+
+  Finds function calls where the arguments passed are provided out of order,
+  based on the difference between the argument name and the parameter names
+  of the function.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-pos47-c
   <clang-tidy/checks/cert-pos47-c>` to
   :doc:`concurrency-thread-canceltype-asynchronous
   <clang-tidy/checks/concurrency-thread-canceltype-asynchronous>` was added.
 
+
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Improved :doc:`bugprone-signal-handler
   <clang-tidy/checks/bugprone-signal-handler>` check.
 
   Added an option to choose the set of allowed functions.
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `readability-redundant-smartptr-get <readability-redundant-smartptr-get.html>`_, "Yes"
    `readability-redundant-string-cstr <readability-redundant-string-cstr.html>`_, "Yes"
    `readability-redundant-string-init <readability-redundant-string-init.html>`_, "Yes"
    `readability-simplify-boolean-expr <readability-simplify-boolean-expr.html>`_, "Yes"
    `readability-simplify-subscript-expr <readability-simplify-subscript-expr.html>`_, "Yes"
    `readability-static-accessed-through-instance <readability-static-accessed-through-instance.html>`_, "Yes"
    `readability-static-definition-in-anonymous-namespace <readability-static-definition-in-anonymous-namespace.html>`_, "Yes"
    `readability-string-compare <readability-string-compare.html>`_, "Yes"
+   `readability-suspicious-call-argument <readability-suspicious-call-argument.html>`_,
    `readability-uniqueptr-delete-release <readability-uniqueptr-delete-release.html>`_, "Yes"
    `readability-uppercase-literal-suffix <readability-uppercase-literal-suffix.html>`_, "Yes"
    `readability-use-anyofallof <readability-use-anyofallof.html>`_,
    `zircon-temporary-objects <zircon-temporary-objects.html>`_,
 
 
 .. csv-table:: Aliases..
    :header: "Name", "Redirect", "Offers fixes"

clang-tools-extra/docs/clang-tidy/checks/readability-suspicious-call-argument.rst

+.. title:: clang-tidy - readability-suspicious-call-argument
+
+readability-suspicious-call-argument
+====================================
+
+Finds function calls where the arguments passed are provided out of order,
+based on the difference between the argument name and the parameter names
+of the function.
+
+Given a function call ``f(foo, bar);`` and a function signature
+``void f(T tvar, U uvar)``, the arguments ``foo`` and ``bar`` are swapped if
+``foo`` (the argument name) is more similar to ``uvar`` (the other parameter)
+than ``tvar`` (the parameter it is currently passed to) **and** ``bar`` is
+more similar to ``tvar`` than ``uvar``.
+
+Warnings might indicate either that the arguments are swapped, or that the
+names' cross-similarity might hinder code comprehension.
+
+.. _heuristics:
+
+Heuristics
+----------
+
+The following heuristics are implemented in the check.
+If **any** of the enabled heuristics deem the arguments to be provided out of
+order, a warning will be issued.
+
+The heuristics themselves are implemented by considering pairs of strings, and
+are symmetric, so in the following there is no distinction on which string is
+the argument name and which string is the parameter name.
+
+Equality
+^^^^^^^^
+
+The most trivial heuristic, which compares the two strings for case-insensitive
+equality.
+
+.. _abbreviation_heuristic:
+
+Abbreviation
+^^^^^^^^^^^^
+
+Common abbreviations can be specified which will deem the strings similar if
+the abbreviated and the abbreviation stand together.
+For example, if ``src`` is registered as an abbreviation for ``source``, then
+the following code example will be warned about.
+
+.. code-block:: c++
+
+    void foo(int source, int x);
+
+    foo(b, src);
+
+The abbreviations to recognise can be configured with the
+:ref:`Abbreviations<opt_Abbreviations>` check option.
+This heuristic is case-insensitive.
+
+Prefix
+^^^^^^
+
+The *prefix* heuristic reports if one of the strings is a sufficiently long
+prefix of the other string, e.g. ``target`` to ``targetPtr``.
+The similarity percentage is the length ratio of the prefix to the longer
+string, in the previous example, it would be `6 / 9 = 66.66...`\%.
+
+This heuristic can be configured with :ref:`bounds<opt_Bounds>`.
+The default bounds are: below `25`\% dissimilar and above `30`\% similar.
+This heuristic is case-insensitive.
+
+Suffix
+^^^^^^
+
+Analogous to the `Prefix` heuristic.
+In the case of ``oldValue`` and ``value`` compared, the similarity percentage
+is `8 / 5 = 62.5`\%.
+
+This heuristic can be configured with :ref:`bounds<opt_Bounds>`.
+The default bounds are: below `25`\% dissimilar and above `30`\% similar.
+This heuristic is case-insensitive.
+
+Substring
+^^^^^^^^^
+
+The substring heuristic combines the prefix and the suffix heuristic, and tries
+to find the *longest common substring* in the two strings provided.
+The similarity percentage is the ratio of the found longest common substring
+against the *longer* of the two input strings.
+For example, given ``val`` and ``rvalue``, the similarity is `3 / 6 = 50`\%.
+If no characters are common in the two string, `0`\%.
+
+This heuristic can be configured with :ref:`bounds<opt_Bounds>`.
+The default bounds are: below `40`\% dissimilar and above `50`\% similar.
+This heuristic is case-insensitive.
+
+Levenshtein distance (as `Levenshtein`)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The `Levenshtein distance <http://en.wikipedia.org/wiki/Levenshtein_distance>`_
+describes how many single-character changes (additions, changes, or removals)
+must be applied to transform one string into another.
+
+The Levenshtein distance is translated into a similarity percentage by dividing
+it with the length of the *longer* string, and taking its complement with
+regards to `100`\%.
+For example, given ``something`` and ``anything``, the distance is `4` edits,
+and the similarity percentage is `100`\% `- 4 / 9 = 55.55...`\%.
+
+This heuristic can be configured with :ref:`bounds<opt_Bounds>`.
+The default bounds are: below `50`\% dissimilar and above `66`\% similar.
+This heuristic is case-sensitive.
+
+Jaro–Winkler distance (as `JaroWinkler`)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The `Jaro–Winkler distance <http://en.wikipedia.org/wiki/Jaro–Winkler_distance>`_
+is an edit distance like the Levenshtein distance.
+It is calculated from the amount of common characters that are sufficiently
+close to each other in position, and to-be-changed characters.
+The original definition of Jaro has been extended by Winkler to weigh prefix
+similarities more.
+The similarity percentage is expressed as an average of the common and
+non-common characters against the length of both strings.
+
+This heuristic can be configured with :ref:`bounds<opt_Bounds>`.
+The default bounds are: below `75`\% dissimilar and above `85`\% similar.
+This heuristic is case-insensitive.
+
+Sørensen–Dice coefficient (as `Dice`)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The `Sørensen–Dice coefficient <http://en.wikipedia.org/wiki/Sørensen–Dice_coefficient>`_
+was originally defined to measure the similarity of two sets.
+Formally, the coefficient is calculated by dividing `2 * #(intersection)` with
+`#(set1) + #(set2)`, where `#()` is the cardinality function of sets.
+This metric is applied to strings by creating bigrams (substring sequences of
+length 2) of the two strings and using the set of bigrams for the two strings
+as the two sets.
+
+This heuristic can be configured with :ref:`bounds<opt_Bounds>`.
+The default bounds are: below `60`\% dissimilar and above `70`\% similar.
+This heuristic is case-insensitive.
+
+
+Options
+-------
+
+.. option:: MinimumIdentifierNameLength
+
+    Sets the minimum required length the argument and parameter names
+    need to have. Names shorter than this length will be ignored.
+    Defaults to `3`.
+
+.. _opt_Abbreviations:
+
+.. option:: Abbreviations
+
+    For the **Abbreviation** heuristic
+    (:ref:`see here<abbreviation_heuristic>`), this option configures the
+    abbreviations in the `"abbreviation=abbreviated_value"` format.
+    The option is a string, with each value joined by `";"`.
+
+    By default, the following abbreviations are set:
+
+       * `addr=address`
+       * `arr=array`
+       * `attr=attribute`
+       * `buf=buffer`
+       * `cl=client`
+       * `cnt=count`
+       * `col=column`
+       * `cpy=copy`
+       * `dest=destination`
+       * `dist=distance`
+       * `dst=distance`
+       * `elem=element`
+       * `hght=height`
+       * `i=index`
+       * `idx=index`
+       * `len=length`
+       * `ln=line`
+       * `lst=list`
+       * `nr=number`
+       * `num=number`
+       * `pos=position`
+       * `ptr=pointer`
+       * `ref=reference`
+       * `src=source`
+       * `srv=server`
+       * `stmt=statement`
+       * `str=string`
+       * `val=value`
+       * `var=variable`
+       * `vec=vector`
+       * `wdth=width`
+
+The configuration options for each implemented heuristic (see above) is
+constructed dynamically.
+In the following, `<HeuristicName>` refers to one of the keys from the
+heuristics implemented.
+
+.. option:: <HeuristicName>
+
+    `True` or `False`, whether a particular heuristic, such as `Equality` or
+    `Levenshtein` is enabled.
+
+    Defaults to `True` for every heuristic.
+
+.. _opt_Bounds:
+
+.. option:: <HeuristicName>DissimilarBelow, <HeuristicName>SimilarAbove
+
+    A value between `0` and `100`, expressing a percentage.
+    The bounds set what percentage of similarity the heuristic must deduce
+    for the two identifiers to be considered similar or dissimilar by the
+    check.
+
+    Given arguments ``arg1`` and ``arg2`` passed to ``param1`` and ``param2``,
+    respectively, the bounds check is performed in the following way:
+    If the similarity of the currently passed argument order
+    (``arg1`` to ``param1``) is **below** the `DissimilarBelow` threshold, and
+    the similarity of the suggested swapped order (``arg1`` to ``param2``) is
+    **above** the `SimilarAbove` threshold, the swap is reported.
+
+    For the defaults of each heuristic, :ref:`see above<heuristics>`.
+
+
+Name synthesis
+--------------
+
+When comparing the argument names and parameter names, the following logic is
+used to gather the names for comparison:
+
+Parameter names are the identifiers as written in the source code.
+
+Argument names are:
+
+  * If a variable is passed, the variable's name.
+  * If a subsequent function call's return value is used as argument, the called
+    function's name.
+  * Otherwise, empty string.
+
+Empty argument or parameter names are ignored by the heuristics.

clang-tools-extra/test/clang-tidy/checkers/readability-suspicious-call-argument.cpp

+// RUN: %check_clang_tidy %s readability-suspicious-call-argument %t -- -- -std=c++11
+
+void foo_1(int aaaaaa, int bbbbbb) {}
+
+void foo_2(int source, int aaaaaa) {}
+
+void foo_3(int valToRet, int aaaaaa) {}
+
+void foo_4(int pointer, int aaaaaa) {}
+
+void foo_5(int aaaaaa, int bbbbbb, int cccccc, ...) {}
+
+void foo_6(const int dddddd, bool &eeeeee) {}
+
+void foo_7(int aaaaaa, int bbbbbb, int cccccc, int ffffff = 7) {}
+
+void foo_8(int frobble1, int frobble2) {}
+
+// Test functions for convertible argument--parameter types.
+void fun(const int &m);
+void fun2() {
+  int m = 3;
+  fun(m);
+}
+
+// Test cases for parameters of const reference and value.
+void value_const_reference(int llllll, const int &kkkkkk);
+
+void const_ref_value_swapped() {
+  const int &kkkkkk = 42;
+  const int &llllll = 42;
+  value_const_reference(kkkkkk, llllll);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'kkkkkk' (passed to 'llllll') looks like it might be swapped with the 2nd, 'llllll' (passed to 'kkkkkk') [readability-suspicious-call-argument]
+  // CHECK-MESSAGES: :[[@LINE-7]]:6: note: in the call to 'value_const_reference', declared here
+}
+
+// Const, non const references.
+void const_nonconst_parameters(const int &mmmmmm, int &nnnnnn);
+
+void const_nonconst_swap1() {
+  const int &nnnnnn = 42;
+  int mmmmmm;
+  // Do not check, because non-const reference parameter cannot bind to const reference argument.
+  const_nonconst_parameters(nnnnnn, mmmmmm);
+}
+
+void const_nonconst_swap3() {
+  const int nnnnnn = 42;
+  int m = 42;
+  int &mmmmmm = m;
+  // Do not check, const int does not bind to non const reference.
+  const_nonconst_parameters(nnnnnn, mmmmmm);
+}
+
+void const_nonconst_swap2() {
+  int nnnnnn;
+  int mmmmmm;
+  // Check for swapped arguments. (Both arguments are non-const.)
+  const_nonconst_parameters(nnnnnn, mmmmmm);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'nnnnnn' (passed to 'mmmmmm') looks like it might be swapped with the 2nd, 'mmmmmm' (passed to 'nnnnnn')
+}
+
+void const_nonconst_pointers(const int *mmmmmm, int *nnnnnn);
+void const_nonconst_pointers2(const int *mmmmmm, const int *nnnnnn);
+
+void const_nonconst_pointers_swapped() {
+  int *mmmmmm;
+  const int *nnnnnn;
+  const_nonconst_pointers(nnnnnn, mmmmmm);
+}
+
+void const_nonconst_pointers_swapped2() {
+  const int *mmmmmm;
+  int *nnnnnn;
+  const_nonconst_pointers2(nnnnnn, mmmmmm);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'nnnnnn' (passed to 'mmmmmm') looks like it might be swapped with the 2nd, 'mmmmmm' (passed to 'nnnnnn')
+}
+
+// Test cases for pointers and arrays.
+void pointer_array_parameters(
+    int *pppppp, int qqqqqq[4]);
+
+void pointer_array_swap() {
+  int qqqqqq[5];
+  int *pppppp;
+  // Check for swapped arguments. An array implicitly converts to a pointer.
+  pointer_array_parameters(qqqqqq, pppppp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'qqqqqq' (passed to 'pppppp') looks like it might be swapped with the 2nd, 'pppppp' (passed to 'qqqqqq')
+}
+
+// Test cases for multilevel pointers.
+void multilevel_pointer_parameters(int *const **pppppp,
+                                   const int *const *volatile const *qqqqqq);
+void multilevel_pointer_parameters2(
+    char *****nnnnnn, char *volatile *const *const *const *const &mmmmmm);
+
+typedef float T;
+typedef T *S;
+typedef S *const volatile R;
+typedef R *Q;
+typedef Q *P;
+typedef P *O;
+void multilevel_pointer_parameters3(float **const volatile ***rrrrrr, O &ssssss);
+
+void multilevel_pointer_swap() {
+  int *const **qqqqqq;
+  int *const **pppppp;
+  multilevel_pointer_parameters(qqqqqq, pppppp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'qqqqqq' (passed to 'pppppp') looks like it might be swapped with the 2nd, 'pppppp' (passed to 'qqqqqq')
+
+  char *****mmmmmm;
+  char *****nnnnnn;
+  multilevel_pointer_parameters2(mmmmmm, nnnnnn);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'mmmmmm' (passed to 'nnnnnn') looks like it might be swapped with the 2nd, 'nnnnnn' (passed to 'mmmmmm')
+
+  float **const volatile ***rrrrrr;
+  float **const volatile ***ssssss;
+  multilevel_pointer_parameters3(ssssss, rrrrrr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'ssssss' (passed to 'rrrrrr') looks like it might be swapped with the 2nd, 'rrrrrr' (passed to 'ssssss')
+}
+
+void multilevel_pointer_parameters4(char ****pppppp,
+                                    char *const volatile **const *qqqqqq);
+void multilevel_pointer_parameters5(
+    bool *****nnnnnn, bool *volatile *const *const *const *&mmmmmm);
+void multilevel_pointer_parameters6(double **llllll, char **&kkkkkk);
+void multilevel_pointer_parameters7(const volatile int ***iiiiii,
+                                    const int *const *const *jjjjjj);
+
+void multilevel_pointer_swap3() {
+  char ****qqqqqq;
+  char *const volatile **const *pppppp;
+  // Do not check.
+  multilevel_pointer_parameters4(qqqqqq, pppppp);
+
+  bool *****mmmmmm;
+  bool *volatile *const *const *const *nnnnnn;
+  // Do not check.
+  multilevel_pointer_parameters5(mmmmmm, nnnnnn);
+
+  double **kkkkkk;
+  char **llllll;
+  multilevel_pointer_parameters6(kkkkkk, llllll);
+
+  const volatile int ***jjjjjj;
+  const int *const *const *iiiiii;
+  multilevel_pointer_parameters7(jjjjjj, iiiiii);
+}
+
+// Test cases for multidimesional arrays.
+void multilevel_array_parameters(int pppppp[2][2][2], const int qqqqqq[][2][2]);
+
+void multilevel_array_parameters2(int (*mmmmmm)[2][2], int nnnnnn[9][2][23]);
+
+void multilevel_array_parameters3(int (*eeeeee)[2][2], int (&ffffff)[1][2][2]);
+
+void multilevel_array_parameters4(int (*llllll)[2][2], int kkkkkk[2][2]);
+
+void multilevel_array_parameters5(int iiiiii[2][2], char jjjjjj[2][2]);
+
+void multilevel_array_parameters6(int (*bbbbbb)[2][2], int cccccc[1][2][2]);
+
+void multilevel_array_swap() {
+  int qqqqqq[1][2][2];
+  int pppppp[][2][2] = {{{1, 2}, {1, 2}}, {{1, 2}, {1, 2}}}; // int [2][2][2]
+  multilevel_array_parameters(qqqqqq, pppppp);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'qqqqqq' (passed to 'pppppp') looks like it might be swapped with the 2nd, 'pppppp' (passed to 'qqqqqq')
+
+  int(*nnnnnn)[2][2];
+  int mmmmmm[9][2][23];
+  // Do not check, array sizes has to match in every dimension, except the first.
+  multilevel_array_parameters2(nnnnnn, mmmmmm);
+
+  int ffffff[][2][2] = {{{1, 2}, {1, 2}}, {{1, 2}, {1, 2}}}; // int [2][2][2]
+  int eeeeee[1][2][2] = {{{1, 2}, {1, 2}}};                  // int [1][2][2]
+  // Do not check, for array references, size has to match in every dimension.
+  multilevel_array_parameters3(ffffff, eeeeee);
+
+  int kkkkkk[2][2][2];
+  int(*llllll)[2];
+  // Do not check, argument dimensions differ.
+  multilevel_array_parameters4(kkkkkk, llllll);
+
+  int jjjjjj[2][2];
+  char iiiiii[2][2];
+  // Do not check, array element types differ.
+  multilevel_array_parameters5(jjjjjj, iiiiii);
+
+  int t[][2][2] = {{{1, 2}, {1, 2}}, {{1, 2}, {1, 2}}}; // int [2][2][2]
+  int(*cccccc)[2][2] = t;                               // int (*)[2][2]
+  int bbbbbb[][2][2] = {{{1, 2}, {1, 2}}};              // int [1][2][2]
+  multilevel_array_parameters6(cccccc, bbbbbb);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'cccccc' (passed to 'bbbbbb') looks like it might be swapped with the 2nd, 'bbbbbb' (passed to 'cccccc')
+}
+
+void multilevel_array_swap2() {
+  int qqqqqq[2][2][2];
+  const int pppppp[][2][2] = {{{1, 2}, {1, 2}}, {{1, 2}, {1, 2}}};
+  // Do not check, pppppp is const and cannot bind to an array with nonconst elements.
+  multilevel_array_parameters(qqqqqq, pppppp);
+}
+
+// Complex test case.
+void multilevel_pointer_array_parameters(const int(*const (*volatile const (*const (*const (*const &aaaaaa)[1])[32])[4])[3][2][2]), const int(*const (*volatile const (*const (*const (*&bbbbbb)[1])[32])[4])[3][2][2]));
+
+void multilevel_pointer_array_swap() {
+  const int(
+          *const(*volatile const(*const(*const(*aaaaaa)[1])[32])[4])[3][2][2]);
+  const int(
+          *const(*volatile const(*const(*const(*bbbbbb)[1])[32])[4])[3][2][2]);
+  multilevel_pointer_array_parameters(bbbbbb, aaaaaa);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'bbbbbb' (passed to 'aaaaaa') looks like it might be swapped with the 2nd, 'aaaaaa' (passed to 'bbbbbb')
+}
+
+enum class numbers_scoped { one,
+                            two };
+
+// Test cases for arithmetic types.
+void arithmetic_type_parameters(float vvvvvv, int wwwwww);
+void arithmetic_type_parameters2(numbers_scoped vvvvvv, int wwwwww);
+
+void arithmetic_types_swap1() {
+  bool wwwwww;
+  float vvvvvv;
+  arithmetic_type_parameters(wwwwww, vvvvvv);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'wwwwww' (passed to 'vvvvvv') looks like it might be swapped with the 2nd, 'vvvvvv' (passed to 'wwwwww')
+}
+
+void arithmetic_types_swap3() {
+  char wwwwww;
+  unsigned long long int vvvvvv;
+  arithmetic_type_parameters(wwwwww, vvvvvv);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'wwwwww' (passed to 'vvvvvv') looks like it might be swapped with the 2nd, 'vvvvvv' (passed to 'wwwwww')
+}
+
+void arithmetic_types_swap4() {
+  enum numbers { one,
+                 two };
+  numbers wwwwww = numbers::one;
+  int vvvvvv;
+  arithmetic_type_parameters(wwwwww, vvvvvv);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'wwwwww' (passed to 'vvvvvv') looks like it might be swapped with the 2nd, 'vvvvvv' (passed to 'wwwwww')
+}
+
+void arithmetic_types_swap5() {
+  wchar_t vvvvvv;
+  float wwwwww;
+  arithmetic_type_parameters(wwwwww, vvvvvv);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'wwwwww' (passed to 'vvvvvv') looks like it might be swapped with the 2nd, 'vvvvvv' (passed to 'wwwwww')
+}
+
+void arithmetic_types_swap6() {
+  wchar_t vvvvvv;
+  numbers_scoped wwwwww = numbers_scoped::one;
+  // Do not check, numers is a scoped enum type.
+  arithmetic_type_parameters2(wwwwww, vvvvvv);
+}
+
+// Base, derived
+class TestClass {
+public:
+  void thisFunction(int integerParam, int thisIsPARAM) {}
+};
+
+class DerivedTestClass : public TestClass {};
+
+void base_derived_pointer_parameters(TestClass *aaaaaa,
+                                     DerivedTestClass *bbbbbb);
+
+void base_derived_swap1() {
+  TestClass *bbbbbb;
+  DerivedTestClass *aaaaaa;
+  // Do not check, because TestClass does not convert implicitly to DerivedTestClass.
+  base_derived_pointer_parameters(bbbbbb, aaaaaa);
+}
+
+void base_derived_swap2() {
+  DerivedTestClass *bbbbbb, *aaaaaa;
+  // Check for swapped arguments, DerivedTestClass converts to TestClass implicitly.
+  base_derived_pointer_parameters(bbbbbb, aaaaaa);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'bbbbbb' (passed to 'aaaaaa') looks like it might be swapped with the 2nd, 'aaaaaa' (passed to 'bbbbbb')
+}
+
+class PrivateDerivedClass : private TestClass {};
+
+void private_derived_pointer_parameters(TestClass *aaaaaa, PrivateDerivedClass *bbbbbb);
+
+void private_base_swap1() {
+  TestClass *bbbbbb;
+  PrivateDerivedClass *aaaaaa;
+  private_derived_pointer_parameters(bbbbbb, aaaaaa);
+}
+
+// Multilevel inheritance
+class DerivedOfDerivedTestClass : public DerivedTestClass {};
+
+void multi_level_inheritance_swap() {
+  DerivedOfDerivedTestClass *aaaaaa, *bbbbbb;
+  // Check for swapped arguments. Derived classes implicitly convert to their base.
+  base_derived_pointer_parameters(
+      bbbbbb, aaaaaa);
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 1st argument 'bbbbbb' (passed to 'aaaaaa') looks like it might be swapped with the 2nd, 'aaaaaa' (passed to 'bbbbbb')
+}
+
+// Tests for function pointer swaps
+void funct_ptr_params(double (*ffffff)(int, int), double (*gggggg)(int, int));
+void funct_ptr_params(double (*ffffff)(int, int), int (*gggggg)(int, int));
+
+double ffffff(int a, int b) { return 0; }
+double gggggg(int a, int b) { return 0; }
+
+void funtionc_ptr_params_swap() {
+  funct_ptr_params(gggggg, ffffff);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'gggggg' (passed to 'ffffff') looks like it might be swapped with the 2nd, 'ffffff' (passed to 'gggggg')
+}
+
+int fffff(int a, int b) { return 0; }
+
+void function_ptr_swap2() {
+  // Do not check, because the function `ffffff` cannot convert to a function
+  // with prototype: double(int,int).
+  funct_ptr_params(gggggg, fffff);
+}
+
+// Paraphrased example from Z3 (src/qe/qe_arrays.cpp) which originally produced
+// a false positive. Operator() calls should ignore the called object
+// "argument".
+struct type1;
+struct type2;
+struct type3;
+
+struct callable1 {
+  void operator()(type1 &mdl, type2 &arr_vars, type3 &fml, type2 &aux_vars) const {}
+};
+
+struct callable2 {
+  void operator()(type1 &mdl, type2 &arr_vars, type3 &fml, type2 &aux_vars,
+                  bool reduce_all_selects) const {
+    (void)reduce_all_selects;
+    callable1 pe;
+    pe(mdl, arr_vars, fml, aux_vars);
+    // NO-WARN: Argument and parameter names match perfectly, "pe" should be
+    // ignored!
+  }
+};
+
+struct binop_t {};
+
+binop_t operator+(const binop_t &lhs, const binop_t &rhs) { return lhs; }
+bool operator<(const binop_t &lhs, const binop_t &rhs) { return true; }
+bool operator>(const binop_t &aaaaaa, const binop_t &bbbbbb) { return false; }
+
+void binop_test() {
+  // NO-WARN: Binary operators are ignored.
+  binop_t lhs, rhs;
+  if (lhs + rhs < rhs)
+    return;
+
+  if (operator<(rhs, lhs))
+    return;
+
+  binop_t aaaaaa, cccccc;
+  if (operator>(cccccc, aaaaaa))
+    return;
+}
+
+int recursion(int aaaa, int bbbb) {
+  if (aaaa)
+    return 0;
+
+  int cccc = 0;
+  return recursion(bbbb, cccc);
+  // NO-WARN: Recursive calls usually shuffle with arguments and we ignore those.
+}
+
+void pass_by_copy(binop_t xxxx, binop_t yyyy) {}
+
+// Paraphrased example from LLVM's code (lib/Analysis/InstructionSimplify.cpp)
+// that generated a false positive.
+struct value;
+enum opcode { Foo,
+              Bar };
+static value *SimplifyRightShift(
+    opcode Opcode, value *Op0, value *Op1, bool isExact,
+    const type1 &Q, unsigned MaxRecurse) {}
+static value *SimplifyLShrInst(value *Op0, value *Op1, bool isExact,
+                               const type1 &Q, unsigned MaxRecurse) {
+  if (value *V = SimplifyRightShift(Foo, Op0, Op1, isExact, Q, MaxRecurse))
+    return V;
+  // NO-WARN: Argument names perfectly match parameter names, sans the enum.
+
+  return nullptr;
+}
+
+void has_unnamed(int aaaaaa, int) {}
+
+int main() {
+  // Equality test.
+  int aaaaaa, cccccc = 0;
+  foo_1(cccccc, aaaaaa);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'cccccc' (passed to 'aaaaaa') looks like it might be swapped with the 2nd, 'aaaaaa' (passed to 'bbbbbb')
+
+  binop_t xxxx, yyyy;
+  pass_by_copy(yyyy, xxxx);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'yyyy' (passed to 'xxxx') looks like it might be swapped with the 2nd, 'xxxx' (passed to 'yyyy')
+
+  // Abbreviation test.
+  int src = 0;
+  foo_2(aaaaaa, src);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'aaaaaa' (passed to 'source') looks like it might be swapped with the 2nd, 'src' (passed to 'aaaaaa')
+
+  // Levenshtein test.
+  int aaaabb = 0;
+  foo_1(cccccc, aaaabb);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'cccccc' (passed to 'aaaaaa') looks like it might be swapped with the 2nd, 'aaaabb' (passed to 'bbbbbb')
+
+  // Prefix test.
+  int aaaa = 0;
+  foo_1(cccccc, aaaa);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'cccccc' (passed to 'aaaaaa') looks like it might be swapped with the 2nd, 'aaaa' (passed to 'bbbbbb')
+
+  // Suffix test.
+  int urce = 0;
+  foo_2(cccccc, urce);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'cccccc' (passed to 'source') looks like it might be swapped with the 2nd, 'urce' (passed to 'aaaaaa')
+
+  // Substring test.
+  int ourc = 0;
+  foo_2(cccccc, ourc);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'cccccc' (passed to 'source') looks like it might be swapped with the 2nd, 'ourc' (passed to 'aaaaaa')
+
+  // Jaro-Winkler test.
+  int iPonter = 0;
+  foo_4(cccccc, iPonter);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'cccccc' (passed to 'pointer') looks like it might be swapped with the 2nd, 'iPonter' (passed to 'aaaaaa')
+
+  // Dice test.
+  int aaabaa = 0;
+  foo_1(cccccc, aaabaa);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'cccccc' (passed to 'aaaaaa') looks like it might be swapped with the 2nd, 'aaabaa' (passed to 'bbbbbb')
+
+  // Variadic function test.
+  int bbbbbb = 0;
+  foo_5(src, bbbbbb, cccccc, aaaaaa); // Should pass.
+  foo_5(cccccc, bbbbbb, aaaaaa, src);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'cccccc' (passed to 'aaaaaa') looks like it might be swapped with the 3rd, 'aaaaaa' (passed to 'cccccc')
+
+  // Test function with default argument.
+  foo_7(src, bbbbbb, cccccc, aaaaaa);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'src' (passed to 'aaaaaa') looks like it might be swapped with the 4th, 'aaaaaa' (passed to 'ffffff')
+
+  foo_7(cccccc, bbbbbb, aaaaaa, src);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'cccccc' (passed to 'aaaaaa') looks like it might be swapped with the 3rd, 'aaaaaa' (passed to 'cccccc')
+
+  int ffffff = 0;
+  foo_7(ffffff, bbbbbb, cccccc); // NO-WARN: Even though 'ffffff' is passed to 'aaaaaa' and there is a 4th parameter 'ffffff', there isn't a **swap** here.
+
+  int frobble1 = 1, frobble2 = 2;
+  foo_8(frobble2, frobble1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'frobble2' (passed to 'frobble1') looks like it might be swapped with the 2nd, 'frobble1' (passed to 'frobble2')
+
+  int bar1 = 1, bar2 = 2;
+  foo_8(bar2, bar1); // NO-WARN.
+
+  // Type match
+  bool dddddd = false;
+  int eeeeee = 0;
+  auto szam = 0;
+  foo_6(eeeeee, dddddd);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'eeeeee' (passed to 'dddddd') looks like it might be swapped with the 2nd, 'dddddd' (passed to 'eeeeee')
+  foo_1(szam, aaaaaa);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 1st argument 'szam' (passed to 'aaaaaa') looks like it might be swapped with the 2nd, 'aaaaaa' (passed to 'bbbbbb')
+
+  // Test lambda.
+  auto testMethod = [&](int method, int randomParam) { return 0; };
+  int method = 0;
+  testMethod(method, 0); // Should pass.
+
+  // Member function test.
+  TestClass test;
+  int integ, thisIsAnArg = 0;
+  test.thisFunction(integ, thisIsAnArg); // Should pass.
+
+  has_unnamed(1, bbbbbb);
+
+  return 0;
+}

llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/readability/BUILD.gn

sources = [
     "RedundantSmartptrGetCheck.cpp",
     "RedundantStringCStrCheck.cpp",
     "RedundantStringInitCheck.cpp",
     "SimplifyBooleanExprCheck.cpp",
     "SimplifySubscriptExprCheck.cpp",
     "StaticAccessedThroughInstanceCheck.cpp",
     "StaticDefinitionInAnonymousNamespaceCheck.cpp",
     "StringCompareCheck.cpp",
+    "SuspiciousCallArgumentCheck.cpp",
     "UniqueptrDeleteReleaseCheck.cpp",
     "UppercaseLiteralSuffixCheck.cpp",
     "UseAnyOfAllOfCheck.cpp",
   ]
 }