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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


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


================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:207
+  double Dist = Arg.edit_distance(Param);
+  Dist = (1 - Dist / LongerLength) * 100;
+  return Dist > Threshold;
----------------



================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:268
+  // Jaro-Winkler distance.
+  Dist = (Dist + (L * 0.1 * (1 - Dist))) * 100;
+  return Dist > Threshold;
----------------



================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:299
+
+/// Checks if ArgType binds to ParamType ragerding reference-ness and
+/// cv-qualifiers.
----------------



================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:371
+
+    // Check whether cv-qualifiers premit compatibility on
+    // current level.
----------------



================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:403
+
+  // Reference-ness has already been checked ad should be removed
+  // before further checking.
----------------



================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:408-409
+
+  bool IsParamContinuouslyConst =
+      !IsParamReference || ParamType.getNonReferenceType().isConstQualified();
+
----------------
Should this move down closer to where it's used?


================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:425-427
+  // Check if the argument and the param are both function types (the parameter
+  // decayed to
+  // a function pointer).
----------------



================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:437
+
+  // When ParamType is an array reference, ArgType has to be of the same sized,
+  // array type with cv-compatible elements.
----------------



================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:453
+
+  // At this point, all possible C language implicit conversion were checked
+  if (!Ctx.getLangOpts().CPlusPlus)
----------------



================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:467
+
+    return ArgDecl->isDerivedFrom(ParamDecl);
+  }
----------------
It would be good to have a test case involving private inheritance.


================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:579
+bool SuspiciousCallArgumentCheck::isHeuristicEnabled(Heuristic H) const {
+  return llvm::find(AppliedHeuristics, H) != AppliedHeuristics.end();
+}
----------------



================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:588
+  if (!Defaults[Idx].hasBounds())
+    return {};
+
----------------



================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:694-695
+
+  for (std::size_t I = 0, E = CalleeFuncDecl->getNumParams(); I != E; ++I) {
+    if (const auto *Param = CalleeFuncDecl->getParamDecl(I)) {
+      ParamTypes.push_back(Param->getType());
----------------
Range-based for loop over `CalledFuncDecl->params()`?


================
Comment at: clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:801
+  }
+  llvm_unreachable("Unhandled heuristic kind");
+}
----------------
This looks pretty reachable to me in the case where there's no bound.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D20689