libreoffice/compilerplugins/clang/elidestringvar.cxx

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

457 lines
13 KiB
C++
Raw Normal View History

/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
/*
* 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 <algorithm>
#include <cassert>
#include <map>
#include "check.hxx"
#include "compat.hxx"
#include "plugin.hxx"
// Find cases where a variable of a string type (at least for now, only OUString) is initialized
// with a literal value (incl. as an empty string) and used only once. Conservatively this only
// covers local non-static variables that are not defined outside of the loop (if any) in which they
// are used, as other cases may deliberately use the variable for performance (or even correctness,
// if addresses are taken and compared) reasons.
//
// For one, the historically heavy syntax for such uses of string literals
// (RTL_CONSTASCII_USTRINGPARAM etc.) probably explains many of these redundant variables, which can
// now be considered cargo-cult baggage. For another, some of those variables are used as arguments
// to functions which also have more efficient overloads directly taking string literals. And for
// yet another, some cases with default-initialized variables turned out to be effectively unused
// code that could be removed completely (d073cca5f7c04de3e1bcedda334d864e98ac7835 "Clean up dead
// code", 91345e7dde6100496a7c9e815b72b2821ae82bc2 "Clean up dead code",
// 868b0763ac47f765cb48c277897274a595b831d0 "Upcoming loplugin:elidestringvar: dbaccess" in
// dbaccess/source/ui/app/AppController.cxx, bde0aac4ccf7b830b5ef21d5b9e75e62aee6aaf9 "Clean up dead
// code", 354aefec42de856b4ab6201ada54a3a3c630b4bd "Upcoming loplugin:elidestringvar: cui" in
// cui/source/dialogs/SpellDialog.cxx).
namespace
{
class ElideStringVar : public loplugin::FilteringPlugin<ElideStringVar>
{
public:
explicit ElideStringVar(loplugin::InstantiationData const& data)
: FilteringPlugin(data)
{
}
bool preRun() override { return compiler.getLangOpts().CPlusPlus; }
void postRun() override
{
for (auto const& var : vars_)
{
if (!var.second.singleUse || *var.second.singleUse == nullptr)
{
continue;
}
if (containsPreprocessingConditionalInclusion(SourceRange(
compat::getBeginLoc(var.first), compat::getEndLoc(*var.second.singleUse))))
{
// This is not perfect, as additional uses can be hidden in conditional blocks that
// only start after the (would-be) single use (as was the case in
// 3bc5057f9689e024957cfa898a221ee2c4c4afe7 "Upcoming loplugin:elidestringvar:
// testtools" when built with --enable-debug, but where also fixing the hidden
// additional use was trivial). If this ever becomes a real problem, we can extend
// the above check to cover more of the current function body's remainder.
continue;
}
report(DiagnosticsEngine::Warning,
"replace single use of literal OUString variable with a literal",
(*var.second.singleUse)->getExprLoc())
<< (*var.second.singleUse)->getSourceRange();
report(DiagnosticsEngine::Note, "literal OUString variable defined here",
var.first->getLocation())
<< var.first->getSourceRange();
}
}
bool VisitVarDecl(VarDecl const* decl)
{
if (ignoreLocation(decl))
{
return true;
}
if (!decl->isThisDeclarationADefinition())
{
return true;
}
if (isa<ParmVarDecl>(decl))
{
return true;
}
if (decl->getStorageDuration() != SD_Automatic)
{
return true;
}
if (!loplugin::TypeCheck(decl->getType())
.Class("OUString")
.Namespace("rtl")
.GlobalNamespace())
{
return true;
}
if (!decl->hasInit())
{
return true;
}
auto const e1 = dyn_cast<CXXConstructExpr>(decl->getInit()->IgnoreParenImpCasts());
if (e1 == nullptr)
{
return true;
}
if (!loplugin::TypeCheck(e1->getType())
.Class("OUString")
.Namespace("rtl")
.GlobalNamespace())
{
return true;
}
switch (e1->getNumArgs())
{
case 0:
break;
case 1:
{
auto const e2 = e1->getArg(0);
if (loplugin::TypeCheck(e2->getType())
Turn OUStringLiteral into a consteval'ed, static-refcound rtl_uString ...from which an OUString can cheaply be instantiated. This is the OUString equivalent of 4b9e440c51be3e40326bc90c33ae69885bfb51e4 "Turn OStringLiteral into a consteval'ed, static-refcound rtl_String". Most remarks about that commit apply here too (this commit is just substantially bigger and a bit more complicated because there were so much more uses of OUStringLiteral than of OStringLiteral): The one downside is that OUStringLiteral now needs to be a template abstracting over the string length. But any uses for which that is a problem (e.g., as the element type of a container that would no longer be homogeneous, or in the signature of a function that shall not be turned into a template for one reason or another) can be replaced with std::u16string_view, without loss of efficiency compared to the original OUStringLiteral, and without loss of expressivity. The new OUStringLiteral ctor code would probably not be very efficient if it were ever executed at runtime, but it is intended to be only executed at compile time. Where available, C++20 "consteval" is used to statically ensure that. The intended use of the new OUStringLiteral is in all cases where an object that shall itself not be an OUString (e.g., because it shall be a global static variable for which the OUString ctor/dtor would be detrimental at library load/unload) must be converted to an OUString instance in at least one place. Other string literal abstractions could use std::u16string_view (or just plain char16_t const[N]), but interestingly OUStringLiteral might be more efficient than constexpr std::u16string_view even for such cases, as it should not need any relocations at library load time. For now, no existing uses of OUStringLiteral have been changed to some other abstraction (unless technically necessary as discussed above), and no additional places that would benefit from OUStringLiteral have been changed to use it. Global constexpr OUStringLiteral variables defined in an included file would be somewhat suboptimal, as each translation unit that uses them would create its own, unshared instance. The envisioned solution is to turn them into static data members of some class (and there may be a loplugin coming to find and fix affected places). Another approach that has been taken here in a few cases where such variables were only used in one .cxx anyway is to move their definitions from the .hxx into that one .cxx (in turn causing some files to become empty and get removed completely)---which also silenced some GCC -Werror=unused-variable if a variable from a .hxx was not used in some .cxx including it. To keep individual commits reasonably manageable, some consumers of OUStringLiteral in rtl/ustrbuf.hxx and rtl/ustring.hxx are left in a somewhat odd state for now, where they don't take advantage of OUStringLiteral's equivalence to rtl_uString, but just keep extracting its contents and copy it elsewhere. In follow-up commits, those consumers should be changed appropriately, making them treat OUStringLiteral like an rtl_uString or dropping the OUStringLiteral overload in favor of an existing (and cheap to use now) OUString overload, etc. In a similar vein, comparison operators between OUString and std::u16string_view have been added to the existing plethora of comparison operator overloads. It would be nice to eventually consolidate them, esp. with the overloads taking OUStringLiteral and/or char16_t const[N] string literals, but that appears tricky to get right without introducing new ambiguities. Also, a handful of places across the code base use comparisons between OUString and OUStringNumber, which are now ambiguous (converting the OUStringNumber to either OUString or std::u16string_view). For simplicity, those few places have manually been fixed for now by adding explicit conversion to std::u16string_view. Also some compilerplugins code needed to be adapted, and some of the compilerplugins/test cases have become irrelevant (and have been removed), as the tested code would no longer compile in the first place. sal/qa/rtl/strings/test_oustring_concat.cxx documents a workaround for GCC bug <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96878> "Failed class template argument deduction in unevaluated, parenthesized context". That place, as well as uses of OUStringLiteral in extensions/source/abpilot/fieldmappingimpl.cxx and i18npool/source/localedata/localedata.cxx, which have been replaced with OUString::Concat (and which is arguably a better choice, anyway), also caused failures with at least Clang 5.0.2 (but would not have caused failures with at least recent Clang 12 trunk, so appear to be bugs in Clang that have meanwhile been fixed). Change-Id: I34174462a28f2000cfeb2d219ffd533a767920b8 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102222 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
2020-09-08 09:48:17 +02:00
.Class("OUStringLiteral")
.Namespace("rtl")
.GlobalNamespace())
{
break;
}
if (e2->isIntegerConstantExpr(compiler.getASTContext()))
{
break;
}
return true;
}
case 2:
{
auto const t = e1->getArg(0)->getType();
if (!(t.isConstQualified() && t->isConstantArrayType()))
{
return true;
}
auto const e2 = e1->getArg(1);
if (!(isa<CXXDefaultArgExpr>(e2)
&& loplugin::TypeCheck(e2->getType())
.Struct("Dummy")
.Namespace("libreoffice_internal")
.Namespace("rtl")
.GlobalNamespace()))
{
return true;
}
break;
}
default:
return true;
}
auto const ok = vars_.emplace(decl, Data(getInnermostLoop()));
assert(ok.second);
(void)ok;
return true;
}
bool VisitDeclRefExpr(DeclRefExpr const* expr)
{
if (ignoreLocation(expr))
{
return true;
}
auto const var = dyn_cast<VarDecl>(expr->getDecl());
if (var == nullptr)
{
return true;
}
auto const i = vars_.find(var);
if (i == vars_.end())
{
return true;
}
i->second.singleUse
= i->second.singleUse || i->second.innermostLoop != getInnermostLoop() ? nullptr : expr;
return true;
}
bool VisitMemberExpr(MemberExpr const* expr)
{
if (ignoreLocation(expr))
{
return true;
}
auto const e = dyn_cast<DeclRefExpr>(expr->getBase()->IgnoreParenImpCasts());
if (e == nullptr)
{
return true;
}
auto const var = dyn_cast<VarDecl>(e->getDecl());
if (var == nullptr)
{
return true;
}
auto const i = vars_.find(var);
if (i == vars_.end())
{
return true;
}
i->second.singleUse = nullptr;
return true;
}
bool VisitUnaryOperator(UnaryOperator const* expr)
{
if (ignoreLocation(expr))
{
return true;
}
if (expr->getOpcode() != UO_AddrOf)
{
return true;
}
auto const e = dyn_cast<DeclRefExpr>(expr->getSubExpr()->IgnoreParenImpCasts());
if (e == nullptr)
{
return true;
}
auto const var = dyn_cast<VarDecl>(e->getDecl());
if (var == nullptr)
{
return true;
}
auto const i = vars_.find(var);
if (i == vars_.end())
{
return true;
}
i->second.singleUse = nullptr;
return true;
}
bool VisitCallExpr(CallExpr const* expr)
{
if (ignoreLocation(expr))
{
return true;
}
auto const fun = expr->getDirectCallee();
if (fun == nullptr)
{
return true;
}
unsigned const n = std::min(fun->getNumParams(), expr->getNumArgs());
for (unsigned i = 0; i != n; ++i)
{
if (!loplugin::TypeCheck(fun->getParamDecl(i)->getType())
.LvalueReference()
.NonConstVolatile())
{
continue;
}
auto const e = dyn_cast<DeclRefExpr>(expr->getArg(i)->IgnoreParenImpCasts());
if (e == nullptr)
{
continue;
}
auto const var = dyn_cast<VarDecl>(e->getDecl());
if (var == nullptr)
{
continue;
}
auto const j = vars_.find(var);
if (j == vars_.end())
{
continue;
}
j->second.singleUse = nullptr;
}
return true;
}
bool VisitCXXConstructExpr(CXXConstructExpr const* expr)
{
if (ignoreLocation(expr))
{
return true;
}
auto const ctor = expr->getConstructor();
unsigned const n = std::min(ctor->getNumParams(), expr->getNumArgs());
for (unsigned i = 0; i != n; ++i)
{
if (!loplugin::TypeCheck(ctor->getParamDecl(i)->getType())
.LvalueReference()
.NonConstVolatile())
{
continue;
}
auto const e = dyn_cast<DeclRefExpr>(expr->getArg(i)->IgnoreParenImpCasts());
if (e == nullptr)
{
continue;
}
auto const var = dyn_cast<VarDecl>(e->getDecl());
if (var == nullptr)
{
continue;
}
auto const j = vars_.find(var);
if (j == vars_.end())
{
continue;
}
j->second.singleUse = nullptr;
}
return true;
}
bool TraverseWhileStmt(WhileStmt* stmt)
{
bool ret = true;
if (PreTraverseWhileStmt(stmt))
{
ret = FilteringPlugin::TraverseWhileStmt(stmt);
PostTraverseWhileStmt(stmt, ret);
}
return ret;
}
bool PreTraverseWhileStmt(WhileStmt* stmt)
{
innermostLoop_.push(stmt);
return true;
}
bool PostTraverseWhileStmt(WhileStmt* stmt, bool)
{
assert(!innermostLoop_.empty());
assert(innermostLoop_.top() == stmt);
(void)stmt;
innermostLoop_.pop();
return true;
}
bool TraverseDoStmt(DoStmt* stmt)
{
bool ret = true;
if (PreTraverseDoStmt(stmt))
{
ret = FilteringPlugin::TraverseDoStmt(stmt);
PostTraverseDoStmt(stmt, ret);
}
return ret;
}
bool PreTraverseDoStmt(DoStmt* stmt)
{
innermostLoop_.push(stmt);
return true;
}
bool PostTraverseDoStmt(DoStmt* stmt, bool)
{
assert(!innermostLoop_.empty());
assert(innermostLoop_.top() == stmt);
(void)stmt;
innermostLoop_.pop();
return true;
}
bool TraverseForStmt(ForStmt* stmt)
{
bool ret = true;
if (PreTraverseForStmt(stmt))
{
ret = FilteringPlugin::TraverseForStmt(stmt);
PostTraverseForStmt(stmt, ret);
}
return ret;
}
bool PreTraverseForStmt(ForStmt* stmt)
{
innermostLoop_.push(stmt);
return true;
}
bool PostTraverseForStmt(ForStmt* stmt, bool)
{
assert(!innermostLoop_.empty());
assert(innermostLoop_.top() == stmt);
(void)stmt;
innermostLoop_.pop();
return true;
}
bool TraverseCXXForRangeStmt(CXXForRangeStmt* stmt)
{
bool ret = true;
if (PreTraverseCXXForRangeStmt(stmt))
{
ret = FilteringPlugin::TraverseCXXForRangeStmt(stmt);
PostTraverseCXXForRangeStmt(stmt, ret);
}
return ret;
}
bool PreTraverseCXXForRangeStmt(CXXForRangeStmt* stmt)
{
innermostLoop_.push(stmt);
return true;
}
bool PostTraverseCXXForRangeStmt(CXXForRangeStmt* stmt, bool)
{
assert(!innermostLoop_.empty());
assert(innermostLoop_.top() == stmt);
(void)stmt;
innermostLoop_.pop();
return true;
}
private:
void run() override
{
if (preRun() && TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()))
{
postRun();
}
}
Stmt const* getInnermostLoop() const
{
return innermostLoop_.empty() ? nullptr : innermostLoop_.top();
}
struct Data
{
Data(Stmt const* theInnermostLoop)
: innermostLoop(theInnermostLoop)
{
}
Stmt const* innermostLoop;
llvm::Optional<Expr const*> singleUse;
};
std::stack<Stmt const*> innermostLoop_;
std::map<VarDecl const*, Data> vars_;
};
loplugin::Plugin::Registration<ElideStringVar> elidestringvar("elidestringvar");
}
#endif
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */