================
@@ -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;
+ collectOperands(RhsExpr, Operands);
+
+ int32_t LhsPosition = -1;
+ for (int32_t i = 0; i < Operands.size(); ++i)
+ if (isSameLhs(Operands[i], LhsStr))
+ {
+ LhsPosition = i;
----------------
zeyi2 wrote:
String concatenation is not commutative. If I understand to code correctly, it
may transform `a = b + a` to `a += b`, the order get reversed.
e.g.
```cpp
#include <iostream>
#include <string>
int main() {
std::string lhs = "World";
std::string other = "Hello";
std::string s1 = lhs;
s1 = s1 + other;
std::cout << "Scenario 1 (s1 = s1 + other): " << s1 << std::endl;
std::string s2 = lhs;
s2 = other + s2;
std::cout << "Scenario 2 (s2 = other + s2): " << s2 << std::endl;
return 0;
}
```
```
repro.cpp:12:6: warning: string concatenation results in allocation of
unnecessary temporary strings; consider using 'operator+=' or
'string::append()' instead [performance-inefficient-string-concatenation]
12 | s1 = s1 + other;
| ~~~^~~~~~~~~~~~
| s1 += other
repro.cpp:20:6: warning: string concatenation results in allocation of
unnecessary temporary strings; consider using 'operator+=' or
'string::append()' instead [performance-inefficient-string-concatenation]
20 | s2 = other + s2;
| ~~~^~~~~~~~~~~~
| s2 += other
```
https://github.com/llvm/llvm-project/pull/188430
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
