diff options
Diffstat (limited to 'compilerplugins/clang/reducevarscope.cxx')
| -rw-r--r-- | compilerplugins/clang/reducevarscope.cxx | 551 |
1 files changed, 551 insertions, 0 deletions
diff --git a/compilerplugins/clang/reducevarscope.cxx b/compilerplugins/clang/reducevarscope.cxx new file mode 100644 index 000000000..be7254a2d --- /dev/null +++ b/compilerplugins/clang/reducevarscope.cxx @@ -0,0 +1,551 @@ +/* -*- 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/. + */ + +#include <cassert> +#include <string> +#include <iostream> +#include <map> +#include <set> +#include <vector> +#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" + +// Original idea from mike kaganski. +// Look for variables that can have their scoped reduced, which makes the code easier to read. + +// TODO when dealing with vars that are referenced in multiple child blocks, the check is very primitive +// and could be greatly improved. + +namespace +{ +class ReduceVarScope : public loplugin::FilteringPlugin<ReduceVarScope> +{ +public: + explicit ReduceVarScope(loplugin::InstantiationData const& data) + : FilteringPlugin(data) + { + } + + bool preRun() override + { + if (!compiler.getLangOpts().CPlusPlus) + return false; + // ignore some files with problematic macros + std::string fn(handler.getMainFileName()); + loplugin::normalizeDotDotInFilePath(fn); + // some declarations look better all together + if (fn == SRCDIR "/package/source/manifest/ManifestExport.cxx") + return false; + // storing pointer to OUString internal data + if (fn == SRCDIR "/connectivity/source/drivers/odbc/ODatabaseMetaDataResultSet.cxx" + || fn == SRCDIR "/sc/source/filter/excel/xestyle.cxx" + || fn == SRCDIR "/sw/source/filter/html/htmlflywriter.cxx" + || fn == SRCDIR "/unoxml/source/dom/element.cxx" + || fn == SRCDIR "/unoxml/source/dom/document.cxx" + || fn == SRCDIR "/sd/source/filter/eppt/pptx-animations.cxx") + return false; + if (fn == SRCDIR "/sal/qa/rtl/strings/nonconstarray.cxx") + return false; + return true; + } + + virtual void run() override + { + if (!preRun()) + return; + + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + postRun(); + } + + virtual void postRun() override + { + for (auto const& pair : maVarDeclMap) + { + auto varDecl = pair.first; + auto const& depthInfo = pair.second; + if (depthInfo.maDeclBlockPath.size() == depthInfo.maCommonBlockPath.size()) + continue; + if (maVarDeclToIgnoreSet.find(varDecl) != maVarDeclToIgnoreSet.end()) + continue; + auto it = maVarUseSourceRangeMap.find(varDecl); + if (it == maVarUseSourceRangeMap.end()) + continue; + report(DiagnosticsEngine::Warning, "can reduce scope of var", varDecl->getLocation()) + << varDecl->getSourceRange(); + for (SourceRange const& useRange : it->second) + report(DiagnosticsEngine::Note, "used here", useRange.getBegin()) << useRange; + } + } + + bool VisitUnaryOperator(UnaryOperator const* expr) + { + // if we take the address of it + UnaryOperator::Opcode op = expr->getOpcode(); + if (op == UO_AddrOf) + recordIgnore(expr->getSubExpr()); + return true; + } + + bool VisitDeclRefExpr(const DeclRefExpr*); + bool VisitVarDecl(const VarDecl*); + bool VisitLambdaExpr(const LambdaExpr*); + + bool PreTraverseFunctionDecl(FunctionDecl*); + bool PostTraverseFunctionDecl(FunctionDecl*, bool); + bool TraverseFunctionDecl(FunctionDecl*); + + bool PreTraverseCompoundStmt(CompoundStmt*); + bool PostTraverseCompoundStmt(CompoundStmt*, bool); + bool TraverseCompoundStmt(CompoundStmt*); + + bool PreTraverseWhileStmt(WhileStmt*); + bool PostTraverseWhileStmt(WhileStmt*, bool); + bool TraverseWhileStmt(WhileStmt*); + + bool PreTraverseDoStmt(DoStmt*); + bool PostTraverseDoStmt(DoStmt*, bool); + bool TraverseDoStmt(DoStmt*); + + bool PreTraverseCXXForRangeStmt(CXXForRangeStmt*); + bool PostTraverseCXXForRangeStmt(CXXForRangeStmt*, bool); + bool TraverseCXXForRangeStmt(CXXForRangeStmt*); + + bool PreTraverseForStmt(ForStmt*); + bool PostTraverseForStmt(ForStmt*, bool); + bool TraverseForStmt(ForStmt*); + + bool PreTraverseSwitchStmt(SwitchStmt*); + bool PostTraverseSwitchStmt(SwitchStmt*, bool); + bool TraverseSwitchStmt(SwitchStmt*); + +private: + struct DepthInfo + { + unsigned int mnFirstDepth = 0; + unsigned int mnFirstLoopDepth = 0; + std::vector<unsigned int> maDeclBlockPath = {}; + std::vector<unsigned int> maCommonBlockPath = {}; + }; + std::unordered_map<VarDecl const*, DepthInfo> maVarDeclMap; // varDecl->depth + std::unordered_set<VarDecl const*> maVarDeclToIgnoreSet; + std::map<VarDecl const*, std::vector<SourceRange>> maVarUseSourceRangeMap; + std::vector<unsigned int> maCurrentBlockPath; + unsigned int mnCurrentDepth = 0; + unsigned int mnCurrentLoopDepth = 0; + static unsigned int gnBlockId; + + bool isTypeOK(QualType qt); + bool isInitConstant(const VarDecl* varDecl); + + void recordIgnore(Expr const* expr) + { + for (;;) + { + expr = expr->IgnoreParenImpCasts(); + if (auto const e = dyn_cast<MemberExpr>(expr)) + { + if (isa<FieldDecl>(e->getMemberDecl())) + { + expr = e->getBase(); + continue; + } + } + if (auto const e = dyn_cast<ArraySubscriptExpr>(expr)) + { + expr = e->getBase(); + continue; + } + if (auto const e = dyn_cast<BinaryOperator>(expr)) + { + if (e->getOpcode() == BO_PtrMemD) + { + expr = e->getLHS(); + continue; + } + } + break; + } + auto const dre = dyn_cast<DeclRefExpr>(expr); + if (dre == nullptr) + return; + auto const var = dyn_cast<VarDecl>(dre->getDecl()); + if (var == nullptr) + return; + maVarDeclToIgnoreSet.insert(var); + } +}; + +unsigned int ReduceVarScope::gnBlockId = 0; + +bool ReduceVarScope::PreTraverseFunctionDecl(FunctionDecl* functionDecl) +{ + // Ignore functions that contains #ifdef-ery, can be quite tricky + // to make useful changes when this plugin fires in such functions + if (containsPreprocessingConditionalInclusion(functionDecl->getSourceRange())) + return false; + return true; +} + +bool ReduceVarScope::PostTraverseFunctionDecl(FunctionDecl*, bool) { return true; } + +bool ReduceVarScope::TraverseFunctionDecl(FunctionDecl* functionDecl) +{ + bool ret = true; + if (PreTraverseFunctionDecl(functionDecl)) + { + ret = FilteringPlugin::TraverseFunctionDecl(functionDecl); + PostTraverseFunctionDecl(functionDecl, ret); + } + return ret; +} + +bool ReduceVarScope::PreTraverseCompoundStmt(CompoundStmt*) +{ + assert(mnCurrentDepth != std::numeric_limits<unsigned int>::max()); + ++mnCurrentDepth; + ++gnBlockId; + maCurrentBlockPath.push_back(gnBlockId); + return true; +} + +bool ReduceVarScope::PostTraverseCompoundStmt(CompoundStmt*, bool) +{ + assert(mnCurrentDepth != 0); + --mnCurrentDepth; + maCurrentBlockPath.pop_back(); + return true; +} + +bool ReduceVarScope::TraverseCompoundStmt(CompoundStmt* decl) +{ + bool ret = true; + if (PreTraverseCompoundStmt(decl)) + { + ret = FilteringPlugin::TraverseCompoundStmt(decl); + PostTraverseCompoundStmt(decl, ret); + } + return ret; +} + +bool ReduceVarScope::PreTraverseWhileStmt(WhileStmt*) +{ + assert(mnCurrentLoopDepth != std::numeric_limits<unsigned int>::max()); + ++mnCurrentLoopDepth; + return true; +} + +bool ReduceVarScope::PostTraverseWhileStmt(WhileStmt*, bool) +{ + assert(mnCurrentLoopDepth != 0); + --mnCurrentLoopDepth; + return true; +} + +bool ReduceVarScope::TraverseWhileStmt(WhileStmt* decl) +{ + bool ret = true; + if (PreTraverseWhileStmt(decl)) + { + ret = FilteringPlugin::TraverseWhileStmt(decl); + PostTraverseWhileStmt(decl, ret); + } + return ret; +} + +bool ReduceVarScope::PreTraverseDoStmt(DoStmt*) +{ + assert(mnCurrentLoopDepth != std::numeric_limits<unsigned int>::max()); + ++mnCurrentLoopDepth; + return true; +} + +bool ReduceVarScope::PostTraverseDoStmt(DoStmt*, bool) +{ + assert(mnCurrentLoopDepth != 0); + --mnCurrentLoopDepth; + return true; +} + +bool ReduceVarScope::TraverseDoStmt(DoStmt* decl) +{ + bool ret = true; + if (PreTraverseDoStmt(decl)) + { + ret = FilteringPlugin::TraverseDoStmt(decl); + PostTraverseDoStmt(decl, ret); + } + return ret; +} + +bool ReduceVarScope::PreTraverseSwitchStmt(SwitchStmt*) +{ + assert(mnCurrentLoopDepth != std::numeric_limits<unsigned int>::max()); + ++mnCurrentLoopDepth; + return true; +} + +bool ReduceVarScope::PostTraverseSwitchStmt(SwitchStmt*, bool) +{ + assert(mnCurrentLoopDepth != 0); + --mnCurrentLoopDepth; + return true; +} + +// Consider a switch to be a loop, because weird things happen inside it +bool ReduceVarScope::TraverseSwitchStmt(SwitchStmt* decl) +{ + bool ret = true; + if (PreTraverseSwitchStmt(decl)) + { + ret = FilteringPlugin::TraverseSwitchStmt(decl); + PostTraverseSwitchStmt(decl, ret); + } + return ret; +} + +bool ReduceVarScope::PreTraverseCXXForRangeStmt(CXXForRangeStmt*) +{ + assert(mnCurrentLoopDepth != std::numeric_limits<unsigned int>::max()); + ++mnCurrentLoopDepth; + return true; +} + +bool ReduceVarScope::PostTraverseCXXForRangeStmt(CXXForRangeStmt*, bool) +{ + assert(mnCurrentLoopDepth != 0); + --mnCurrentLoopDepth; + return true; +} + +bool ReduceVarScope::TraverseCXXForRangeStmt(CXXForRangeStmt* decl) +{ + bool ret = true; + if (PreTraverseCXXForRangeStmt(decl)) + { + ret = FilteringPlugin::TraverseCXXForRangeStmt(decl); + PostTraverseCXXForRangeStmt(decl, ret); + } + return ret; +} + +bool ReduceVarScope::PreTraverseForStmt(ForStmt* forStmt) +{ + assert(mnCurrentLoopDepth != std::numeric_limits<unsigned int>::max()); + ++mnCurrentLoopDepth; + + auto declStmt = dyn_cast_or_null<DeclStmt>(forStmt->getInit()); + if (declStmt) + { + if (declStmt->isSingleDecl()) + { + if (auto varDecl = dyn_cast_or_null<VarDecl>(declStmt->getSingleDecl())) + maVarDeclToIgnoreSet.insert(varDecl); + } + else + { + for (auto const& decl : declStmt->getDeclGroup()) + if (auto varDecl = dyn_cast_or_null<VarDecl>(decl)) + maVarDeclToIgnoreSet.insert(varDecl); + } + } + + return true; +} + +bool ReduceVarScope::PostTraverseForStmt(ForStmt*, bool) +{ + assert(mnCurrentLoopDepth != 0); + --mnCurrentLoopDepth; + return true; +} + +bool ReduceVarScope::TraverseForStmt(ForStmt* decl) +{ + bool ret = true; + if (PreTraverseForStmt(decl)) + { + ret = FilteringPlugin::TraverseForStmt(decl); + PostTraverseForStmt(decl, ret); + } + return ret; +} + +bool ReduceVarScope::VisitVarDecl(const VarDecl* varDecl) +{ + if (ignoreLocation(varDecl)) + return true; + if (varDecl->isExceptionVariable() || isa<ParmVarDecl>(varDecl)) + return true; + // ignore stuff in header files (which should really not be there, but anyhow) + if (!compiler.getSourceManager().isInMainFile(varDecl->getLocation())) + return true; + // Ignore macros like FD_ZERO + if (compiler.getSourceManager().isMacroBodyExpansion(varDecl->getBeginLoc())) + return true; + if (varDecl->hasGlobalStorage()) + return true; + if (varDecl->isConstexpr()) + return true; + if (varDecl->isInitCapture()) + return true; + if (varDecl->isCXXForRangeDecl()) + return true; + if (!isTypeOK(varDecl->getType())) + return true; + + if (varDecl->hasInit() && !isInitConstant(varDecl)) + return true; + + maVarDeclMap[varDecl].mnFirstDepth = mnCurrentDepth; + maVarDeclMap[varDecl].mnFirstLoopDepth = mnCurrentLoopDepth; + maVarDeclMap[varDecl].maDeclBlockPath = maCurrentBlockPath; + + return true; +} + +bool ReduceVarScope::isInitConstant(const VarDecl* varDecl) +{ + // check for string or scalar literals + const Expr* initExpr = varDecl->getInit(); + if (auto e = dyn_cast<ExprWithCleanups>(initExpr)) + initExpr = e->getSubExpr(); + if (isa<clang::StringLiteral>(initExpr)) + return true; + if (auto constructExpr = dyn_cast<CXXConstructExpr>(initExpr)) + { + if (constructExpr->getNumArgs() == 0) + { + return true; // i.e., empty string + } + else + { + auto stringLit2 = dyn_cast<clang::StringLiteral>(constructExpr->getArg(0)); + if (stringLit2) + return true; + } + } + + auto const init = varDecl->getInit(); + if (init->isValueDependent()) + return false; + return init->isConstantInitializer(compiler.getASTContext(), false /*ForRef*/); +} + +bool ReduceVarScope::isTypeOK(QualType varType) +{ + // TODO improve this - requires more analysis because it's really easy to + // take a pointer to an array + if (varType->isArrayType()) + return false; + + if (varType.isCXX11PODType(compiler.getASTContext())) + return true; + if (!varType->isRecordType()) + return false; + auto recordDecl = dyn_cast_or_null<CXXRecordDecl>(varType->getAs<RecordType>()->getDecl()); + if (recordDecl && recordDecl->hasTrivialDestructor()) + return true; + auto const tc = loplugin::TypeCheck(varType); + // Safe types with destructors that don't do anything interesting + if (tc.Class("OString").Namespace("rtl").GlobalNamespace() + || tc.Class("OUString").Namespace("rtl").GlobalNamespace() + || tc.Class("OStringBuffer").Namespace("rtl").GlobalNamespace() + || tc.Class("OUStringBuffer").Namespace("rtl").GlobalNamespace() + || tc.Class("Color").GlobalNamespace() || tc.Class("Pair").GlobalNamespace() + || tc.Class("Point").GlobalNamespace() || tc.Class("Size").GlobalNamespace() + || tc.Class("Range").GlobalNamespace() || tc.Class("Selection").GlobalNamespace() + || tc.Class("Rectangle").Namespace("tools").GlobalNamespace()) + return true; + return false; +} + +bool ReduceVarScope::VisitDeclRefExpr(const DeclRefExpr* declRefExpr) +{ + if (ignoreLocation(declRefExpr)) + return true; + const Decl* decl = declRefExpr->getDecl(); + if (!isa<VarDecl>(decl) || isa<ParmVarDecl>(decl)) + return true; + const VarDecl* varDecl = dyn_cast<VarDecl>(decl)->getCanonicalDecl(); + // ignore stuff in header files (which should really not be there, but anyhow) + if (!compiler.getSourceManager().isInMainFile(varDecl->getLocation())) + return true; + + auto varIt = maVarDeclMap.find(varDecl); + if (varIt == maVarDeclMap.end()) + return true; + + auto& depthInfo = varIt->second; + + // merge block paths to get common ancestor path + if (depthInfo.maCommonBlockPath.empty()) + depthInfo.maCommonBlockPath = maCurrentBlockPath; + else + { + auto len = std::min(depthInfo.maCommonBlockPath.size(), maCurrentBlockPath.size()); + unsigned int i = 0; + while (i < len && depthInfo.maCommonBlockPath[i] == maCurrentBlockPath[i]) + ++i; + depthInfo.maCommonBlockPath.resize(i); + if (depthInfo.maCommonBlockPath == depthInfo.maDeclBlockPath) + { + maVarDeclMap.erase(varIt); + maVarUseSourceRangeMap.erase(varDecl); + return true; + } + } + + // seen in a loop below initial decl + if (mnCurrentLoopDepth > depthInfo.mnFirstLoopDepth) + { + // TODO, we could additionally check if we are reading or writing to the var inside a loop + // We only need to exclude vars that are written to, or passed taken-addr-of, or have non-const method called, + // or passed as arg to non-const-ref parameter. + maVarDeclMap.erase(varIt); + maVarUseSourceRangeMap.erase(varDecl); + return true; + } + + auto it = maVarUseSourceRangeMap.find(varDecl); + if (it == maVarUseSourceRangeMap.end()) + it = maVarUseSourceRangeMap.emplace(varDecl, std::vector<SourceRange>()).first; + it->second.push_back(declRefExpr->getSourceRange()); + + return true; +} + +bool ReduceVarScope::VisitLambdaExpr(const LambdaExpr* lambdaExpr) +{ + if (ignoreLocation(lambdaExpr)) + return true; + for (auto captureIt = lambdaExpr->capture_begin(); captureIt != lambdaExpr->capture_end(); + ++captureIt) + { + const LambdaCapture& capture = *captureIt; + if (capture.capturesVariable()) + { + auto varDecl = capture.getCapturedVar(); + maVarDeclMap.erase(varDecl); + maVarUseSourceRangeMap.erase(varDecl); + } + } + return true; +} + +loplugin::Plugin::Registration<ReduceVarScope> reducevarscope("reducevarscope", false); +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ |