summaryrefslogtreecommitdiffstats
path: root/compilerplugins/clang/stringadd.cxx
diff options
context:
space:
mode:
Diffstat (limited to 'compilerplugins/clang/stringadd.cxx')
-rw-r--r--compilerplugins/clang/stringadd.cxx124
1 files changed, 94 insertions, 30 deletions
diff --git a/compilerplugins/clang/stringadd.cxx b/compilerplugins/clang/stringadd.cxx
index f10072497767..0ac4ee6d3c65 100644
--- a/compilerplugins/clang/stringadd.cxx
+++ b/compilerplugins/clang/stringadd.cxx
@@ -16,11 +16,13 @@
#include "plugin.hxx"
#include "check.hxx"
+#include "compat.hxx"
+#include "config_clang.h"
#include "clang/AST/CXXInheritance.h"
#include "clang/AST/StmtVisitor.h"
/**
- Look for repeated addition to OUString/OString.
+ Look for repeated addition to OUString/OString/OUStringBuffer/OStringBuffer.
Eg.
OUString x = "xxx";
@@ -60,6 +62,9 @@ public:
// TODO the += depends on the result of the preceding assign, so can't merge
if (fn == SRCDIR "/editeng/source/misc/svxacorr.cxx")
return false;
+ // TODO this file has a boatload of buffer appends' and I don't feel like fixing them all now
+ if (fn == SRCDIR "/vcl/source/gdi/pdfwriter_impl.cxx")
+ return false;
return true;
}
@@ -142,12 +147,32 @@ StringAdd::VarDeclAndSummands StringAdd::findAssignOrAdd(Stmt const* stmt)
{
auto tc = loplugin::TypeCheck(varDeclLHS->getType());
if (!tc.Class("OUString").Namespace("rtl").GlobalNamespace()
- && !tc.Class("OString").Namespace("rtl").GlobalNamespace())
+ && !tc.Class("OString").Namespace("rtl").GlobalNamespace()
+ && !tc.Class("OUStringBuffer").Namespace("rtl").GlobalNamespace()
+ && !tc.Class("OStringBuffer").Namespace("rtl").GlobalNamespace())
return {};
if (varDeclLHS->getStorageDuration() == SD_Static)
return {};
if (!varDeclLHS->hasInit())
return {};
+ if (tc.Class("OUStringBuffer").Namespace("rtl").GlobalNamespace()
+ || tc.Class("OStringBuffer").Namespace("rtl").GlobalNamespace())
+ {
+ // ignore the constructor that gives the buffer a default size
+ if (auto cxxConstructor = dyn_cast<CXXConstructExpr>(varDeclLHS->getInit()))
+ if (auto constructorDecl = cxxConstructor->getConstructor())
+ if ((constructorDecl->getNumParams() == 1
+ && loplugin::TypeCheck(constructorDecl->getParamDecl(0)->getType())
+ .Typedef("sal_Int32")
+ .GlobalNamespace())
+ || (constructorDecl->getNumParams() == 2
+ && constructorDecl->getParamDecl(0)->getType()->isIntegralType(
+ compiler.getASTContext())
+ && constructorDecl->getParamDecl(1)
+ ->getType()
+ ->isSpecificBuiltinType(BuiltinType::Int)))
+ return {};
+ }
return { varDeclLHS, (isCompileTimeConstant(varDeclLHS->getInit())
? Summands::OnlyCompileTimeConstants
: (isSideEffectFree(varDeclLHS->getInit())
@@ -170,6 +195,24 @@ StringAdd::VarDeclAndSummands StringAdd::findAssignOrAdd(Stmt const* stmt)
: (isSideEffectFree(rhs) ? Summands::OnlySideEffectFree
: Summands::SideEffect)) };
}
+ if (auto memberCall = dyn_cast<CXXMemberCallExpr>(stmt))
+ if (auto cxxMethodDecl = dyn_cast_or_null<CXXMethodDecl>(memberCall->getDirectCallee()))
+ if (cxxMethodDecl->getIdentifier() && cxxMethodDecl->getName() == "append")
+ if (auto declRefExprLHS
+ = dyn_cast<DeclRefExpr>(ignore(memberCall->getImplicitObjectArgument())))
+ if (auto varDeclLHS = dyn_cast<VarDecl>(declRefExprLHS->getDecl()))
+ {
+ auto tc = loplugin::TypeCheck(varDeclLHS->getType());
+ if (!tc.Class("OUStringBuffer").Namespace("rtl").GlobalNamespace()
+ && !tc.Class("OStringBuffer").Namespace("rtl").GlobalNamespace())
+ return {};
+ auto rhs = memberCall->getArg(0);
+ return { varDeclLHS,
+ (isCompileTimeConstant(rhs)
+ ? Summands::OnlyCompileTimeConstants
+ : (isSideEffectFree(rhs) ? Summands::OnlySideEffectFree
+ : Summands::SideEffect)) };
+ }
return {};
}
@@ -181,20 +224,41 @@ bool StringAdd::checkForCompoundAssign(Stmt const* stmt1, Stmt const* stmt2,
stmt2 = exprCleanup->getSubExpr();
if (auto switchCase = dyn_cast<SwitchCase>(stmt2))
stmt2 = switchCase->getSubStmt();
- auto operatorCall = dyn_cast<CXXOperatorCallExpr>(stmt2);
- if (!operatorCall)
- return false;
- if (operatorCall->getOperator() != OO_PlusEqual)
- return false;
- auto declRefExprLHS = dyn_cast<DeclRefExpr>(ignore(operatorCall->getArg(0)));
+
+ const DeclRefExpr* declRefExprLHS;
+ const Expr* rhs;
+ auto tc = loplugin::TypeCheck(varDecl.varDecl->getType());
+ if (tc.Class("OString") || tc.Class("OUString"))
+ {
+ auto operatorCall = dyn_cast<CXXOperatorCallExpr>(stmt2);
+ if (!operatorCall)
+ return false;
+ if (operatorCall->getOperator() != OO_PlusEqual)
+ return false;
+ declRefExprLHS = dyn_cast<DeclRefExpr>(ignore(operatorCall->getArg(0)));
+ rhs = operatorCall->getArg(1);
+ }
+ else
+ {
+ // OUStringBuffer, OStringBuffer
+ auto memberCall = dyn_cast<CXXMemberCallExpr>(stmt2);
+ if (!memberCall)
+ return false;
+ auto cxxMethodDecl = dyn_cast_or_null<CXXMethodDecl>(memberCall->getDirectCallee());
+ if (!cxxMethodDecl)
+ return false;
+ if (!cxxMethodDecl->getIdentifier() || cxxMethodDecl->getName() != "append")
+ return false;
+ declRefExprLHS = dyn_cast<DeclRefExpr>(ignore(memberCall->getImplicitObjectArgument()));
+ rhs = memberCall->getArg(0);
+ }
if (!declRefExprLHS)
return false;
if (declRefExprLHS->getDecl() != varDecl.varDecl)
return false;
// if either side is a compile-time-constant, then we don't care about
// side-effects
- auto rhs = operatorCall->getArg(1);
- auto const ctcRhs = isCompileTimeConstant(rhs);
+ bool const ctcRhs = isCompileTimeConstant(rhs);
if (!ctcRhs)
{
auto const sefRhs = isSideEffectFree(rhs);
@@ -206,17 +270,24 @@ bool StringAdd::checkForCompoundAssign(Stmt const* stmt1, Stmt const* stmt2,
return true;
}
}
+ SourceRange mergeRange(stmt1->getSourceRange().getBegin(), stmt2->getSourceRange().getEnd());
// if we cross a #ifdef boundary
- if (containsPreprocessingConditionalInclusion(
- SourceRange(stmt1->getSourceRange().getBegin(), stmt2->getSourceRange().getEnd())))
+ if (containsPreprocessingConditionalInclusion(mergeRange))
{
varDecl.summands
= ctcRhs ? Summands::OnlyCompileTimeConstants
: isSideEffectFree(rhs) ? Summands::OnlySideEffectFree : Summands::SideEffect;
return true;
}
- report(DiagnosticsEngine::Warning, "simplify by merging with the preceding assignment",
- compat::getBeginLoc(stmt2))
+ // If there is a comment between two calls, rather don't suggest merge
+ // IMO, code clarity trumps efficiency (as far as plugin warnings go, anyway).
+ if (containsComment(mergeRange))
+ return true;
+ // I don't think the OUStringAppend functionality can handle this efficiently
+ if (isa<ConditionalOperator>(ignore(rhs)))
+ return false;
+ report(DiagnosticsEngine::Warning, "simplify by merging with the preceding assign/append",
+ stmt2->getBeginLoc())
<< stmt2->getSourceRange();
return true;
}
@@ -230,8 +301,7 @@ bool StringAdd::VisitCXXOperatorCallExpr(CXXOperatorCallExpr const* operatorCall
if (operatorCall->getOperator() != OO_Plus)
return true;
auto tc = loplugin::TypeCheck(operatorCall->getType()->getUnqualifiedDesugaredType());
- if (!tc.Struct("OUStringConcat").Namespace("rtl").GlobalNamespace()
- && !tc.Struct("OStringConcat").Namespace("rtl").GlobalNamespace()
+ if (!tc.Struct("StringConcat").Namespace("rtl").GlobalNamespace()
&& !tc.Class("OUString").Namespace("rtl").GlobalNamespace()
&& !tc.Class("OString").Namespace("rtl").GlobalNamespace())
return true;
@@ -253,7 +323,7 @@ bool StringAdd::VisitCXXOperatorCallExpr(CXXOperatorCallExpr const* operatorCall
("rather use O[U]String::Concat than constructing %0 from %1 on %select{L|R}2HS of "
"+ (where %select{R|L}2HS is of"
" type %3)"),
- compat::getBeginLoc(e))
+ e->getBeginLoc())
<< e->getType().getLocalUnqualifiedType() << e->getSubExprAsWritten()->getType() << arg
<< operatorCall->getArg(1 - arg)->IgnoreImpCasts()->getType() << e->getSourceRange();
};
@@ -276,10 +346,6 @@ bool StringAdd::VisitCXXMemberCallExpr(CXXMemberCallExpr const* methodCall)
if (!tc1.Class("OUStringBuffer").Namespace("rtl").GlobalNamespace()
&& !tc1.Class("OStringBuffer").Namespace("rtl").GlobalNamespace())
return true;
- auto paramType = methodDecl->getParamDecl(0)->getType();
- // char is still a pain to work with, when constructing a chained +
- if (paramType->isCharType() || loplugin::TypeCheck(paramType).Typedef("sal_Unicode"))
- return true;
auto arg = methodCall->getArg(0);
// I don't think the OUStringAppend functionality can handle this efficiently
if (isa<ConditionalOperator>(ignore(arg)))
@@ -296,17 +362,13 @@ bool StringAdd::VisitCXXMemberCallExpr(CXXMemberCallExpr const* methodCall)
if (!methodDecl2->getIdentifier() || methodDecl2->getName() != "append"
|| methodCall2->getNumArgs() == 0)
return true;
- auto paramType2 = methodDecl2->getParamDecl(0)->getType();
- // char is still a pain to work with, when constructing a chained +
- if (paramType2->isCharType() || loplugin::TypeCheck(paramType2).Typedef("sal_Unicode"))
- return true;
arg = methodCall2->getArg(0);
// I don't think the OUStringAppend functionality can handle this efficiently
if (isa<ConditionalOperator>(ignore(arg)))
return true;
report(DiagnosticsEngine::Warning,
"chained append, rather use single append call and + operator",
- compat::getBeginLoc(methodCall2))
+ methodCall2->getBeginLoc())
<< methodCall2->getSourceRange();
return true;
@@ -314,7 +376,7 @@ bool StringAdd::VisitCXXMemberCallExpr(CXXMemberCallExpr const* methodCall)
Expr const* StringAdd::ignore(Expr const* expr)
{
- return compat::IgnoreImplicit(compat::IgnoreImplicit(expr)->IgnoreParens());
+ return expr->IgnoreImplicit()->IgnoreParens()->IgnoreImplicit();
}
bool StringAdd::isSideEffectFree(Expr const* expr)
@@ -417,7 +479,7 @@ bool StringAdd::isSideEffectFree(Expr const* expr)
if (isSideEffectFree(callExpr->getArg(0)))
return true;
// allowlist some known-safe methods
- if (name.endswith("ResId") || name == "GetXMLToken")
+ if (compat::ends_with(name, "ResId") || name == "GetXMLToken")
if (isSideEffectFree(callExpr->getArg(0)))
return true;
}
@@ -427,7 +489,9 @@ bool StringAdd::isSideEffectFree(Expr const* expr)
if (auto constructExpr = dyn_cast<CXXConstructExpr>(expr))
{
auto dc = loplugin::DeclCheck(constructExpr->getConstructor());
- if (dc.MemberFunction().Class("OUString") || dc.MemberFunction().Class("OString"))
+ if (dc.MemberFunction().Class("OUString") || dc.MemberFunction().Class("OString")
+ || dc.MemberFunction().Class("OUStringBuffer")
+ || dc.MemberFunction().Class("OStringBuffer"))
if (constructExpr->getNumArgs() == 0 || isSideEffectFree(constructExpr->getArg(0)))
return true;
// Expr::HasSideEffects does not like stuff that passes through OUStringLiteral
@@ -451,7 +515,7 @@ bool StringAdd::isSideEffectFree(Expr const* expr)
bool StringAdd::isCompileTimeConstant(Expr const* expr)
{
- expr = compat::IgnoreImplicit(expr);
+ expr = expr->IgnoreImplicit();
if (auto cxxConstructExpr = dyn_cast<CXXConstructExpr>(expr))
if (cxxConstructExpr->getNumArgs() > 0)
expr = cxxConstructExpr->getArg(0);