diff options
Diffstat (limited to 'compilerplugins')
| -rw-r--r-- | compilerplugins/clang/stringadd.cxx | 89 | ||||
| -rw-r--r-- | compilerplugins/clang/test/stringadd.cxx | 44 |
2 files changed, 122 insertions, 11 deletions
diff --git a/compilerplugins/clang/stringadd.cxx b/compilerplugins/clang/stringadd.cxx index 641000e28ecf..a3435bab7cb5 100644 --- a/compilerplugins/clang/stringadd.cxx +++ b/compilerplugins/clang/stringadd.cxx @@ -68,11 +68,13 @@ public: } bool VisitCompoundStmt(CompoundStmt const*); + bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr const*); private: const VarDecl* findAssignOrAdd(Stmt const*); - Expr const* ignore(Expr const*); void checkForCompoundAssign(Stmt const* stmt1, Stmt const* stmt2, VarDecl const* varDecl); + + Expr const* ignore(Expr const*); std::string getSourceAsString(SourceLocation startLoc, SourceLocation endLoc); bool isSideEffectFree(Expr const*); }; @@ -175,6 +177,65 @@ void StringAdd::checkForCompoundAssign(Stmt const* stmt1, Stmt const* stmt2, Var << stmt2->getSourceRange(); } +// Check for generating temporaries when adding strings +// +bool StringAdd::VisitCXXOperatorCallExpr(CXXOperatorCallExpr const* operatorCall) +{ + if (ignoreLocation(operatorCall)) + return true; + // TODO PlusEqual seems to generate temporaries, does not do the StringConcat optimisation + 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() + && !tc.Class("OUString").Namespace("rtl").GlobalNamespace() + && !tc.Class("OString").Namespace("rtl").GlobalNamespace()) + return true; + + auto check = [operatorCall, this, &tc](const MaterializeTemporaryExpr* matTempExpr) { + auto tc3 = loplugin::TypeCheck(matTempExpr->getType()); + if (!tc3.Class("OUString").Namespace("rtl").GlobalNamespace() + && !tc3.Class("OString").Namespace("rtl").GlobalNamespace()) + return; + if (auto bindTemp + = dyn_cast<CXXBindTemporaryExpr>(matTempExpr->GetTemporaryExpr()->IgnoreCasts())) + { + // ignore temporaries returned from function calls + if (isa<CallExpr>(bindTemp->getSubExpr())) + return; + // we don't have OStringLiteral1, so char needs to generate a temporary + if (tc.Class("OString").Namespace("rtl").GlobalNamespace() + || tc.Struct("OStringConcat").Namespace("rtl").GlobalNamespace()) + if (auto cxxConstruct = dyn_cast<CXXConstructExpr>(bindTemp->getSubExpr())) + if (loplugin::TypeCheck( + cxxConstruct->getConstructor()->getParamDecl(0)->getType()) + .Char()) + return; + // calls where we pass in an explicit character encoding + if (auto cxxTemp = dyn_cast<CXXTemporaryObjectExpr>(bindTemp->getSubExpr())) + if (cxxTemp->getNumArgs() > 1) + return; + } + // conditional operators ( a ? b : c ) will result in temporaries + if (isa<ConditionalOperator>( + matTempExpr->GetTemporaryExpr()->IgnoreCasts()->IgnoreParens())) + return; + report(DiagnosticsEngine::Warning, "avoid constructing temporary copies during +", + compat::getBeginLoc(matTempExpr)) + << matTempExpr->getSourceRange(); + // operatorCall->dump(); + // matTempExpr->getType()->dump(); + // operatorCall->getType()->getUnqualifiedDesugaredType()->dump(); + }; + + if (auto matTempExpr = dyn_cast<MaterializeTemporaryExpr>(operatorCall->getArg(0))) + check(matTempExpr); + if (auto matTempExpr = dyn_cast<MaterializeTemporaryExpr>(operatorCall->getArg(1))) + check(matTempExpr); + return true; +} + Expr const* StringAdd::ignore(Expr const* expr) { return compat::IgnoreImplicit(compat::IgnoreImplicit(expr)->IgnoreParens()); @@ -183,6 +244,9 @@ Expr const* StringAdd::ignore(Expr const* expr) bool StringAdd::isSideEffectFree(Expr const* expr) { expr = ignore(expr); + // I don't think the OUStringAppend functionality can handle this efficiently + if (isa<ConditionalOperator>(expr)) + return false; // Multiple statements have a well defined evaluation order (sequence points between them) // but a single expression may be evaluated in arbitrary order; // if there are side effects in one of the sub-expressions that have an effect on another subexpression, @@ -204,15 +268,20 @@ bool StringAdd::isSideEffectFree(Expr const* expr) if (auto callExpr = dyn_cast<CallExpr>(expr)) { - // check for calls through OUString::number + // check for calls through OUString::number/OUString::unacquired if (auto calleeMethodDecl = dyn_cast_or_null<CXXMethodDecl>(callExpr->getCalleeDecl())) - if (calleeMethodDecl && calleeMethodDecl->getIdentifier() - && calleeMethodDecl->getName() == "number") + if (calleeMethodDecl && calleeMethodDecl->getIdentifier()) { - auto tc = loplugin::TypeCheck(calleeMethodDecl->getParent()); - if (tc.Class("OUString") || tc.Class("OString")) - if (isSideEffectFree(callExpr->getArg(0))) - return true; + auto name = calleeMethodDecl->getName(); + if (name == "number" || name == "unacquired") + { + auto tc = loplugin::TypeCheck(calleeMethodDecl->getParent()); + if (tc.Class("OUString") || tc.Class("OString")) + { + if (isSideEffectFree(callExpr->getArg(0))) + return true; + } + } } if (auto calleeFunctionDecl = dyn_cast_or_null<FunctionDecl>(callExpr->getCalleeDecl())) if (calleeFunctionDecl && calleeFunctionDecl->getIdentifier()) @@ -222,8 +291,8 @@ bool StringAdd::isSideEffectFree(Expr const* expr) if (name == "OUStringToOString" || name == "OStringToOUString") if (isSideEffectFree(callExpr->getArg(0))) return true; - // check for calls through *ResId - if (name.endswith("ResId")) + // whitelist some known-safe methods + if (name.endswith("ResId") || name == "GetXMLToken") if (isSideEffectFree(callExpr->getArg(0))) return true; } diff --git a/compilerplugins/clang/test/stringadd.cxx b/compilerplugins/clang/test/stringadd.cxx index c9ef37f09bfd..0e0986258254 100644 --- a/compilerplugins/clang/test/stringadd.cxx +++ b/compilerplugins/clang/test/stringadd.cxx @@ -7,8 +7,13 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#include <rtl/ustring.hxx> +#include <rtl/strbuf.hxx> #include <rtl/string.hxx> +#include <rtl/ustrbuf.hxx> +#include <rtl/ustring.hxx> + +// --------------------------------------------------------------- +// += tests namespace test1 { @@ -147,4 +152,41 @@ void g(int x, const Foo& aValidation) } } +// --------------------------------------------------------------- +// detecting OUString temporary construction in + + +namespace test9 +{ +OUString getByValue(); +const OUString& getByRef(); +void f1(OUString s, OUString t, int i, const char* pChar) +{ + // no warning expected + t = t + "xxx"; + // expected-error@+1 {{avoid constructing temporary copies during + [loplugin:stringadd]}} + s = s + OUString("xxx"); + // expected-error@+1 {{avoid constructing temporary copies during + [loplugin:stringadd]}} + s = s + OUString(getByRef()); + + // no warning expected + OUString a; + a = a + getByValue(); + + // no warning expected + OUString b; + b = b + (i == 1 ? "aaa" : "bbb"); + + // no warning expected + OUString c; + c = c + OUString(pChar, strlen(pChar), RTL_TEXTENCODING_UTF8); +} +void f2(char ch) +{ + OString s; + // expected-error@+1 {{avoid constructing temporary copies during + [loplugin:stringadd]}} + s = s + OString("xxx"); + // no warning expected, no OStringLiteral1 + s = s + OString(ch); +} +} /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ |