[clang-tools-extra] [clang-tidy] suggest fix it hints for string concat (PR #188430)

Zeyi Xu via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 25 06:00:10 PDT 2026


================
@@ -40,40 +41,190 @@ void InefficientStringConcatenationCheck::registerMatchers(
           hasDescendant(BasicStringPlusOperator))
           .bind("plusOperator");
 
+ 
+
+   /*  const auto AssignOperator =
+    cxxOperatorCallExpr(
+        hasOverloadedOperatorName("="),
+
+        hasArgument(0, ignoringParenImpCasts(
+            declRefExpr(BasicStringType,
+                        hasDeclaration(decl().bind("lhsStrT")))
+                .bind("lhsStr"))),
+
+        hasArgument(1, ignoringParenImpCasts(
+            expr(
+                hasDescendant(declRefExpr(
+                    hasDeclaration(decl(equalsBoundNode("lhsStrT")))
+                )),
+                hasDescendant(BasicStringPlusOperator)
+            ).bind("rhsExpr")
+        ))
+    ).bind("assign"); */
+
+    
   const auto AssignOperator = cxxOperatorCallExpr(
-      hasOverloadedOperatorName("="),
-      hasArgument(0, declRefExpr(BasicStringType,
-                                 hasDeclaration(decl().bind("lhsStrT")))
-                         .bind("lhsStr")),
-      hasArgument(1, stmt(hasDescendant(declRefExpr(
-                         hasDeclaration(decl(equalsBoundNode("lhsStrT"))))))),
-      hasDescendant(BasicStringPlusOperator));
+        hasOverloadedOperatorName("="),
+        hasArgument(0, ignoringParenImpCasts(
+            declRefExpr(BasicStringType,
+                        hasDeclaration(decl().bind("lhsStrT")))
+            .bind("lhsStr"))),
+        hasArgument(1, expr(hasDescendant(BasicStringPlusOperator)).bind("rhsExpr"))
+    ).bind("assign");
+
 
   if (StrictMode) {
     Finder->addMatcher(cxxOperatorCallExpr(anyOf(AssignOperator, PlusOperator)),
-                       this);
+                       this);  
   } else {
     Finder->addMatcher(cxxOperatorCallExpr(anyOf(AssignOperator, PlusOperator),
                                            hasAncestor(stmt(anyOf(
                                                cxxForRangeStmt(), whileStmt(),
                                                forStmt(), doStmt())))),
                        this);
+    
+   
+  }
+}
+
+static const Expr *strip(const Expr *E) {
+  while (true) {
+    E = E->IgnoreParenImpCasts();
+    if (const auto *ICE  = dyn_cast<ImplicitCastExpr>(E)) {
+       E = ICE->getSubExpr();
+      }
+    else if (const auto *M = dyn_cast<MaterializeTemporaryExpr>(E))
+    {
+      E = M->getSubExpr();
+    }
+
+    else if (const auto *B = dyn_cast<CXXBindTemporaryExpr>(E)) {
+      E = B->getSubExpr();
+    }
+    else {
+      break;
+    }
   }
+  return E;
+}
+
+static void collectOperands(const Expr *E, std::vector<const Expr *> &Ops) {
+  E = strip(E);
+  E = E->IgnoreParenImpCasts();
+
+  if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {
+    if (BinOp->getOpcode() == BO_Add) {
+      collectOperands(BinOp->getLHS(), Ops);
+      collectOperands(BinOp->getRHS(), Ops);
+      return;
+    }
+  }
+
+   
+  if (const auto *OpCall = dyn_cast<CXXOperatorCallExpr>(E)) {
+    if (OpCall->getOperator() == OO_Plus) {
+      collectOperands(OpCall->getArg(0), Ops);
+      collectOperands(OpCall->getArg(1), Ops);
+      return;
+    }
+  }
+
+  Ops.push_back(E);
+}
+
+static bool isSameLhs(const Expr *E, const DeclRefExpr *Lhs) {
+  E = strip(E)->IgnoreParenImpCasts();
+  auto *DR = dyn_cast<DeclRefExpr>(E);
+  return DR && DR->getDecl() == Lhs->getDecl();
 }
 
 void InefficientStringConcatenationCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto *LhsStr = Result.Nodes.getNodeAs<DeclRefExpr>("lhsStr");
   const auto *PlusOperator =
       Result.Nodes.getNodeAs<CXXOperatorCallExpr>("plusOperator");
+  const auto *Assign = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("assign");
+  const auto *RhsExpr = Result.Nodes.getNodeAs<Expr>("rhsExpr");
+
+    /* if (Assign) {
+      llvm::errs() << "Assign: " << Assign->getSourceRange().printToString(*Result.SourceManager) << "\n";
+       Assign->printPretty(llvm::outs(), nullptr, PrintingPolicy(Result.Context->getLangOpts()));
+       llvm::outs() << "\n";
+    } 
+    if (RhsExpr) {
+      llvm::errs() << "RhsExpr: " << RhsExpr->getSourceRange().printToString(*Result.SourceManager) << "\n";
+      RhsExpr->printPretty(llvm::outs(), nullptr, PrintingPolicy(Result.Context->getLangOpts()));
+       llvm::outs() << "\n";
+       //RhsExpr->dump();
+    }
+    if (LhsStr) {
+      llvm::errs() << "LhsStr: " << LhsStr->getSourceRange().printToString(*Result.SourceManager) << "\n";
+    } */
+
+    /* std::vector<const Expr *> Operands;
+    collectOperands(RhsExpr, Operands); */
+   /*  llvm::errs() << "Operands in RhsExpr:\n";
+    for (const auto *Op : Operands) {
+      
+      Op->printPretty(llvm::outs(), nullptr, PrintingPolicy(Result.Context->getLangOpts()));
+      llvm::outs() << "\n";
+    } */
+
   const char *DiagMsg =
       "string concatenation results in allocation of unnecessary temporary "
       "strings; consider using 'operator+=' or 'string::append()' instead";
 
-  if (LhsStr)
+
+   if (Assign && LhsStr && RhsExpr) {
+     const auto &SM = *Result.SourceManager;
+     const auto &LO = Result.Context->getLangOpts();
+
+    std::vector<const Expr *> Operands;
----------------
zeyi2 wrote:

Could we use `SmallVector` instead of `std::vector`?

Please see: https://llvm.org/docs/CodingStandards.html#c-standard-library

https://github.com/llvm/llvm-project/pull/188430


More information about the cfe-commits mailing list