[Libreoffice-commits] core.git: compilerplugins/clang cppuhelper/source drawinglayer/source formula/source i18nlangtag/source i18npool/qa idlc/source l10ntools/source lingucomponent/source linguistic/source sc/qa slideshow/source solenv/CompilerTest_compilerplugins_clang.mk sot/source stoc/source sw/source writerfilter/source xmlhelp/source
Noel Grandin (via logerrit)
logerrit at kemper.freedesktop.org
Tue Nov 10 16:11:48 UTC 2020
compilerplugins/clang/reducevarscope.cxx | 550 +++++++++++++
compilerplugins/clang/test/reducevarscope.cxx | 96 ++
cppuhelper/source/servicemanager.cxx | 2
drawinglayer/source/processor2d/vclmetafileprocessor2d.cxx | 2
formula/source/core/api/FormulaCompiler.cxx | 2
formula/source/core/api/token.cxx | 3
i18nlangtag/source/languagetag/languagetag.cxx | 3
i18npool/qa/cppunit/test_breakiterator.cxx | 6
idlc/source/idlc.cxx | 2
idlc/source/options.cxx | 2
l10ntools/source/cfgmerge.cxx | 2
l10ntools/source/helpmerge.cxx | 3
lingucomponent/source/hyphenator/hyphen/hyphenimp.cxx | 3
linguistic/source/hyphdsp.cxx | 2
sc/qa/unit/ucalc_sort.cxx | 4
slideshow/source/engine/shapes/drawinglayeranimation.cxx | 2
solenv/CompilerTest_compilerplugins_clang.mk | 1
sot/source/sdstor/stgavl.cxx | 3
stoc/source/defaultregistry/defaultregistry.cxx | 9
sw/source/core/txtnode/fntcache.cxx | 4
writerfilter/source/dmapper/PropertyMap.cxx | 4
xmlhelp/source/cxxhelp/provider/databases.cxx | 2
22 files changed, 672 insertions(+), 35 deletions(-)
New commits:
commit 242320d303d43a34ce2255a07783fbd51e253cd0
Author: Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Sat Sep 5 11:53:35 2020 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Tue Nov 10 17:11:00 2020 +0100
new loplugin:reducevarscope
Change-Id: Iefe922c2e0d605114d54673d63eccc5e4abd545d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102143
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/compilerplugins/clang/reducevarscope.cxx b/compilerplugins/clang/reducevarscope.cxx
new file mode 100644
index 000000000000..c293fd432e6a
--- /dev/null
+++ b/compilerplugins/clang/reducevarscope.cxx
@@ -0,0 +1,550 @@
+/* -*- 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 "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(compat::getBeginLoc(varDecl)))
+ 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: */
diff --git a/compilerplugins/clang/test/reducevarscope.cxx b/compilerplugins/clang/test/reducevarscope.cxx
new file mode 100644
index 000000000000..ee600c988efe
--- /dev/null
+++ b/compilerplugins/clang/test/reducevarscope.cxx
@@ -0,0 +1,96 @@
+/* -*- 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/.
+ */
+
+#include <rtl/ustring.hxx>
+
+void test1()
+{
+ int i = 2; // expected-error {{can reduce scope of var [loplugin:reducevarscope]}}
+ {
+ i = 3; // expected-note {{used here [loplugin:reducevarscope]}}
+ }
+ int j = 2; // expected-error {{can reduce scope of var [loplugin:reducevarscope]}}
+ {
+ j = 3; // expected-note {{used here [loplugin:reducevarscope]}}
+ {
+ j = 4; // expected-note {{used here [loplugin:reducevarscope]}}
+ }
+ }
+}
+
+// negative test - seen inside a loop
+void test2()
+{
+ int i = 2;
+ for (int j = 1; j < 10; ++j)
+ {
+ i = 3;
+ }
+}
+
+// negative test - initial assignment from non-constant
+void test3()
+{
+ int j = 1;
+ int i = j;
+ {
+ i = 3;
+ }
+}
+
+// negative test
+void test4()
+{
+ int i = 2;
+ {
+ i = 3;
+ }
+ i = 4;
+}
+
+// negative test
+void test5()
+{
+ int i = 2;
+ i = 3;
+}
+
+// negative test - seen in 2 child blocks
+void test6()
+{
+ int i;
+ {
+ i = 3;
+ }
+ {
+ i = 3;
+ }
+}
+
+// TODO negative test - storing pointer to OUString data
+// void test7()
+// {
+// OUString s;
+// const sal_Unicode* p = nullptr;
+// {
+// p = s.getStr();
+// }
+// auto p2 = p;
+// (void)p2;
+// }
+
+// negative test - passing var into lambda
+void test8()
+{
+ int i;
+ auto l1 = [&]() { i = 1; };
+ (void)l1;
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/cppuhelper/source/servicemanager.cxx b/cppuhelper/source/servicemanager.cxx
index fe9ef7dbd2b4..070f1edd966b 100644
--- a/cppuhelper/source/servicemanager.cxx
+++ b/cppuhelper/source/servicemanager.cxx
@@ -1912,13 +1912,13 @@ void cppuhelper::ServiceManager::preloadImplementations() {
if (aModule.is() &&
!rEntry.second->environment.isEmpty())
{
- OUString aSymFactory;
oslGenericFunction fpFactory;
css::uno::Environment aTargetEnv;
css::uno::Reference<css::uno::XInterface> xFactory;
if(rEntry.second->constructorName.isEmpty())
{
+ OUString aSymFactory;
// expand full name component factory symbol
if (rEntry.second->prefix == "direct")
aSymFactory = rEntry.second->name.replace('.', '_') + "_" COMPONENT_GETFACTORY;
diff --git a/drawinglayer/source/processor2d/vclmetafileprocessor2d.cxx b/drawinglayer/source/processor2d/vclmetafileprocessor2d.cxx
index 27e21a881540..526f00518910 100644
--- a/drawinglayer/source/processor2d/vclmetafileprocessor2d.cxx
+++ b/drawinglayer/source/processor2d/vclmetafileprocessor2d.cxx
@@ -2202,7 +2202,6 @@ void VclMetafileProcessor2D::processTransparencePrimitive2D(
static_cast<sal_Int32>(ceil(aViewRange.getMaxY())));
const tools::Rectangle aRectPixel(mpOutputDevice->LogicToPixel(aRectLogic));
Size aSizePixel(aRectPixel.GetSize());
- const Point aEmptyPoint;
ScopedVclPtrInstance<VirtualDevice> aBufferDevice;
const sal_uInt32 nMaxQuadratPixels(500000);
const sal_uInt32 nViewVisibleArea(aSizePixel.getWidth() * aSizePixel.getHeight());
@@ -2258,6 +2257,7 @@ void VclMetafileProcessor2D::processTransparencePrimitive2D(
VclPixelProcessor2D aBufferProcessor(aViewInfo, *aBufferDevice);
// draw content using pixel renderer
+ const Point aEmptyPoint;
aBufferProcessor.process(rContent);
const Bitmap aBmContent(aBufferDevice->GetBitmap(aEmptyPoint, aSizePixel));
diff --git a/formula/source/core/api/FormulaCompiler.cxx b/formula/source/core/api/FormulaCompiler.cxx
index 3829ffe562df..bd1d1f082455 100644
--- a/formula/source/core/api/FormulaCompiler.cxx
+++ b/formula/source/core/api/FormulaCompiler.cxx
@@ -2211,9 +2211,9 @@ void FormulaCompiler::CreateStringFromTokenArray( OUStringBuffer& rBuffer )
else if ( FormulaGrammar::isOOXML( meGrammar ) )
{
// Scan token array for missing args and rewrite if present.
- MissingConventionOOXML aConv;
if (pArr->NeedsOoxmlRewrite())
{
+ MissingConventionOOXML aConv;
pArr = pArr->RewriteMissing( aConv );
maArrIterator = FormulaTokenArrayPlainIterator( *pArr );
}
diff --git a/formula/source/core/api/token.cxx b/formula/source/core/api/token.cxx
index 92f60254ec8f..37dd26979ced 100644
--- a/formula/source/core/api/token.cxx
+++ b/formula/source/core/api/token.cxx
@@ -1389,12 +1389,11 @@ FormulaTokenArray * FormulaTokenArray::RewriteMissing( const MissingConvention &
++nFn; // all following operations on _that_ function
pCtx[ nFn ].mpFunc = aIter.PeekPrevNoSpaces();
pCtx[ nFn ].mnCurArg = 0;
- OpCode eOp;
if (rConv.isPODF() && pCtx[ nFn ].mpFunc && pCtx[ nFn ].mpFunc->GetOpCode() == ocAddress)
pOcas[ nOcas++ ] = nFn; // entering ADDRESS() if PODF
else if ((rConv.isODFF() || rConv.isOOXML()) && pCtx[ nFn ].mpFunc)
{
- eOp = pCtx[ nFn ].mpFunc->GetOpCode();
+ OpCode eOp = pCtx[ nFn ].mpFunc->GetOpCode();
if (eOp == ocDBCount || eOp == ocDBCount2)
pOcds[ nOcds++ ] = nFn; // entering DCOUNT() or DCOUNTA() if ODFF or OOXML
}
diff --git a/i18nlangtag/source/languagetag/languagetag.cxx b/i18nlangtag/source/languagetag/languagetag.cxx
index 23bd70300135..0c52a2b3cf45 100644
--- a/i18nlangtag/source/languagetag/languagetag.cxx
+++ b/i18nlangtag/source/languagetag/languagetag.cxx
@@ -2241,12 +2241,11 @@ LanguageTag & LanguageTag::makeFallback()
else if (maBcp47 == "en-GB-oxendict")
aVec.emplace_back("en-GB-oed");
- OUString aScript;
OUString aVariants( getVariants());
OUString aTmp;
if (hasScript())
{
- aScript = getScript();
+ OUString aScript = getScript();
bool bHaveLanguageScriptVariant = false;
if (!aCountry.isEmpty())
{
diff --git a/i18npool/qa/cppunit/test_breakiterator.cxx b/i18npool/qa/cppunit/test_breakiterator.cxx
index 36f145e79958..864387415124 100644
--- a/i18npool/qa/cppunit/test_breakiterator.cxx
+++ b/i18npool/qa/cppunit/test_breakiterator.cxx
@@ -123,12 +123,11 @@ void TestBreakIterator::testLineBreaking()
//See https://bz.apache.org/ooo/show_bug.cgi?id=19716
{
- OUString aTest("aaa]aaa");
-
aLocale.Language = "en";
aLocale.Country = "US";
{
+ OUString aTest("aaa]aaa");
//Here we want the line break to move the whole lot to the next line
i18n::LineBreakResults aResult = m_xBreak->getLineBreak(aTest, aTest.getLength()-2, aLocale, 0,
aHyphOptions, aUserOptions);
@@ -1030,13 +1029,12 @@ void TestBreakIterator::testChinese()
lang::Locale aLocale;
aLocale.Language = "zh";
aLocale.Country = "CN";
- i18n::Boundary aBounds;
{
const sal_Unicode CHINESE[] = { 0x6A35, 0x6A30, 0x69FE, 0x8919, 0xD867, 0xDEDB };
OUString aTest(CHINESE, SAL_N_ELEMENTS(CHINESE));
- aBounds = m_xBreak->getWordBoundary(aTest, 4, aLocale,
+ i18n::Boundary aBounds = m_xBreak->getWordBoundary(aTest, 4, aLocale,
i18n::WordType::DICTIONARY_WORD, true);
CPPUNIT_ASSERT(aBounds.startPos == 4 && aBounds.endPos == 6);
}
diff --git a/idlc/source/idlc.cxx b/idlc/source/idlc.cxx
index 5984ca759448..c1d09b4c1b2e 100644
--- a/idlc/source/idlc.cxx
+++ b/idlc/source/idlc.cxx
@@ -273,8 +273,8 @@ OUString Idlc::processDocumentation()
static void lcl_writeString(::osl::File & rFile, ::osl::FileBase::RC & o_rRC,
OString const& rString)
{
- sal_uInt64 nWritten(0);
if (::osl::FileBase::E_None == o_rRC) {
+ sal_uInt64 nWritten(0);
o_rRC = rFile.write(rString.getStr(), rString.getLength(), nWritten);
if (static_cast<sal_uInt64>(rString.getLength()) != nWritten) {
o_rRC = ::osl::FileBase::E_INVAL; //?
diff --git a/idlc/source/options.cxx b/idlc/source/options.cxx
index 0ab0a9cc32d2..1b74a2367578 100644
--- a/idlc/source/options.cxx
+++ b/idlc/source/options.cxx
@@ -168,9 +168,9 @@ bool Options::checkCommandFile (std::vector< std::string > & rArgs, char const *
bool Options::badOption(char const * reason, std::string const & rArg)
{
- OStringBuffer message;
if (reason != nullptr)
{
+ OStringBuffer message;
message.append(reason); message.append(" option '"); message.append(rArg.c_str()); message.append("'");
throw IllegalArgument(message.makeStringAndClear());
}
diff --git a/l10ntools/source/cfgmerge.cxx b/l10ntools/source/cfgmerge.cxx
index e02e497d5d21..5715bd1c1d14 100644
--- a/l10ntools/source/cfgmerge.cxx
+++ b/l10ntools/source/cfgmerge.cxx
@@ -164,7 +164,6 @@ void CfgParser::ExecuteAnalyzedToken( int nToken, char *pToken )
sLastWhitespace += sToken;
OString sTokenName;
- OString sTokenId;
bool bOutput = true;
@@ -222,6 +221,7 @@ void CfgParser::ExecuteAnalyzedToken( int nToken, char *pToken )
}
break;
}
+ OString sTokenId;
if ( !sSearch.isEmpty())
{
OString sTemp = sToken.copy( sToken.indexOf( sSearch ));
diff --git a/l10ntools/source/helpmerge.cxx b/l10ntools/source/helpmerge.cxx
index abab8a5a03a1..10a9de923175 100644
--- a/l10ntools/source/helpmerge.cxx
+++ b/l10ntools/source/helpmerge.cxx
@@ -192,7 +192,6 @@ void HelpParser::MergeSingleFile( XMLFile* file , MergeDataFile* pMergeDataFile
void HelpParser::ProcessHelp( LangHashMap* aLangHM , const OString& sCur , ResData *pResData , MergeDataFile* pMergeDataFile ){
XMLElement* pXMLElement = nullptr;
- MergeEntrys *pEntrys = nullptr;
if( sCur.equalsIgnoreAsciiCase("en-US") )
return;
@@ -228,7 +227,7 @@ void HelpParser::ProcessHelp( LangHashMap* aLangHM , const OString& sCur , ResDa
}
else if( pMergeDataFile )
{
- pEntrys = pMergeDataFile->GetMergeEntrys( pResData );
+ MergeEntrys *pEntrys = pMergeDataFile->GetMergeEntrys( pResData );
if( pEntrys != nullptr)
{
pEntrys->GetText( sNewText, sCur, true );
diff --git a/lingucomponent/source/hyphenator/hyphen/hyphenimp.cxx b/lingucomponent/source/hyphenator/hyphen/hyphenimp.cxx
index 988306757a96..f60e2c7b83fe 100644
--- a/lingucomponent/source/hyphenator/hyphen/hyphenimp.cxx
+++ b/lingucomponent/source/hyphenator/hyphen/hyphenimp.cxx
@@ -258,7 +258,6 @@ Reference< XHyphenatedWord > SAL_CALL Hyphenator::hyphenate( const OUString& aWo
sal_Int16 minLen = rHelper.GetMinWordLength();
bool bNoHyphenateCaps = rHelper.IsNoHyphenateCaps();
- HyphenDict *dict = nullptr;
rtl_TextEncoding eEnc = RTL_TEXTENCODING_DONTKNOW;
Reference< XHyphenatedWord > xRes;
@@ -285,7 +284,7 @@ Reference< XHyphenatedWord > SAL_CALL Hyphenator::hyphenate( const OUString& aWo
}
// otherwise hyphenate the word with that dictionary
- dict = mvDicts[k].aPtr;
+ HyphenDict *dict = mvDicts[k].aPtr;
eEnc = mvDicts[k].eEnc;
CharClass * pCC = mvDicts[k].apCC.get();
diff --git a/linguistic/source/hyphdsp.cxx b/linguistic/source/hyphdsp.cxx
index 4ce8d4aa099d..be33885880d3 100644
--- a/linguistic/source/hyphdsp.cxx
+++ b/linguistic/source/hyphdsp.cxx
@@ -88,7 +88,6 @@ Reference<XHyphenatedWord> HyphenatorDispatcher::buildHyphWord(
if (nTextLen > 0 && aText[ nTextLen - 1 ] != '=' && aText[ nTextLen - 1 ] != '[')
{
sal_Int16 nHyphenationPos = -1;
- sal_Int32 nHyphenPos = -1;
sal_Int16 nOrigHyphPos = -1;
OUStringBuffer aTmp( nTextLen );
@@ -149,6 +148,7 @@ Reference<XHyphenatedWord> HyphenatorDispatcher::buildHyphWord(
}
}
#endif
+ sal_Int32 nHyphenPos = -1;
if (aText[ nOrigHyphPos ] == '[') // alternative hyphenation
{
sal_Int16 split = 0;
diff --git a/sc/qa/unit/ucalc_sort.cxx b/sc/qa/unit/ucalc_sort.cxx
index 64acbcc1eab0..829fa82b56e0 100644
--- a/sc/qa/unit/ucalc_sort.cxx
+++ b/sc/qa/unit/ucalc_sort.cxx
@@ -41,7 +41,6 @@ void Test::testSort()
// We need a drawing layer in order to create caption objects.
m_pDoc->InitDrawLayer(&getDocShell());
- ScRange aDataRange;
ScAddress aPos(0,0,0);
{
const char* aData[][2] = {
@@ -52,7 +51,7 @@ void Test::testSort()
};
clearRange(m_pDoc, ScRange(0, 0, 0, 1, SAL_N_ELEMENTS(aData), 0));
- aDataRange = insertRangeData(m_pDoc, aPos, aData, SAL_N_ELEMENTS(aData));
+ ScRange aDataRange = insertRangeData(m_pDoc, aPos, aData, SAL_N_ELEMENTS(aData));
CPPUNIT_ASSERT_EQUAL_MESSAGE("failed to insert range data at correct position", aPos, aDataRange.aStart);
}
@@ -80,6 +79,7 @@ void Test::testSort()
pNote = m_pDoc->GetNote(1, 0, 0);
CPPUNIT_ASSERT(pNote);
+ ScRange aDataRange;
clearRange(m_pDoc, ScRange(0, 0, 0, 1, 9, 0)); // Clear A1:B10.
{
// 0 = empty cell
diff --git a/slideshow/source/engine/shapes/drawinglayeranimation.cxx b/slideshow/source/engine/shapes/drawinglayeranimation.cxx
index 371f7058d1d6..0b37071fb644 100644
--- a/slideshow/source/engine/shapes/drawinglayeranimation.cxx
+++ b/slideshow/source/engine/shapes/drawinglayeranimation.cxx
@@ -305,7 +305,7 @@ double ActivityImpl::GetMixerState( sal_uInt32 nTime )
sal_uInt32 ActivityImpl::GetStepWidthLogic() const
{
// #i69847# Assuming higher DPI
- sal_uInt32 const PIXEL_TO_LOGIC = 30;
+ constexpr sal_uInt32 PIXEL_TO_LOGIC = 30;
sal_uInt32 nRetval(0);
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index a8cc4ad731af..b707954c3319 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -58,6 +58,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
compilerplugins/clang/test/passparamsbyref \
compilerplugins/clang/test/passstuffbyref \
compilerplugins/clang/test/pointerbool \
+ compilerplugins/clang/test/reducevarscope \
compilerplugins/clang/test/redundantcast \
compilerplugins/clang/test/redundantfcast \
compilerplugins/clang/test/redundantinline \
diff --git a/sot/source/sdstor/stgavl.cxx b/sot/source/sdstor/stgavl.cxx
index 0c78054045f0..98a86f3edb96 100644
--- a/sot/source/sdstor/stgavl.cxx
+++ b/sot/source/sdstor/stgavl.cxx
@@ -284,7 +284,7 @@ void StgAvlNode::StgEnum( short& n )
bool StgAvlNode::Insert( StgAvlNode** pRoot, StgAvlNode* pIns )
{
- StgAvlNode* pPivot, *pHeavy, *pNewRoot, *pParent, *pPrev;
+ StgAvlNode* pPivot, *pHeavy, *pParent, *pPrev;
if ( !pRoot )
return false;
@@ -310,6 +310,7 @@ bool StgAvlNode::Insert( StgAvlNode** pRoot, StgAvlNode* pIns )
short nDelta = pPivot->Adjust( &pHeavy, pIns );
if( pPivot->m_nBalance >= 2 || pPivot->m_nBalance <= -2 )
{
+ StgAvlNode* pNewRoot;
pHeavy = ( nDelta < 0 ) ? pPivot->m_pRight : pPivot->m_pLeft;
// left imbalance
if( nDelta > 0 )
diff --git a/stoc/source/defaultregistry/defaultregistry.cxx b/stoc/source/defaultregistry/defaultregistry.cxx
index 92adf4642938..ff4c2483e34b 100644
--- a/stoc/source/defaultregistry/defaultregistry.cxx
+++ b/stoc/source/defaultregistry/defaultregistry.cxx
@@ -807,13 +807,12 @@ sal_Bool SAL_CALL NestedKeyImpl::createLink( const OUString& aLinkName, const OU
throw InvalidRegistryException();
}
- OUString linkName;
OUString resolvedName;
sal_Int32 lastIndex = aLinkName.lastIndexOf('/');
if ( lastIndex > 0 )
{
- linkName = aLinkName.copy(0, lastIndex);
+ OUString linkName = aLinkName.copy(0, lastIndex);
resolvedName = computeName(linkName);
@@ -862,13 +861,12 @@ void SAL_CALL NestedKeyImpl::deleteLink( const OUString& rLinkName )
throw InvalidRegistryException();
}
- OUString linkName;
OUString resolvedName;
sal_Int32 lastIndex = rLinkName.lastIndexOf('/');
if ( lastIndex > 0 )
{
- linkName = rLinkName.copy(0, lastIndex);
+ OUString linkName = rLinkName.copy(0, lastIndex);
resolvedName = computeName(linkName);
@@ -905,13 +903,12 @@ OUString SAL_CALL NestedKeyImpl::getLinkTarget( const OUString& rLinkName )
throw InvalidRegistryException();
}
- OUString linkName;
OUString resolvedName;
sal_Int32 lastIndex = rLinkName.lastIndexOf('/');
if ( lastIndex > 0 )
{
- linkName = rLinkName.copy(0, lastIndex);
+ OUString linkName = rLinkName.copy(0, lastIndex);
resolvedName = computeName(linkName);
diff --git a/sw/source/core/txtnode/fntcache.cxx b/sw/source/core/txtnode/fntcache.cxx
index a1430890f010..8f1aca1a64b4 100644
--- a/sw/source/core/txtnode/fntcache.cxx
+++ b/sw/source/core/txtnode/fntcache.cxx
@@ -2613,7 +2613,6 @@ bool SwDrawTextInfo::ApplyAutoColor( vcl::Font* pFont )
std::optional<Color> pCol;
if (GetFont())
pCol = GetFont()->GetBackColor();
- Color aColor;
if( ! pCol || COL_TRANSPARENT == *pCol )
{
const SvxBrushItem* pItem;
@@ -2631,8 +2630,7 @@ bool SwDrawTextInfo::ApplyAutoColor( vcl::Font* pFont )
if (aFillAttributes && aFillAttributes->isUsed())
{
// First see if fill attributes provide a color.
- aColor = Color(aFillAttributes->getAverageColor(aGlobalRetoucheColor.getBColor()));
- pCol = aColor;
+ pCol = Color(aFillAttributes->getAverageColor(aGlobalRetoucheColor.getBColor()));
}
// If not, then fall back to the old brush item.
diff --git a/writerfilter/source/dmapper/PropertyMap.cxx b/writerfilter/source/dmapper/PropertyMap.cxx
index c4dadd35dc74..acdcede9d6f9 100644
--- a/writerfilter/source/dmapper/PropertyMap.cxx
+++ b/writerfilter/source/dmapper/PropertyMap.cxx
@@ -1389,10 +1389,10 @@ void SectionPropertyMap::CloseSectionGroup( DomainMapper_Impl& rDM_Impl )
uno::Reference< text::XTextRange > xRange;
aFramedRedlines[i] >>= xRange;
uno::Reference< beans::XPropertySet > xRangeProperties;
- OUString sTableName;
- OUString sCellName;
if ( xRange.is() )
{
+ OUString sTableName;
+ OUString sCellName;
xRangeProperties.set( xRange, uno::UNO_QUERY_THROW );
if (xRangeProperties->getPropertySetInfo()->hasPropertyByName("TextTable"))
{
diff --git a/xmlhelp/source/cxxhelp/provider/databases.cxx b/xmlhelp/source/cxxhelp/provider/databases.cxx
index 5c42fa2349e5..5353f6e82ac3 100644
--- a/xmlhelp/source/cxxhelp/provider/databases.cxx
+++ b/xmlhelp/source/cxxhelp/provider/databases.cxx
@@ -781,9 +781,9 @@ Reference< XHierarchicalNameAccess > Databases::jarFile( const OUString& jar,
if( ! it->second.is() )
{
- OUString zipFile;
try
{
+ OUString zipFile;
// Extension jar file? Search for ?
sal_Int32 nQuestionMark1 = jar.indexOf( '?' );
sal_Int32 nQuestionMark2 = jar.lastIndexOf( '?' );
More information about the Libreoffice-commits
mailing list