https://github.com/mirimmad updated https://github.com/llvm/llvm-project/pull/188430
>From 346c20f3ed58659e4c8db9de451c7fd570e94524 Mon Sep 17 00:00:00 2001 From: Immad Mir <[email protected]> Date: Wed, 25 Mar 2026 13:57:57 +0530 Subject: [PATCH 1/4] suggest fix it hints for string concat --- .../InefficientStringConcatenationCheck.cpp | 169 +++++++++++++++++- .../inefficient-string-concatenation.cpp | 27 ++- 2 files changed, 179 insertions(+), 17 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp b/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp index 1067fca289a2c..ac0cdaa091f85 100644 --- a/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp @@ -8,6 +8,7 @@ #include "InefficientStringConcatenationCheck.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" using namespace clang::ast_matchers; @@ -40,25 +41,101 @@ 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( @@ -66,14 +143,88 @@ void InefficientStringConcatenationCheck::check( 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; + collectOperands(RhsExpr, Operands); + + int32_t LhsPosition = -1; + for (int32_t i = 0; i < Operands.size(); ++i) + if (isSameLhs(Operands[i], LhsStr)) + { + LhsPosition = i; + break; + } + // skip this pattern: a => b + c + d + if (LhsPosition == -1) { + return; + } + + auto ReplacementText = Lexer::getSourceText( + CharSourceRange::getTokenRange(LhsStr->getSourceRange()), SM, LO).str(); + + + if (Operands.size() > 2) { + for (int32_t i = 0; i < Operands.size(); ++i) { + if (i == LhsPosition) + continue; + auto OpText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Operands[i]->getSourceRange()), SM, LO); + ReplacementText += ".append(" + OpText.str() + ")"; + } + } else { + auto RhsText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Operands[LhsPosition == 0 ? 1 : 0]->getSourceRange()), SM, LO); + ReplacementText += " += " + RhsText.str() ; + } + + + diag(Assign->getExprLoc(), DiagMsg) + << FixItHint::CreateReplacement(Assign->getSourceRange(), + ReplacementText); + + + } else { + if (LhsStr) diag(LhsStr->getExprLoc(), DiagMsg); else if (PlusOperator) diag(PlusOperator->getExprLoc(), DiagMsg); + + } } } // namespace clang::tidy::performance diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-string-concatenation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-string-concatenation.cpp index adc37e4c4bedf..15542052a5a8c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-string-concatenation.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-string-concatenation.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s performance-inefficient-string-concatenation %t +// RUN: %check_clang_tidy %s performance-inefficient-string-concatenation %t #include <string> void f(std::string) {} @@ -14,18 +14,28 @@ int main() { f(mystr1 + mystr2 + mystr1); // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: string concatenation results in allocation of unnecessary temporary strings; consider using 'operator+=' or 'string::append()' instead mystr1 = mystr1 + mystr2; - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: string concatenation + // CHECK-FIXES: mystr1 += mystr2; mystr1 = mystr2 + mystr2 + mystr2; - // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: string concatenation + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: string concatenation mystr1 = mystr2 + mystr1; - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: string concatenation + // CHECK-FIXES: mystr1 += mystr2; mywstr1 = mywstr2 + mywstr1; - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: string concatenation + // CHECK-FIXES: mywstr1 += mywstr2; mywstr1 = mywstr2 + mywstr2 + mywstr2; // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: string concatenation myautostr1 = myautostr1 + myautostr2; - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation - + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: string concatenation + // CHECK-FIXES: myautostr1 += myautostr2; + + // Match against multple lines + /* mystr1 = mystr1 + mystr2 + mystr2; + // CHECK-DAG: :33:12: warning: string concatenation + // CHECK-DAG: :33:30: warning: string concatenation + // CHECK-FIXES: mystr1.append(mystr2).append(mystr2) */ + mywstr1 = mywstr2 + mywstr2; mystr1 = mystr2 + mystr2; mystr1 += mystr2; @@ -35,7 +45,8 @@ int main() { do { mystr1 = mystr1 + mystr2; - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation results in allocation of unnecessary temporary strings; consider using 'operator+=' or 'string::append()' instead + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: string concatenation results in allocation of unnecessary temporary strings; consider using 'operator+=' or 'string::append()' instead + // CHECK-FIXES: mystr1 += mystr2; } while (0); return 0; >From b563637429cc75564814f03b72934ed62c3d68bb Mon Sep 17 00:00:00 2001 From: Immad Mir <[email protected]> Date: Wed, 25 Mar 2026 15:21:07 +0530 Subject: [PATCH 2/4] format --- .../InefficientStringConcatenationCheck.cpp | 194 +++++++++--------- 1 file changed, 93 insertions(+), 101 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp b/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp index ac0cdaa091f85..3087ddc27cb53 100644 --- a/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp @@ -41,67 +41,60 @@ 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, ignoringParenImpCasts( - declRefExpr(BasicStringType, - hasDeclaration(decl().bind("lhsStrT"))) - .bind("lhsStr"))), - hasArgument(1, expr(hasDescendant(BasicStringPlusOperator)).bind("rhsExpr")) - ).bind("assign"); - + /* 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, 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)) - { + 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 { + } else { break; } } @@ -120,7 +113,6 @@ static void collectOperands(const Expr *E, std::vector<const Expr *> &Ops) { } } - if (const auto *OpCall = dyn_cast<CXXOperatorCallExpr>(E)) { if (OpCall->getOperator() == OO_Plus) { collectOperands(OpCall->getArg(0), Ops); @@ -146,85 +138,85 @@ void InefficientStringConcatenationCheck::check( 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"; - } */ + /* 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 (Assign && LhsStr && RhsExpr) { - const auto &SM = *Result.SourceManager; - const auto &LO = Result.Context->getLangOpts(); + if (Assign && LhsStr && RhsExpr) { + const auto &SM = *Result.SourceManager; + const auto &LO = Result.Context->getLangOpts(); std::vector<const Expr *> Operands; collectOperands(RhsExpr, Operands); int32_t LhsPosition = -1; for (int32_t i = 0; i < Operands.size(); ++i) - if (isSameLhs(Operands[i], LhsStr)) - { - LhsPosition = i; - break; - } + if (isSameLhs(Operands[i], LhsStr)) { + LhsPosition = i; + break; + } // skip this pattern: a => b + c + d - if (LhsPosition == -1) { + if (LhsPosition == -1) return; - } - - auto ReplacementText = Lexer::getSourceText( - CharSourceRange::getTokenRange(LhsStr->getSourceRange()), SM, LO).str(); + auto ReplacementText = + Lexer::getSourceText( + CharSourceRange::getTokenRange(LhsStr->getSourceRange()), SM, LO) + .str(); if (Operands.size() > 2) { - for (int32_t i = 0; i < Operands.size(); ++i) { - if (i == LhsPosition) - continue; - auto OpText = Lexer::getSourceText( - CharSourceRange::getTokenRange(Operands[i]->getSourceRange()), SM, LO); + for (int32_t i = 0; i < Operands.size(); ++i) { + if (i == LhsPosition) + continue; + auto OpText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Operands[i]->getSourceRange()), SM, + LO); ReplacementText += ".append(" + OpText.str() + ")"; + } + } else { + auto RhsText = Lexer::getSourceText( + CharSourceRange::getTokenRange( + Operands[LhsPosition == 0 ? 1 : 0]->getSourceRange()), + SM, LO); + ReplacementText += " += " + RhsText.str(); } + + diag(Assign->getExprLoc(), DiagMsg) << FixItHint::CreateReplacement( + Assign->getSourceRange(), ReplacementText); + } else { - auto RhsText = Lexer::getSourceText( - CharSourceRange::getTokenRange(Operands[LhsPosition == 0 ? 1 : 0]->getSourceRange()), SM, LO); - ReplacementText += " += " + RhsText.str() ; + if (LhsStr) + diag(LhsStr->getExprLoc(), DiagMsg); + else if (PlusOperator) + diag(PlusOperator->getExprLoc(), DiagMsg); } - - - diag(Assign->getExprLoc(), DiagMsg) - << FixItHint::CreateReplacement(Assign->getSourceRange(), - ReplacementText); - - - } else { - if (LhsStr) - diag(LhsStr->getExprLoc(), DiagMsg); - else if (PlusOperator) - diag(PlusOperator->getExprLoc(), DiagMsg); - - } } } // namespace clang::tidy::performance >From ba715001f0315519351caaa5088880d7704eb827 Mon Sep 17 00:00:00 2001 From: Immad Mir <[email protected]> Date: Thu, 2 Apr 2026 15:55:00 +0530 Subject: [PATCH 3/4] only allow stmt of the form a = a + b +c +... --- .../InefficientStringConcatenationCheck.cpp | 60 +++---------------- .../inefficient-string-concatenation.cpp | 2 - 2 files changed, 7 insertions(+), 55 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp b/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp index 3087ddc27cb53..2f212cd018bac 100644 --- a/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp @@ -41,25 +41,6 @@ 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("="), @@ -101,7 +82,7 @@ static const Expr *strip(const Expr *E) { return E; } -static void collectOperands(const Expr *E, std::vector<const Expr *> &Ops) { +static void collectOperands(const Expr *E, SmallVector<const Expr *> &Ops) { E = strip(E); E = E->IgnoreParenImpCasts(); @@ -138,33 +119,6 @@ void InefficientStringConcatenationCheck::check( 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"; @@ -173,17 +127,17 @@ void InefficientStringConcatenationCheck::check( const auto &SM = *Result.SourceManager; const auto &LO = Result.Context->getLangOpts(); - std::vector<const Expr *> Operands; + SmallVector<const Expr *> Operands; collectOperands(RhsExpr, Operands); - int32_t LhsPosition = -1; - for (int32_t i = 0; i < Operands.size(); ++i) + size_t LhsPosition = -1; + for (size_t i = 0; i < Operands.size(); ++i) if (isSameLhs(Operands[i], LhsStr)) { LhsPosition = i; break; } - // skip this pattern: a => b + c + d - if (LhsPosition == -1) + // skip if the LHS string is not the leftmost operand. + if (LhsPosition != 0) return; auto ReplacementText = @@ -192,7 +146,7 @@ void InefficientStringConcatenationCheck::check( .str(); if (Operands.size() > 2) { - for (int32_t i = 0; i < Operands.size(); ++i) { + for (size_t i = 0; i < Operands.size(); ++i) { if (i == LhsPosition) continue; auto OpText = Lexer::getSourceText( diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-string-concatenation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-string-concatenation.cpp index 15542052a5a8c..3f6463f4324f6 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-string-concatenation.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-string-concatenation.cpp @@ -20,10 +20,8 @@ int main() { // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: string concatenation mystr1 = mystr2 + mystr1; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: string concatenation - // CHECK-FIXES: mystr1 += mystr2; mywstr1 = mywstr2 + mywstr1; // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: string concatenation - // CHECK-FIXES: mywstr1 += mywstr2; mywstr1 = mywstr2 + mywstr2 + mywstr2; // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: string concatenation myautostr1 = myautostr1 + myautostr2; >From 54fe1dff878c66bb76c4150e6704289661c0f80a Mon Sep 17 00:00:00 2001 From: Immad Mir <[email protected]> Date: Mon, 6 Apr 2026 16:46:47 +0530 Subject: [PATCH 4/4] make changes to the test --- .../inefficient-string-concatenation.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-string-concatenation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-string-concatenation.cpp index 3f6463f4324f6..0d7ada4f29748 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-string-concatenation.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-string-concatenation.cpp @@ -17,23 +17,21 @@ int main() { // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: string concatenation // CHECK-FIXES: mystr1 += mystr2; mystr1 = mystr2 + mystr2 + mystr2; - // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: string concatenation + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: string concatenation mystr1 = mystr2 + mystr1; - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: string concatenation mywstr1 = mywstr2 + mywstr1; - // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: string concatenation mywstr1 = mywstr2 + mywstr2 + mywstr2; // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: string concatenation myautostr1 = myautostr1 + myautostr2; // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: string concatenation // CHECK-FIXES: myautostr1 += myautostr2; - // Match against multple lines - /* mystr1 = mystr1 + mystr2 + mystr2; - // CHECK-DAG: :33:12: warning: string concatenation - // CHECK-DAG: :33:30: warning: string concatenation - // CHECK-FIXES: mystr1.append(mystr2).append(mystr2) */ - + // Match against multple lines + /* + mystr1 = mystr1 + mystr2 + mystr2; + // check for fix and multiples warnings here + // fix: mystr1.append(mystr2).append(mystr2); + */ mywstr1 = mywstr2 + mywstr2; mystr1 = mystr2 + mystr2; mystr1 += mystr2; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
