diff options
Diffstat (limited to '')
| -rw-r--r-- | compilerplugins/clang/stringadd.cxx | 467 |
1 files changed, 467 insertions, 0 deletions
diff --git a/compilerplugins/clang/stringadd.cxx b/compilerplugins/clang/stringadd.cxx new file mode 100644 index 000000000..339192a78 --- /dev/null +++ b/compilerplugins/clang/stringadd.cxx @@ -0,0 +1,467 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +#ifndef LO_CLANG_SHARED_PLUGINS + +#include <cassert> +#include <string> +#include <iostream> +#include <unordered_map> +#include <unordered_set> + +#include "plugin.hxx" +#include "check.hxx" +#include "config_clang.h" +#include "clang/AST/CXXInheritance.h" +#include "clang/AST/StmtVisitor.h" + +/** + Look for repeated addition to OUString/OString. + + Eg. + OUString x = "xxx"; + x += b; + + which can be simplified to + x = "xxx" + b + + which is more efficient, because of the OUStringConcat magic. +*/ + +namespace +{ +class StringAdd : public loplugin::FilteringPlugin<StringAdd> +{ +public: + explicit StringAdd(loplugin::InstantiationData const& data) + : FilteringPlugin(data) + { + } + + bool preRun() override + { + std::string fn(handler.getMainFileName()); + loplugin::normalizeDotDotInFilePath(fn); + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sal/qa/rtl/oustring/")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sal/qa/rtl/oustringbuffer/")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sal/qa/rtl/strings/")) + return false; + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sal/qa/OStringBuffer/")) + return false; + // there is an ifdef here, but my check is not working, not sure why + if (fn == SRCDIR "/pyuno/source/module/pyuno_runtime.cxx") + return false; + // TODO the += depends on the result of the preceding assign, so can't merge + if (fn == SRCDIR "/editeng/source/misc/svxacorr.cxx") + return false; + return true; + } + + virtual void run() override + { + if (!preRun()) + return; + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } + + bool VisitCompoundStmt(CompoundStmt const*); + bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr const*); + bool VisitCXXMemberCallExpr(CXXMemberCallExpr const*); + +private: + enum class Summands + { + OnlyCompileTimeConstants, + OnlySideEffectFree, + SideEffect + }; + + struct VarDeclAndSummands + { + const VarDecl* varDecl; + Summands summands; + }; + + VarDeclAndSummands findAssignOrAdd(Stmt const*); + bool checkForCompoundAssign(Stmt const* stmt1, Stmt const* stmt2, VarDeclAndSummands& varDecl); + + Expr const* ignore(Expr const*); + bool isSideEffectFree(Expr const*); + bool isCompileTimeConstant(Expr const*); +}; + +bool StringAdd::VisitCompoundStmt(CompoundStmt const* compoundStmt) +{ + if (ignoreLocation(compoundStmt)) + return true; + + auto it = compoundStmt->body_begin(); + while (true) + { + if (it == compoundStmt->body_end()) + break; + VarDeclAndSummands foundVar = findAssignOrAdd(*it); + // reference types have slightly weird behaviour + if (foundVar.varDecl && !foundVar.varDecl->getType()->isReferenceType()) + { + auto stmt1 = *it; + ++it; + while (it != compoundStmt->body_end()) + { + if (!checkForCompoundAssign(stmt1, *it, foundVar)) + { + break; + } + stmt1 = *it; + ++it; + } + } + else + ++it; + } + + return true; +} + +StringAdd::VarDeclAndSummands StringAdd::findAssignOrAdd(Stmt const* stmt) +{ + if (auto exprCleanup = dyn_cast<ExprWithCleanups>(stmt)) + stmt = exprCleanup->getSubExpr(); + if (auto switchCase = dyn_cast<SwitchCase>(stmt)) + stmt = switchCase->getSubStmt(); + + if (auto declStmt = dyn_cast<DeclStmt>(stmt)) + if (declStmt->isSingleDecl()) + if (auto varDeclLHS = dyn_cast_or_null<VarDecl>(declStmt->getSingleDecl())) + { + auto tc = loplugin::TypeCheck(varDeclLHS->getType()); + if (!tc.Class("OUString").Namespace("rtl").GlobalNamespace() + && !tc.Class("OString").Namespace("rtl").GlobalNamespace()) + return {}; + if (varDeclLHS->getStorageDuration() == SD_Static) + return {}; + if (!varDeclLHS->hasInit()) + return {}; + return { varDeclLHS, (isCompileTimeConstant(varDeclLHS->getInit()) + ? Summands::OnlyCompileTimeConstants + : (isSideEffectFree(varDeclLHS->getInit()) + ? Summands::OnlySideEffectFree + : Summands::SideEffect)) }; + } + if (auto operatorCall = dyn_cast<CXXOperatorCallExpr>(stmt)) + if (operatorCall->getOperator() == OO_Equal || operatorCall->getOperator() == OO_PlusEqual) + if (auto declRefExprLHS = dyn_cast<DeclRefExpr>(ignore(operatorCall->getArg(0)))) + if (auto varDeclLHS = dyn_cast<VarDecl>(declRefExprLHS->getDecl())) + { + auto tc = loplugin::TypeCheck(varDeclLHS->getType()); + if (!tc.Class("OUString").Namespace("rtl").GlobalNamespace() + && !tc.Class("OString").Namespace("rtl").GlobalNamespace()) + return {}; + auto rhs = operatorCall->getArg(1); + return { varDeclLHS, + (isCompileTimeConstant(rhs) + ? Summands::OnlyCompileTimeConstants + : (isSideEffectFree(rhs) ? Summands::OnlySideEffectFree + : Summands::SideEffect)) }; + } + return {}; +} + +bool StringAdd::checkForCompoundAssign(Stmt const* stmt1, Stmt const* stmt2, + VarDeclAndSummands& varDecl) +{ + // OString additions are frequently wrapped in these + if (auto exprCleanup = dyn_cast<ExprWithCleanups>(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))); + 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); + if (!ctcRhs) + { + auto const sefRhs = isSideEffectFree(rhs); + auto const oldSummands = varDecl.summands; + varDecl.summands = sefRhs ? Summands::OnlySideEffectFree : Summands::SideEffect; + if (oldSummands != Summands::OnlyCompileTimeConstants + && (oldSummands == Summands::SideEffect || !sefRhs)) + { + return true; + } + } + // if we cross a #ifdef boundary + if (containsPreprocessingConditionalInclusion( + SourceRange(stmt1->getSourceRange().getBegin(), stmt2->getSourceRange().getEnd()))) + { + varDecl.summands + = ctcRhs ? Summands::OnlyCompileTimeConstants + : isSideEffectFree(rhs) ? Summands::OnlySideEffectFree : Summands::SideEffect; + return true; + } + report(DiagnosticsEngine::Warning, "simplify by merging with the preceding assignment", + stmt2->getBeginLoc()) + << stmt2->getSourceRange(); + return true; +} + +// Check for generating temporaries when adding strings +// +bool StringAdd::VisitCXXOperatorCallExpr(CXXOperatorCallExpr const* operatorCall) +{ + if (ignoreLocation(operatorCall)) + return true; + 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](unsigned arg) { + auto const e + = dyn_cast<CXXFunctionalCastExpr>(operatorCall->getArg(arg)->IgnoreParenImpCasts()); + if (e == nullptr) + return; + auto tc3 = loplugin::TypeCheck(e->getType()); + if (!tc3.Class("OUString").Namespace("rtl").GlobalNamespace() + && !tc3.Class("OString").Namespace("rtl").GlobalNamespace() + && !tc3.Class("OUStringLiteral").Namespace("rtl").GlobalNamespace() + && !tc3.Class("OStringLiteral").Namespace("rtl").GlobalNamespace() + && !tc3.Class("OUStringBuffer").Namespace("rtl").GlobalNamespace() + && !tc3.Class("OStringBuffer").Namespace("rtl").GlobalNamespace()) + return; + report(DiagnosticsEngine::Warning, + ("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)"), + e->getBeginLoc()) + << e->getType().getLocalUnqualifiedType() << e->getSubExprAsWritten()->getType() << arg + << operatorCall->getArg(1 - arg)->IgnoreImpCasts()->getType() << e->getSourceRange(); + }; + + check(0); + check(1); + return true; +} + +bool StringAdd::VisitCXXMemberCallExpr(CXXMemberCallExpr const* methodCall) +{ + if (ignoreLocation(methodCall)) + return true; + + auto methodDecl = methodCall->getMethodDecl(); + if (!methodDecl || !methodDecl->getIdentifier() || methodDecl->getName() != "append" + || methodCall->getNumArgs() == 0) + return true; + auto tc1 = loplugin::TypeCheck(methodCall->getType()); + 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))) + return true; + + auto methodCall2 = dyn_cast<CXXMemberCallExpr>(ignore(methodCall->getImplicitObjectArgument())); + if (!methodCall2) + return true; + auto tc = loplugin::TypeCheck(methodCall2->getType()); + if (!tc.Class("OUStringBuffer").Namespace("rtl").GlobalNamespace() + && !tc.Class("OStringBuffer").Namespace("rtl").GlobalNamespace()) + return true; + auto methodDecl2 = methodCall2->getMethodDecl(); + 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", + methodCall2->getBeginLoc()) + << methodCall2->getSourceRange(); + + return true; +} + +Expr const* StringAdd::ignore(Expr const* expr) +{ + return expr->IgnoreImplicit()->IgnoreParens()->IgnoreImplicit(); +} + +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, + // the result may be incorrect, and you don't necessarily notice in tests because the order is compiler-dependent. + // for example see commit afd743141f7a7dd05914d0872c9afe079f16fe0c where such a refactoring introduced such a bug. + // So only consider simple RHS expressions. + if (!expr->HasSideEffects(compiler.getASTContext())) + return true; + + // check for chained adds which are side-effect free + if (auto operatorCall = dyn_cast<CXXOperatorCallExpr>(expr)) + { + auto op = operatorCall->getOperator(); + if (op == OO_PlusEqual || op == OO_Plus) + if (isSideEffectFree(operatorCall->getArg(0)) + && isSideEffectFree(operatorCall->getArg(1))) + return true; + } + + if (auto callExpr = dyn_cast<CallExpr>(expr)) + { + // check for calls through OUString::number/OUString::unacquired + if (auto calleeMethodDecl = dyn_cast_or_null<CXXMethodDecl>(callExpr->getCalleeDecl())) + { + if (calleeMethodDecl->getIdentifier()) + { + auto name = calleeMethodDecl->getName(); + if (callExpr->getNumArgs() > 0 + && (name == "number" || name == "unacquired" || name == "boolean" + || name == "copy")) + { + auto tc = loplugin::TypeCheck(calleeMethodDecl->getParent()); + if (tc.Class("OUString") || tc.Class("OString")) + { + if (isSideEffectFree(callExpr->getArg(0))) + return true; + } + } + } + else if (auto const d = dyn_cast<CXXConversionDecl>(calleeMethodDecl)) + { + if (loplugin::TypeCheck(d->getConversionType()) + .ClassOrStruct("basic_string_view") + .StdNamespace()) + { + auto const tc = loplugin::TypeCheck(calleeMethodDecl->getParent()); + if (tc.Class("OUString").Namespace("rtl").GlobalNamespace() + || tc.Class("OString").Namespace("rtl").GlobalNamespace()) + { + if (isSideEffectFree(callExpr->getCallee())) + return true; + } + } + } + // Aggressively assume that calls to const member functions are side effect free (if + // all of the call's sub-expressions are): + if (calleeMethodDecl->isConst()) + { + auto sef = true; + // Other options besides CXXMemberCallExpr are e.g. CXXOperatorCallExpr which + // does not have such a target expression: + if (auto const mce = dyn_cast<CXXMemberCallExpr>(callExpr)) + { + if (!isSideEffectFree(mce->getImplicitObjectArgument())) + { + sef = false; + } + } + if (sef) + { + for (unsigned i = 0; i != callExpr->getNumArgs(); ++i) + { + if (!isSideEffectFree(callExpr->getArg(i))) + { + sef = false; + break; + } + } + } + if (sef) + { + return true; + } + } + } + if (auto calleeFunctionDecl = dyn_cast_or_null<FunctionDecl>(callExpr->getCalleeDecl())) + if (calleeFunctionDecl && calleeFunctionDecl->getIdentifier()) + { + auto name = calleeFunctionDecl->getName(); + // check for calls through OUStringToOString + if (name == "OUStringToOString" || name == "OStringToOUString") + if (isSideEffectFree(callExpr->getArg(0))) + return true; + // allowlist some known-safe methods + if (name.endswith("ResId") || name == "GetXMLToken") + if (isSideEffectFree(callExpr->getArg(0))) + return true; + } + } + + // sometimes we have a constructor call on the RHS + if (auto constructExpr = dyn_cast<CXXConstructExpr>(expr)) + { + auto dc = loplugin::DeclCheck(constructExpr->getConstructor()); + if (dc.MemberFunction().Class("OUString") || dc.MemberFunction().Class("OString")) + if (constructExpr->getNumArgs() == 0 || isSideEffectFree(constructExpr->getArg(0))) + return true; + // Expr::HasSideEffects does not like stuff that passes through OUStringLiteral + auto dc2 = loplugin::DeclCheck(constructExpr->getConstructor()->getParent()); + if (dc2.Class("OUStringLiteral").Namespace("rtl").GlobalNamespace() + || dc2.Class("OStringLiteral").Namespace("rtl").GlobalNamespace()) + return true; + } + + // when adding literals, we sometimes get this + if (auto functionalCastExpr = dyn_cast<CXXFunctionalCastExpr>(expr)) + { + auto tc = loplugin::TypeCheck(functionalCastExpr->getType()); + if (tc.Class("OUStringLiteral").Namespace("rtl").GlobalNamespace() + || tc.Class("OStringLiteral").Namespace("rtl").GlobalNamespace()) + return isSideEffectFree(functionalCastExpr->getSubExpr()); + } + + return false; +} + +bool StringAdd::isCompileTimeConstant(Expr const* expr) +{ + expr = expr->IgnoreImplicit(); + if (auto cxxConstructExpr = dyn_cast<CXXConstructExpr>(expr)) + if (cxxConstructExpr->getNumArgs() > 0) + expr = cxxConstructExpr->getArg(0); + return isa<clang::StringLiteral>(expr); +} + +loplugin::Plugin::Registration<StringAdd> stringadd("stringadd"); +} + +#endif // LO_CLANG_SHARED_PLUGINS + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ |