[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/abseil/RedundantStrcatCallsCheck.cpp:28
+
+void RedundantStrcatCallsCheck::registerMatchers(MatchFinder* Finder) {
+  if (!getLangOpts().CPlusPlus) 
----------------
The formatting here looks off -- you should run the patch through clang-format, if you haven't already.


================
Comment at: clang-tidy/abseil/RedundantStrcatCallsCheck.cpp:37-38
+  // Those are handled on the ancestor call.
+  const auto CallToEither = callExpr(
+      callee(functionDecl(hasAnyName("::absl::StrCat", "::absl::StrAppend"))));
+  Finder->addMatcher(
----------------
Can this be replaced by `anyOf(CallToStrcat, CallToStrappend)`?


================
Comment at: clang-tidy/abseil/RedundantStrcatCallsCheck.cpp:50
+  int NumCalls = 0;
+  std::vector<FixItHint> hints;
+};
----------------
s/hints/Hints


================
Comment at: clang-tidy/abseil/RedundantStrcatCallsCheck.cpp:96
+    int StartArg = CallExpr == RootCall && IsAppend;
+    for (const auto Arg : CallExpr->arguments()) {
+      if (StartArg-- > 0) 
----------------
`const auto *` please (so it's obvious the type is a pointer).


================
Comment at: clang-tidy/abseil/RedundantStrcatCallsCheck.cpp:99
+      	continue;
+      if (const clang::CallExpr* sub =
+              ProcessArgument(Arg, Result, &CheckResult)) {
----------------
s/sub/Sub


================
Comment at: clang-tidy/abseil/RedundantStrcatCallsCheck.cpp:113-119
+  if ((RootCall = Result.Nodes.getNodeAs<CallExpr>("StrCat"))) {
+    IsAppend = false;
+  } else if ((RootCall = Result.Nodes.getNodeAs<CallExpr>("StrAppend"))) {
+    IsAppend = true;
+  } else {
+    return;
+  }
----------------
Elide braces


================
Comment at: clang-tidy/abseil/RedundantStrcatCallsCheck.cpp:136
+
+  diag(RootCall->getBeginLoc(), "redundant StrCat calls")
+      << CheckResult.hints;
----------------
This text doesn't really help the user to understand what they've done wrong with their code. Perhaps `multiple calls to 'StrCat' can be flattened into a single call` or something along those lines?


https://reviews.llvm.org/D51132