[clang-tidy] Support replacements in macro arguments in misc-inefficient-algorithm

[alexfh]

Summary

Support replacements in macro arguments in the
misc-inefficient-algorithm check.

[klimek]

clang-tidy/misc/InefficientAlgorithmCheck.cpp

114–118

Any reason we don't want to use Lexer::makeFileCharRange here (which should handle all the cases correctly).

[alexfh]

clang-tidy/misc/InefficientAlgorithmCheck.cpp

114–118

It was the first thing I tried, but it gets the test case wrong, i.e. it replaces the whole macro invocation instead of just the argument:

#define SECOND(x, y, z) y
  SECOND(q,std::count(s.begin(), s.end(), 22),w);

becomes

#define SECOND(x, y, z) y
  s.count(22);

[klimek]

LG

clang-tidy/misc/InefficientAlgorithmCheck.cpp

114–118

You're right. Add a FIXME to figure something out regarding a common function to get this use case covered.

clang-tidy/misc/InefficientAlgorithmCheck.cpp

void InefficientAlgorithmCheck::check(const MatchFinder::MatchResult &Result) {
 
   if (Unordered && AlgDecl->getName().find("bound") != llvm::StringRef::npos)
     return;
 
   const auto *AlgParam = Result.Nodes.getNodeAs<Expr>("AlgParam");
   const auto *IneffContExpr = Result.Nodes.getNodeAs<Expr>("IneffContExpr");
   FixItHint Hint;
 
-  if (!AlgCall->getLocStart().isMacroID() && !Maplike && CompatibleTypes) {
+  SourceManager &SM = *Result.SourceManager;
+  LangOptions LangOpts = Result.Context->getLangOpts();
+
+  CharSourceRange CallRange =
+      CharSourceRange::getTokenRange(AlgCall->getSourceRange());
+
+  if (SM.isMacroArgExpansion(CallRange.getBegin()) &&
+      SM.isMacroArgExpansion(CallRange.getEnd())) {
+    CallRange.setBegin(SM.getSpellingLoc(CallRange.getBegin()));
+    CallRange.setEnd(SM.getSpellingLoc(CallRange.getEnd()));
+  }
+
+  if (!CallRange.getBegin().isMacroID() && !Maplike && CompatibleTypes) {
+    StringRef ContainerText = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(IneffContExpr->getSourceRange()), SM,
+        LangOpts);
+    StringRef ParamText = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(AlgParam->getSourceRange()), SM,
+        LangOpts);
     std::string ReplacementText =
-        (llvm::Twine(Lexer::getSourceText(
-             CharSourceRange::getTokenRange(IneffContExpr->getSourceRange()),
-             *Result.SourceManager, Result.Context->getLangOpts())) +
-         (PtrToContainer ? "->" : ".") + AlgDecl->getName() + "(" +
-         Lexer::getSourceText(
-             CharSourceRange::getTokenRange(AlgParam->getSourceRange()),
-             *Result.SourceManager, Result.Context->getLangOpts()) +
-         ")").str();
-    Hint = FixItHint::CreateReplacement(AlgCall->getSourceRange(),
-                                        ReplacementText);
+        (llvm::Twine(ContainerText) + (PtrToContainer ? "->" : ".") +
+         AlgDecl->getName() + "(" + ParamText + ")")
+            .str();
+    Hint = FixItHint::CreateReplacement(CallRange, ReplacementText);
   }
 
   diag(AlgCall->getLocStart(),
        "this STL algorithm call should be replaced with a container method")
       << Hint;
 }
 
 } // namespace misc
 } // namespace tidy
 } // namespace clang

test/clang-tidy/misc-inefficient-algorithm.cpp

 int main() {
   std::set<int> s;
   auto it = std::find(s.begin(), s.end(), 43);
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: this STL algorithm call should be replaced with a container method [misc-inefficient-algorithm]
   // CHECK-FIXES: {{^  }}auto it = s.find(43);{{$}}
   auto c = count(s.begin(), s.end(), 43);
   // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: this STL algorithm call should be
   // CHECK-FIXES: {{^  }}auto c = s.count(43);{{$}}
+
+#define SECOND(x, y, z) y
+  SECOND(q,std::count(s.begin(), s.end(), 22),w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: this STL algorithm call should be
+  // CHECK-FIXES: {{^  }}SECOND(q,s.count(22),w);{{$}}
+
   it = find_if(s.begin(), s.end(), [](int) { return false; });
 
   std::multiset<int> ms;
   find(ms.begin(), ms.end(), 46);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be
   // CHECK-FIXES: {{^  }}ms.find(46);{{$}}
 
   const std::multiset<int> &msref = ms;