canvas/source/factory/cf_service.cxx | 24 canvas/source/tools/canvastools.cxx | 5 chart2/source/controller/chartapiwrapper/WrappedAxisAndGridExistenceProperties.cxx | 4 chart2/source/tools/ObjectIdentifier.cxx | 3 comphelper/source/misc/threadpool.cxx | 9 compilerplugins/clang/buriedassign.cxx | 558 ++++++++-- compilerplugins/clang/test/buriedassign.cxx | 43 connectivity/source/commontools/FDatabaseMetaDataResultSetMetaData.cxx | 26 connectivity/source/commontools/dbtools2.cxx | 7 connectivity/source/commontools/parameters.cxx | 2 connectivity/source/drivers/dbase/DIndexIter.cxx | 29 connectivity/source/drivers/dbase/dindexnode.cxx | 7 connectivity/source/drivers/firebird/Util.cxx | 10 connectivity/source/drivers/mysql_jdbc/YTable.cxx | 14 connectivity/source/drivers/odbc/OFunctions.cxx | 3 connectivity/source/drivers/odbc/OResultSetMetaData.cxx | 3 connectivity/source/drivers/postgresql/pq_tools.cxx | 14 cppcanvas/source/mtfrenderer/implrenderer.cxx | 6 cppu/source/typelib/static_types.cxx | 9 cppu/source/typelib/typelib.cxx | 59 - cppu/source/uno/copy.hxx | 8 cppu/source/uno/lbenv.cxx | 7 cppu/source/uno/sequence.cxx | 4 cppuhelper/source/tdmgr.cxx | 16 cui/source/dialogs/insdlg.cxx | 6 cui/source/tabpages/backgrnd.cxx | 15 26 files changed, 654 insertions(+), 237 deletions(-)
New commits: commit 11785217594d863efb518aa8b8f2910cdcb9c59d Author: Noel Grandin <noel.gran...@collabora.co.uk> AuthorDate: Tue Apr 14 14:55:22 2020 +0200 Commit: Noel Grandin <noel.gran...@collabora.co.uk> CommitDate: Tue Apr 14 16:35:38 2020 +0200 loplugin:buriedassign in c* Change-Id: Id14fed7e5c0f588ad3c927f12251432d12c1a7c8 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/92190 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> diff --git a/canvas/source/factory/cf_service.cxx b/canvas/source/factory/cf_service.cxx index 86dbc2f12aeb..936c99885f20 100644 --- a/canvas/source/factory/cf_service.cxx +++ b/canvas/source/factory/cf_service.cxx @@ -321,13 +321,13 @@ Reference<XInterface> CanvasFactory::lookupAndUse( // try to reuse last working implementation for given service name const CacheVector::iterator aEnd(m_aCachedImplementations.end()); - CacheVector::iterator aMatch; - if( (aMatch=std::find_if( + auto aMatch = std::find_if( m_aCachedImplementations.begin(), aEnd, [&serviceName](CachePair const& cp) { return serviceName == cp.first; } - )) != aEnd) { + ); + if( aMatch != aEnd ) { Reference<XInterface> xCanvas( use( aMatch->second, args, xContext ) ); if(xCanvas.is()) return xCanvas; @@ -335,35 +335,35 @@ Reference<XInterface> CanvasFactory::lookupAndUse( // lookup in available service list const AvailVector::const_iterator aAvailEnd(m_aAvailableImplementations.end()); - AvailVector::const_iterator aAvailImplsMatch; - if( (aAvailImplsMatch=std::find_if( + auto aAvailImplsMatch = std::find_if( m_aAvailableImplementations.begin(), aAvailEnd, [&serviceName](AvailPair const& ap) { return serviceName == ap.first; } - )) == aAvailEnd ) { + ); + if( aAvailImplsMatch == aAvailEnd ) { return Reference<XInterface>(); } const AvailVector::const_iterator aAAEnd(m_aAAImplementations.end()); - AvailVector::const_iterator aAAImplsMatch; - if( (aAAImplsMatch=std::find_if( + auto aAAImplsMatch = std::find_if( m_aAAImplementations.begin(), aAAEnd, [&serviceName](AvailPair const& ap) { return serviceName == ap.first; } - )) == aAAEnd) { + ); + if( aAAImplsMatch == aAAEnd ) { return Reference<XInterface>(); } const AvailVector::const_iterator aAccelEnd(m_aAcceleratedImplementations.end()); - AvailVector::const_iterator aAccelImplsMatch; - if( (aAccelImplsMatch=std::find_if( + auto aAccelImplsMatch = std::find_if( m_aAcceleratedImplementations.begin(), aAccelEnd, [&serviceName](AvailPair const& ap) { return serviceName == ap.first; } - )) == aAccelEnd ) { + ); + if( aAccelImplsMatch == aAccelEnd ) { return Reference<XInterface>(); } diff --git a/canvas/source/tools/canvastools.cxx b/canvas/source/tools/canvastools.cxx index 957a7bb90b0a..30f92f484967 100644 --- a/canvas/source/tools/canvastools.cxx +++ b/canvas/source/tools/canvastools.cxx @@ -911,7 +911,10 @@ namespace canvas::tools const ::basegfx::B2DHomMatrix& i_transformation ) { if( i_srcRect.isEmpty() ) - return o_transform=i_transformation; + { + o_transform = i_transformation; + return o_transform; + } // transform by given transformation ::basegfx::B2DRectangle aTransformedRect; diff --git a/chart2/source/controller/chartapiwrapper/WrappedAxisAndGridExistenceProperties.cxx b/chart2/source/controller/chartapiwrapper/WrappedAxisAndGridExistenceProperties.cxx index 5b69f2292d9c..a4239d10ce22 100644 --- a/chart2/source/controller/chartapiwrapper/WrappedAxisAndGridExistenceProperties.cxx +++ b/chart2/source/controller/chartapiwrapper/WrappedAxisAndGridExistenceProperties.cxx @@ -340,14 +340,14 @@ WrappedAxisLabelExistenceProperty::WrappedAxisLabelExistenceProperty(bool bMain, switch( m_nDimensionIndex ) { case 0: - m_bMain ? m_aOuterName = "HasXAxisDescription" : m_aOuterName = "HasSecondaryXAxisDescription"; + m_aOuterName = m_bMain ? OUStringLiteral("HasXAxisDescription") : OUStringLiteral("HasSecondaryXAxisDescription"); break; case 2: OSL_ENSURE(m_bMain,"there is no description available for a secondary z axis"); m_aOuterName = "HasZAxisDescription"; break; default: - m_bMain ? m_aOuterName = "HasYAxisDescription" : m_aOuterName = "HasSecondaryYAxisDescription"; + m_aOuterName = m_bMain ? OUStringLiteral("HasYAxisDescription") : OUStringLiteral("HasSecondaryYAxisDescription"); break; } } diff --git a/chart2/source/tools/ObjectIdentifier.cxx b/chart2/source/tools/ObjectIdentifier.cxx index c0fc94ec1326..b4c6632fff3c 100644 --- a/chart2/source/tools/ObjectIdentifier.cxx +++ b/chart2/source/tools/ObjectIdentifier.cxx @@ -1127,8 +1127,7 @@ OUString ObjectIdentifier::createSeriesSubObjectStub( ObjectType eSubObjectType OUString ObjectIdentifier::createPointCID( const OUString& rPointCID_Stub, sal_Int32 nIndex ) { - OUString aRet(rPointCID_Stub); - return aRet+=OUString::number( nIndex ); + return rPointCID_Stub + OUString::number( nIndex ); } OUString ObjectIdentifier::getParticleID( const OUString& rCID ) diff --git a/comphelper/source/misc/threadpool.cxx b/comphelper/source/misc/threadpool.cxx index 9dfbff0e2900..f93400d96f9f 100644 --- a/comphelper/source/misc/threadpool.cxx +++ b/comphelper/source/misc/threadpool.cxx @@ -240,10 +240,13 @@ void ThreadPool::waitUntilDone(const std::shared_ptr<ThreadTaskTag>& rTag, bool if( maWorkers.empty() ) { // no threads at all -> execute the work in-line - std::unique_ptr<ThreadTask> pTask; - while (!rTag->isDone() && - ( pTask = popWorkLocked(aGuard, false) ) ) + while (!rTag->isDone()) + { + std::unique_ptr<ThreadTask> pTask = popWorkLocked(aGuard, false); + if (!pTask) + break; pTask->exec(); + } } } diff --git a/compilerplugins/clang/buriedassign.cxx b/compilerplugins/clang/buriedassign.cxx index 8d0e6cc79e59..64043b8aed39 100644 --- a/compilerplugins/clang/buriedassign.cxx +++ b/compilerplugins/clang/buriedassign.cxx @@ -10,7 +10,6 @@ #include <cassert> #include <string> #include <iostream> -#include <unordered_map> #include <unordered_set> #include "plugin.hxx" @@ -18,15 +17,11 @@ #include "clang/AST/CXXInheritance.h" #include "clang/AST/StmtVisitor.h" -/** - TODO deal with C++ operator overload assign -*/ +// This checker aims to pull buried assignments out of complex expressions, +// where they are quite hard to notice amidst the other conditional logic. namespace { -//static bool startswith(const std::string& rStr, const char* pSubStr) { -// return rStr.compare(0, strlen(pSubStr), pSubStr) == 0; -//} class BuriedAssign : public loplugin::FilteringPlugin<BuriedAssign> { public: @@ -39,25 +34,187 @@ public: { std::string fn(handler.getMainFileName()); loplugin::normalizeDotDotInFilePath(fn); - // getParentStmt appears not to be working very well here - if (fn == SRCDIR "/stoc/source/inspect/introspection.cxx" - || fn == SRCDIR "/stoc/source/corereflection/criface.cxx") + + // code where I don't have a better alternative + if (fn == SRCDIR "/sal/osl/unx/profile.cxx") + return; + if (fn == SRCDIR "/sal/rtl/uri.cxx") + return; + if (fn == SRCDIR "/sal/osl/unx/process.cxx") + return; + if (fn == SRCDIR "/sal/rtl/bootstrap.cxx") + return; + if (fn == SRCDIR "/i18npool/source/textconversion/genconv_dict.cxx") + return; + if (fn == SRCDIR "/soltools/cpp/_macro.c") + return; + if (fn == SRCDIR "/stoc/source/inspect/introspection.cxx") + return; + if (fn == SRCDIR "/tools/source/fsys/urlobj.cxx") + return; + if (fn == SRCDIR "/sax/source/tools/fastserializer.cxx") + return; + if (fn == SRCDIR "/svl/source/crypto/cryptosign.cxx") + return; + if (fn == SRCDIR "/svl/source/numbers/zforfind.cxx") + return; + if (fn == SRCDIR "/svl/source/numbers/zformat.cxx") + return; + if (fn == SRCDIR "/svl/source/numbers/zforscan.cxx") + return; + if (fn == SRCDIR "/svl/source/numbers/zforlist.cxx") return; - // calling an acquire via function pointer - if (fn == SRCDIR "/cppu/source/uno/lbenv.cxx" - || fn == SRCDIR "cppu/source/typelib/static_types.cxx") + if (fn == SRCDIR "/vcl/source/window/debugevent.cxx") + return; + if (fn == SRCDIR "/vcl/source/control/scrbar.cxx") + return; + if (fn == SRCDIR "/vcl/source/gdi/bitmap3.cxx") return; - // false+, not sure why if (fn == SRCDIR "/vcl/source/window/menu.cxx") return; + if (fn == SRCDIR "/vcl/source/fontsubset/sft.cxx") + return; + if (fn == SRCDIR "/vcl/unx/generic/print/prtsetup.cxx") + return; + if (fn == SRCDIR "/svtools/source/brwbox/brwbox1.cxx") + return; + if (fn == SRCDIR "/svtools/source/control/valueset.cxx") + return; + if (fn == SRCDIR "/basic/source/runtime/iosys.cxx") + return; + if (fn == SRCDIR "/basic/source/runtime/runtime.cxx") + return; + if (fn == SRCDIR "/basic/source/sbx/sbxvalue.cxx") + return; + if (fn == SRCDIR "/basic/source/sbx/sbxvalue.cxx") + return; + if (fn == SRCDIR "/sfx2/source/dialog/templdlg.cxx") + return; + if (fn == SRCDIR "/sfx2/source/view/viewfrm.cxx") + return; + if (fn == SRCDIR "/connectivity/source/commontools/dbtools.cxx") + return; + if (fn == SRCDIR "/xmloff/source/style/xmlnumfi.cxx") + return; + if (fn == SRCDIR "/xmloff/source/style/xmlnumfe .cxx") + return; + if (fn == SRCDIR "/editeng/source/items/textitem.cxx") + return; + if (fn == SRCDIR "/editeng/source/rtf/rtfitem.cxx") + return; + if (fn == SRCDIR "/editeng/source/rtf/svxrtf.cxx") + return; + if (fn == SRCDIR "/editeng/source/misc/svxacorr.cxx") + return; + if (fn == SRCDIR "/svx/source/items/numfmtsh.cxx") + return; + if (fn == SRCDIR "/svx/source/dialog/hdft.cxx") + return; + if (fn == SRCDIR "/cui/source/dialogs/insdlg.cxx") + return; + if (fn == SRCDIR "/cui/source/tabpages/paragrph.cxx") + return; + if (fn == SRCDIR "/cui/source/tabpages/page.cxx") + return; + if (fn == SRCDIR "/cui/source/tabpages/border.cxx") + return; + if (fn == SRCDIR "/cui/source/tabpages/chardlg.cxx") + return; + if (fn == SRCDIR "/cui/source/tabpages/numpages.cxx") + return; + if (fn == SRCDIR "/cui/source/dialogs/SpellDialog.cxx") + return; + if (fn == SRCDIR "/oox/source/drawingml/diagram/diagramlayoutatoms.cxx") + return; + if (fn == SRCDIR "/dbaccess/source/core/dataaccess/intercept.cxx") + return; + if (fn == SRCDIR "/writerfilter/source/dmapper/DomainMapper.cxx") + return; + if (fn == SRCDIR "/writerfilter/source/dmapper/DomainMapper_Impl.cxx") + return; + if (fn == SRCDIR "/lotuswordpro/source/filter/lwptablelayout.cxx") + return; + if (fn == SRCDIR "/i18npool/source/characterclassification/cclass_unicode_parser.cxx") + return; + if (fn == SRCDIR "/sd/source/filter/eppt/pptx-animations.cxx") + return; + if (fn == SRCDIR "/sc/source/core/tool/address.cxx") + return; + if (fn == SRCDIR "/sc/source/core/tool/interpr1.cxx") + return; + if (fn == SRCDIR "/sc/source/core/tool/interpr4.cxx") + return; + if (fn == SRCDIR "/sc/source/core/tool/interpr5.cxx") + return; + if (fn == SRCDIR "/sc/source/core/tool/compiler.cxx") + return; + if (fn == SRCDIR "/sc/source/core/data/table4.cxx") + return; + if (fn == SRCDIR "/sc/source/ui/drawfunc/fudraw.cxx") + return; + if (fn == SRCDIR "/sc/source/filter/xml/xmlcelli.cxx") + return; + if (fn == SRCDIR "/sc/source/ui/miscdlgs/crnrdlg.cxx") + return; + if (fn == SRCDIR "/sc/source/ui/app/inputwin.cxx") + return; + if (fn == SRCDIR "/sc/source/ui/view/viewfun2.cxx") + return; + if (fn == SRCDIR "/sc/source/ui/unoobj/docuno.cxx") + return; + if (fn == SRCDIR "/sc/source/ui/view/gridwin.cxx") + return; + if (fn == SRCDIR "/sw/source/core/crsr/callnk.cxx") + return; + if (fn == SRCDIR "/sw/source/core/crsr/findtxt.cxx") + return; + if (fn == SRCDIR "/sw/source/core/crsr/crsrsh.cxx") + return; + if (fn == SRCDIR "/sw/source/core/crsr/crstrvl.cxx") + return; + if (fn == SRCDIR "/sw/source/core/doc/doccomp.cxx") + return; + if (fn == SRCDIR "/sw/source/core/doc/docedt.cxx") + return; + if (fn == SRCDIR "/sw/source/core/doc/docfly.cxx") + return; + if (fn == SRCDIR "/sw/source/core/doc/DocumentRedlineManager.cxx") + return; + if (fn == SRCDIR "/sw/source/core/doc/notxtfrm.cxx") + return; + // the point at which I gave up on sw/ because it just does it EVERYWHER + if (loplugin::hasPathnamePrefix(fn, SRCDIR "/sw/")) + return; + if (fn == SRCDIR "/starmath/source/mathtype.cxx") + return; + if (fn == SRCDIR "/starmath/source/mathmlexport.cxx") + return; + if (fn == SRCDIR "/starmath/source/view.cxx") + return; + if (fn == SRCDIR "/xmlhelp/source/treeview/tvread.cxx") + return; TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); } bool VisitBinaryOperator(BinaryOperator const*); bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr const*); + bool VisitCompoundStmt(CompoundStmt const*); + bool VisitIfStmt(IfStmt const*); + bool VisitLabelStmt(LabelStmt const*); + bool VisitForStmt(ForStmt const*); + bool VisitCXXForRangeStmt(CXXForRangeStmt const*); + bool VisitWhileStmt(WhileStmt const*); + bool VisitDoStmt(DoStmt const*); + bool VisitCaseStmt(CaseStmt const*); + bool VisitDefaultStmt(DefaultStmt const*); + bool VisitVarDecl(VarDecl const*); + bool VisitCXXFoldExpr(CXXFoldExpr const*); private: - void checkExpr(Expr const*); + void MarkIfAssignment(Stmt const*); + void MarkConditionForControlLoops(Expr const*); + + std::unordered_set<const Stmt*> m_handled; }; static bool isAssignmentOp(clang::BinaryOperatorKind op) @@ -80,15 +237,67 @@ static bool isAssignmentOp(clang::OverloadedOperatorKind Opc) || Opc == OO_AmpEqual || Opc == OO_CaretEqual || Opc == OO_PipeEqual; } +static bool isComparisonOp(clang::OverloadedOperatorKind op) +{ + return op == OO_Less || op == OO_Greater || op == OO_LessEqual || op == OO_GreaterEqual + || op == OO_EqualEqual || op == OO_ExclaimEqual; +} + +static const Expr* IgnoreImplicitAndConversionOperator(const Expr* expr) +{ + expr = compat::IgnoreImplicit(expr); + if (auto memberCall = dyn_cast<CXXMemberCallExpr>(expr)) + { + if (auto conversionDecl = dyn_cast_or_null<CXXConversionDecl>(memberCall->getMethodDecl())) + { + if (!conversionDecl->isExplicit()) + expr = compat::IgnoreImplicit(memberCall->getImplicitObjectArgument()); + } + } + return expr; +} + bool BuriedAssign::VisitBinaryOperator(BinaryOperator const* binaryOp) { if (ignoreLocation(binaryOp)) return true; - + if (compat::getBeginLoc(binaryOp).isMacroID()) + return true; if (!isAssignmentOp(binaryOp->getOpcode())) return true; + auto expr = IgnoreImplicitAndConversionOperator(binaryOp->getRHS()); + if (auto rhs = dyn_cast<BinaryOperator>(expr)) + { + // Ignore chained assignment. + // TODO limit this to only ordinary assignment + if (isAssignmentOp(rhs->getOpcode())) + m_handled.insert(rhs); + } + else if (auto rhs = dyn_cast<CXXOperatorCallExpr>(expr)) + { + // Ignore chained assignment. + // TODO limit this to only ordinary assignment + if (isAssignmentOp(rhs->getOperator())) + m_handled.insert(rhs); + } + else if (auto cxxConstruct = dyn_cast<CXXConstructExpr>(expr)) + { + if (cxxConstruct->getNumArgs() == 1) + MarkIfAssignment(cxxConstruct->getArg(0)); + } + if (!m_handled.insert(binaryOp).second) + return true; + + // assignment in constructor + StringRef aFileName = getFilenameOfLocation( + compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(binaryOp))); + if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/include/comphelper/flagguard.hxx")) + return true; - checkExpr(binaryOp); + report(DiagnosticsEngine::Warning, "buried assignment, rather put on own line", + compat::getBeginLoc(binaryOp)) + << binaryOp->getSourceRange(); + //getParentStmt(getParentStmt(getParentStmt(binaryOp)))->dump(); return true; } @@ -96,127 +305,256 @@ bool BuriedAssign::VisitCXXOperatorCallExpr(CXXOperatorCallExpr const* cxxOper) { if (ignoreLocation(cxxOper)) return true; + if (compat::getBeginLoc(cxxOper).isMacroID()) + return true; if (!isAssignmentOp(cxxOper->getOperator())) return true; - checkExpr(cxxOper); + auto expr = IgnoreImplicitAndConversionOperator(cxxOper->getArg(1)); + if (auto rhs = dyn_cast<BinaryOperator>(expr)) + { + // Ignore chained assignment. + // TODO limit this to only ordinary assignment + if (isAssignmentOp(rhs->getOpcode())) + m_handled.insert(rhs); + } + else if (auto rhs = dyn_cast<CXXOperatorCallExpr>(expr)) + { + // Ignore chained assignment. + // TODO limit this to only ordinary assignment + if (isAssignmentOp(rhs->getOperator())) + m_handled.insert(rhs); + } + else if (auto cxxConstruct = dyn_cast<CXXConstructExpr>(expr)) + { + if (cxxConstruct->getNumArgs() == 1) + MarkIfAssignment(cxxConstruct->getArg(0)); + } + if (!m_handled.insert(cxxOper).second) + return true; + report(DiagnosticsEngine::Warning, "buried assignment, rather put on own line", + compat::getBeginLoc(cxxOper)) + << cxxOper->getSourceRange(); + //getParentStmt(getParentStmt(getParentStmt(getParentStmt(getParentStmt(cxxOper)))))->dump(); return true; } -void BuriedAssign::checkExpr(Expr const* binaryOp) +bool BuriedAssign::VisitCompoundStmt(CompoundStmt const* compoundStmt) { - if (compiler.getSourceManager().isMacroBodyExpansion(compat::getBeginLoc(binaryOp))) - return; - if (compiler.getSourceManager().isMacroArgExpansion(compat::getBeginLoc(binaryOp))) - return; - - /** - Note: I tried writing this plugin without getParentStmt, but in order to make that work, I had to - hack things like TraverseWhileStmt to call TraverseStmt on the child nodes myself, so I could know whether I was inside the body or the condition. - But I could not get that to work, so switched to this approach. - */ - - // look up past the temporary nodes - Stmt const* child = binaryOp; - Stmt const* parent = getParentStmt(binaryOp); - while (true) + if (ignoreLocation(compoundStmt)) + return true; + for (auto i = compoundStmt->child_begin(); i != compoundStmt->child_end(); ++i) { - // This part is not ideal, but getParentStmt() appears to fail us in some cases, notably when the assignment - // is inside a decl like: - // int x = a = 1; - if (!parent) - return; - if (!(isa<MaterializeTemporaryExpr>(parent) || isa<CXXBindTemporaryExpr>(parent) - || isa<ImplicitCastExpr>(parent) || isa<CXXConstructExpr>(parent) - || isa<ParenExpr>(parent) || isa<ExprWithCleanups>(parent))) - break; - child = parent; - parent = getParentStmt(parent); + if (auto expr = dyn_cast<Expr>(*i)) + { + expr = compat::IgnoreImplicit(expr); + if (auto binaryOp = dyn_cast<BinaryOperator>(expr)) + { + // ignore comma-chained statements at this level + if (binaryOp->getOpcode() == BO_Comma) + { + MarkIfAssignment(binaryOp->getLHS()); + MarkIfAssignment(binaryOp->getRHS()); + continue; + } + } + MarkIfAssignment(expr); + } } + return true; +} - if (isa<CompoundStmt>(parent)) - return; - // ignore chained assignment like "a = b = 1;" - if (auto cxxOper = dyn_cast<CXXOperatorCallExpr>(parent)) - { - if (cxxOper->getOperator() == OO_Equal) - return; - } - // ignore chained assignment like "a = b = 1;" - if (auto parentBinOp = dyn_cast<BinaryOperator>(parent)) +void BuriedAssign::MarkIfAssignment(Stmt const* stmt) +{ + if (auto expr = dyn_cast_or_null<Expr>(stmt)) { - if (parentBinOp->getOpcode() == BO_Assign) - return; + expr = compat::IgnoreImplicit(expr); + if (auto binaryOp = dyn_cast<BinaryOperator>(expr)) + { + if (isAssignmentOp(binaryOp->getOpcode())) + { + m_handled.insert(expr); + MarkIfAssignment(binaryOp->getRHS()); // in case it is chained + } + else if (binaryOp->getOpcode() == BO_Comma) + { + MarkIfAssignment(binaryOp->getLHS()); + MarkIfAssignment(binaryOp->getRHS()); + } + } + else if (auto cxxOper = dyn_cast<CXXOperatorCallExpr>(expr)) + { + if (isAssignmentOp(cxxOper->getOperator())) + { + m_handled.insert(expr); + MarkIfAssignment(cxxOper->getArg(1)); // in case it is chained + } + } } - // ignore chained assignment like "int a = b = 1;" - if (isa<DeclStmt>(parent)) - return; +} + +bool BuriedAssign::VisitIfStmt(IfStmt const* ifStmt) +{ + if (ignoreLocation(ifStmt)) + return true; + MarkConditionForControlLoops(ifStmt->getCond()); + MarkIfAssignment(ifStmt->getThen()); + MarkIfAssignment(ifStmt->getElse()); + return true; +} + +bool BuriedAssign::VisitCaseStmt(CaseStmt const* stmt) +{ + if (ignoreLocation(stmt)) + return true; + MarkIfAssignment(stmt->getSubStmt()); + return true; +} + +bool BuriedAssign::VisitDefaultStmt(DefaultStmt const* stmt) +{ + if (ignoreLocation(stmt)) + return true; + MarkIfAssignment(stmt->getSubStmt()); + return true; +} + +bool BuriedAssign::VisitWhileStmt(WhileStmt const* stmt) +{ + if (ignoreLocation(stmt)) + return true; + MarkConditionForControlLoops(stmt->getCond()); + MarkIfAssignment(stmt->getBody()); + return true; +} + +bool BuriedAssign::VisitDoStmt(DoStmt const* stmt) +{ + if (ignoreLocation(stmt)) + return true; + MarkConditionForControlLoops(stmt->getCond()); + MarkIfAssignment(stmt->getBody()); + return true; +} - if (isa<CaseStmt>(parent) || isa<DefaultStmt>(parent) || isa<LabelStmt>(parent) - || isa<ForStmt>(parent) || isa<CXXForRangeStmt>(parent) || isa<IfStmt>(parent) - || isa<DoStmt>(parent) || isa<WhileStmt>(parent) || isa<ReturnStmt>(parent)) +/** stuff like + * while ((x = foo()) + * and + * while ((x = foo() < 0) + * is considered idiomatic. + */ +void BuriedAssign::MarkConditionForControlLoops(Expr const* expr) +{ + if (!expr) return; + expr = compat::IgnoreImplicit(expr); - // now check for the statements where we don't care at all if we see a buried assignment - while (true) + if (auto binaryOp = dyn_cast<BinaryOperator>(expr)) { - if (isa<CompoundStmt>(parent)) - break; - if (isa<CaseStmt>(parent) || isa<DefaultStmt>(parent) || isa<LabelStmt>(parent)) - return; - // Ignore assign in these statements, just seems to be part of the "natural" idiom of C/C++ - // TODO: perhaps include ReturnStmt? - if (auto forStmt = dyn_cast<ForStmt>(parent)) - { - if (child == forStmt->getBody()) - break; - return; - } - if (auto forRangeStmt = dyn_cast<CXXForRangeStmt>(parent)) + // ignore comma-chained statements at this level + if (binaryOp->getOpcode() == BO_Comma) { - if (child == forRangeStmt->getBody()) - break; + MarkConditionForControlLoops(binaryOp->getLHS()); + MarkConditionForControlLoops(binaryOp->getRHS()); return; } - if (auto ifStmt = dyn_cast<IfStmt>(parent)) + } + + // unwrap conversion to bool + if (auto memberCall = dyn_cast<CXXMemberCallExpr>(expr)) + { + if (memberCall->getMethodDecl() && isa<CXXConversionDecl>(memberCall->getMethodDecl())) { - if (child == ifStmt->getCond()) - return; + // TODO check that the conversion is converting to bool + expr = compat::IgnoreImplicit(memberCall->getImplicitObjectArgument()); } - if (auto doStmt = dyn_cast<DoStmt>(parent)) + } + + if (auto binaryOp = dyn_cast<BinaryOperator>(expr)) + { + // handle: ((xxx = foo()) != error) + if (binaryOp->isComparisonOp()) { - if (child == doStmt->getCond()) - return; + MarkIfAssignment(compat::IgnoreImplicit(binaryOp->getLHS())->IgnoreParens()); + MarkIfAssignment(compat::IgnoreImplicit(binaryOp->getRHS())->IgnoreParens()); } - if (auto whileStmt = dyn_cast<WhileStmt>(parent)) + } + else if (auto cxxOper = dyn_cast<CXXOperatorCallExpr>(expr)) + { + // handle: ((xxx = foo()) != error) + if (isComparisonOp(cxxOper->getOperator())) { - if (child == whileStmt->getBody()) - break; - return; + MarkIfAssignment(compat::IgnoreImplicit(cxxOper->getArg(0))->IgnoreParens()); + MarkIfAssignment(compat::IgnoreImplicit(cxxOper->getArg(1))->IgnoreParens()); } - if (isa<ReturnStmt>(parent)) - return; - // This appears to be a valid way of making it obvious that we need to call acquire when assigning such ref-counted - // stuff e.g. - // rtl_uString_acquire( a = b ); - if (auto callExpr = dyn_cast<CallExpr>(parent)) + // handle: (!(xxx = foo())) + else if (cxxOper->getOperator() == OO_Exclaim) + MarkIfAssignment(compat::IgnoreImplicit(cxxOper->getArg(0))->IgnoreParens()); + } + else if (auto parenExpr = dyn_cast<ParenExpr>(expr)) + { + // handle: ((xxx = foo())) + MarkIfAssignment(compat::IgnoreImplicit(parenExpr->getSubExpr())); + } + else if (auto unaryOp = dyn_cast<UnaryOperator>(expr)) + { + // handle: (!(xxx = foo())) + MarkIfAssignment(compat::IgnoreImplicit(unaryOp->getSubExpr())->IgnoreParens()); + } + else + MarkIfAssignment(expr); +} + +bool BuriedAssign::VisitForStmt(ForStmt const* stmt) +{ + if (ignoreLocation(stmt)) + return true; + MarkIfAssignment(stmt->getInit()); + MarkConditionForControlLoops(stmt->getCond()); + MarkIfAssignment(stmt->getInc()); + MarkIfAssignment(stmt->getBody()); + return true; +} + +bool BuriedAssign::VisitCXXForRangeStmt(CXXForRangeStmt const* stmt) +{ + if (ignoreLocation(stmt)) + return true; + MarkIfAssignment(stmt->getBody()); + return true; +} + +bool BuriedAssign::VisitLabelStmt(LabelStmt const* stmt) +{ + if (ignoreLocation(stmt)) + return true; + MarkIfAssignment(stmt->getSubStmt()); + return true; +} + +bool BuriedAssign::VisitVarDecl(VarDecl const* stmt) +{ + if (ignoreLocation(stmt)) + return true; + if (stmt->getInit()) + { + auto expr = IgnoreImplicitAndConversionOperator(stmt->getInit()); + MarkIfAssignment(expr); + if (auto cxxConstruct = dyn_cast<CXXConstructExpr>(expr)) { - if (callExpr->getDirectCallee() && callExpr->getDirectCallee()->getIdentifier()) - { - auto name = callExpr->getDirectCallee()->getName(); - if (name == "rtl_uString_acquire" || name == "_acquire" - || name == "typelib_typedescriptionreference_acquire") - return; - } + if (cxxConstruct->getNumArgs() == 1) + MarkIfAssignment(cxxConstruct->getArg(0)); } - child = parent; - parent = getParentStmt(parent); - if (!parent) - break; } + return true; +} - report(DiagnosticsEngine::Warning, "buried assignment, very hard to read", - compat::getBeginLoc(binaryOp)) - << binaryOp->getSourceRange(); +bool BuriedAssign::VisitCXXFoldExpr(CXXFoldExpr const* stmt) +{ + if (ignoreLocation(stmt)) + return true; + MarkConditionForControlLoops(stmt->getLHS()); + MarkConditionForControlLoops(stmt->getRHS()); + return true; } // off by default because it uses getParentStmt so it's more expensive to run diff --git a/compilerplugins/clang/test/buriedassign.cxx b/compilerplugins/clang/test/buriedassign.cxx index 7e41a83ccaa5..b44a7cce6039 100644 --- a/compilerplugins/clang/test/buriedassign.cxx +++ b/compilerplugins/clang/test/buriedassign.cxx @@ -8,6 +8,7 @@ */ #include <map> +#include <memory> #include <rtl/ustring.hxx> namespace test1 @@ -17,18 +18,19 @@ int foo(int); void main() { int x = 1; - foo(x = 2); // expected-error {{buried assignment, very hard to read [loplugin:buriedassign]}} + foo(x = 2); + // expected-error@-1 {{buried assignment, rather put on own line [loplugin:buriedassign]}} int y = x = 1; // no warning expected (void)y; - int z = foo( - x = 1); // expected-error {{buried assignment, very hard to read [loplugin:buriedassign]}} + int z = foo(x = 1); + // expected-error@-1 {{buried assignment, rather put on own line [loplugin:buriedassign]}} (void)z; switch (x = 1) - { // expected-error@-1 {{buried assignment, very hard to read [loplugin:buriedassign]}} + { // expected-error@-1 {{buried assignment, rather put on own line [loplugin:buriedassign]}} } std::map<int, int> map1; - map1[x = 1] - = 1; // expected-error@-1 {{buried assignment, very hard to read [loplugin:buriedassign]}} + map1[x = 1] = 1; + // expected-error@-1 {{buried assignment, rather put on own line [loplugin:buriedassign]}} } } @@ -51,16 +53,17 @@ MyInt foo(MyInt); void main() { MyInt x = 1; - foo(x = 2); // expected-error {{buried assignment, very hard to read [loplugin:buriedassign]}} + foo(x = 2); + // expected-error@-1 {{buried assignment, rather put on own line [loplugin:buriedassign]}} MyInt y = x = 1; // no warning expected (void)y; - MyInt z = foo( - x = 1); // expected-error {{buried assignment, very hard to read [loplugin:buriedassign]}} + MyInt z = foo(x = 1); + // expected-error@-1 {{buried assignment, rather put on own line [loplugin:buriedassign]}} (void)z; z = x; // no warning expected std::map<MyInt, int> map1; - map1[x = 1] - = 1; // expected-error@-1 {{buried assignment, very hard to read [loplugin:buriedassign]}} + map1[x = 1] = 1; + // expected-error@-1 {{buried assignment, rather put on own line [loplugin:buriedassign]}} } } @@ -73,8 +76,24 @@ void main(OUString sUserAutoCorrFile, OUString sExt) sRet = sUserAutoCorrFile; // no warning expected if (sUserAutoCorrFile == "yyy") (sRet = sUserAutoCorrFile) - += sExt; // expected-error@-1 {{buried assignment, very hard to read [loplugin:buriedassign]}} + += sExt; // expected-error@-1 {{buried assignment, rather put on own line [loplugin:buriedassign]}} } } +// no warning expected +namespace test4 +{ +struct Task +{ + void exec(); +}; +std::unique_ptr<Task> pop(); + +void foo() +{ + std::unique_ptr<Task> pTask; + while ((pTask = pop())) + pTask->exec(); +} +} /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/connectivity/source/commontools/FDatabaseMetaDataResultSetMetaData.cxx b/connectivity/source/commontools/FDatabaseMetaDataResultSetMetaData.cxx index 612aee377686..561953a0796c 100644 --- a/connectivity/source/commontools/FDatabaseMetaDataResultSetMetaData.cxx +++ b/connectivity/source/commontools/FDatabaseMetaDataResultSetMetaData.cxx @@ -32,7 +32,7 @@ ODatabaseMetaDataResultSetMetaData::~ODatabaseMetaDataResultSetMetaData() sal_Int32 SAL_CALL ODatabaseMetaDataResultSetMetaData::getColumnDisplaySize( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.getColumnDisplaySize(); return 0; @@ -40,7 +40,7 @@ sal_Int32 SAL_CALL ODatabaseMetaDataResultSetMetaData::getColumnDisplaySize( sal sal_Int32 SAL_CALL ODatabaseMetaDataResultSetMetaData::getColumnType( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.getColumnType(); return 1; } @@ -52,7 +52,7 @@ sal_Int32 SAL_CALL ODatabaseMetaDataResultSetMetaData::getColumnCount( ) sal_Bool SAL_CALL ODatabaseMetaDataResultSetMetaData::isCaseSensitive( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.isCaseSensitive(); return true; } @@ -64,14 +64,14 @@ OUString SAL_CALL ODatabaseMetaDataResultSetMetaData::getSchemaName( sal_Int32 / OUString SAL_CALL ODatabaseMetaDataResultSetMetaData::getColumnName( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.getColumnName(); return OUString(); } OUString SAL_CALL ODatabaseMetaDataResultSetMetaData::getTableName( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.getTableName(); return OUString(); } @@ -88,7 +88,7 @@ OUString SAL_CALL ODatabaseMetaDataResultSetMetaData::getColumnTypeName( sal_Int OUString SAL_CALL ODatabaseMetaDataResultSetMetaData::getColumnLabel( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.getColumnLabel(); return getColumnName(column); } @@ -100,35 +100,35 @@ OUString SAL_CALL ODatabaseMetaDataResultSetMetaData::getColumnServiceName( sal_ sal_Bool SAL_CALL ODatabaseMetaDataResultSetMetaData::isCurrency( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.isCurrency(); return false; } sal_Bool SAL_CALL ODatabaseMetaDataResultSetMetaData::isAutoIncrement( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.isAutoIncrement(); return false; } sal_Bool SAL_CALL ODatabaseMetaDataResultSetMetaData::isSigned( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.isSigned(); return false; } sal_Int32 SAL_CALL ODatabaseMetaDataResultSetMetaData::getPrecision( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.getPrecision(); return 0; } sal_Int32 SAL_CALL ODatabaseMetaDataResultSetMetaData::getScale( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.getScale(); return 0; @@ -136,7 +136,7 @@ sal_Int32 SAL_CALL ODatabaseMetaDataResultSetMetaData::getScale( sal_Int32 colum sal_Int32 SAL_CALL ODatabaseMetaDataResultSetMetaData::isNullable( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.isNullable(); return sal_Int32(false); @@ -144,7 +144,7 @@ sal_Int32 SAL_CALL ODatabaseMetaDataResultSetMetaData::isNullable( sal_Int32 col sal_Bool SAL_CALL ODatabaseMetaDataResultSetMetaData::isSearchable( sal_Int32 column ) { - if(!m_mColumns.empty() && (m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) + if((m_mColumnsIter = m_mColumns.find(column)) != m_mColumns.end()) return (*m_mColumnsIter).second.isSearchable(); return true; } diff --git a/connectivity/source/commontools/dbtools2.cxx b/connectivity/source/commontools/dbtools2.cxx index 26b60b23933a..38efeea57598 100644 --- a/connectivity/source/commontools/dbtools2.cxx +++ b/connectivity/source/commontools/dbtools2.cxx @@ -116,10 +116,11 @@ OUString createStandardTypePart(const Reference< XPropertySet >& xColProp,const } } - sal_Int32 nIndex = 0; - if ( !sAutoIncrementValue.isEmpty() && (nIndex = sTypeName.indexOf(sAutoIncrementValue)) != -1 ) + if ( !sAutoIncrementValue.isEmpty() ) { - sTypeName = sTypeName.replaceAt(nIndex,sTypeName.getLength() - nIndex,OUString()); + sal_Int32 nIndex = sTypeName.indexOf(sAutoIncrementValue); + if (nIndex != -1) + sTypeName = sTypeName.replaceAt(nIndex,sTypeName.getLength() - nIndex,OUString()); } if ( (nPrecision > 0 || nScale > 0) && bUseLiteral ) diff --git a/connectivity/source/commontools/parameters.cxx b/connectivity/source/commontools/parameters.cxx index 1461374173b5..bd114ea19341 100644 --- a/connectivity/source/commontools/parameters.cxx +++ b/connectivity/source/commontools/parameters.cxx @@ -236,7 +236,7 @@ namespace dbtools o_rNewParamName += "_"; } - return sFilter += " =:" + o_rNewParamName; + return sFilter + " =:" + o_rNewParamName; } diff --git a/connectivity/source/drivers/dbase/DIndexIter.cxx b/connectivity/source/drivers/dbase/DIndexIter.cxx index 48ddf65459eb..62cc6601a54f 100644 --- a/connectivity/source/drivers/dbase/DIndexIter.cxx +++ b/connectivity/source/drivers/dbase/DIndexIter.cxx @@ -153,10 +153,14 @@ sal_uInt32 OIndexIterator::GetCompare(bool bFirst) switch (ePredicateType) { case SQLFilterOperator::NOT_EQUAL: - while ( ( ( pKey = GetNextKey() ) != nullptr ) && !m_pOperator->operate(pKey,m_pOperand)) ; + while ( ( pKey = GetNextKey() ) != nullptr ) + if (m_pOperator->operate(pKey,m_pOperand)) + break; break; case SQLFilterOperator::LESS: - while ( ( ( pKey = GetNextKey() ) != nullptr ) && pKey->getValue().isNull()) ; + while ( ( pKey = GetNextKey() ) != nullptr ) + if (!pKey->getValue().isNull()) + break; break; case SQLFilterOperator::LESS_EQUAL: while ( ( pKey = GetNextKey() ) != nullptr ) ; @@ -168,7 +172,9 @@ sal_uInt32 OIndexIterator::GetCompare(bool bFirst) case SQLFilterOperator::GREATER: pKey = GetFirstKey(m_aRoot,*m_pOperand); if ( !pKey ) - while ( ( ( pKey = GetNextKey() ) != nullptr ) && !m_pOperator->operate(pKey,m_pOperand)) ; + while ( ( pKey = GetNextKey() ) != nullptr ) + if (m_pOperator->operate(pKey,m_pOperand)) + break; } } else @@ -176,13 +182,15 @@ sal_uInt32 OIndexIterator::GetCompare(bool bFirst) switch (ePredicateType) { case SQLFilterOperator::NOT_EQUAL: - while ( ( ( pKey = GetNextKey() ) != nullptr ) && !m_pOperator->operate(pKey,m_pOperand)) - ; + while ( ( pKey = GetNextKey() ) != nullptr ) + if (m_pOperator->operate(pKey,m_pOperand)) + break; break; case SQLFilterOperator::LESS: case SQLFilterOperator::LESS_EQUAL: case SQLFilterOperator::EQUAL: - if ( ( ( pKey = GetNextKey() ) == nullptr ) || !m_pOperator->operate(pKey,m_pOperand)) + pKey = GetNextKey(); + if ( pKey == nullptr || !m_pOperator->operate(pKey,m_pOperand)) { pKey = nullptr; m_aCurLeaf.Clear(); @@ -212,8 +220,9 @@ sal_uInt32 OIndexIterator::GetLike(bool bFirst) } ONDXKey* pKey; - while ( ( ( pKey = GetNextKey() ) != nullptr ) && !m_pOperator->operate(pKey,m_pOperand)) - ; + while ( ( pKey = GetNextKey() ) != nullptr ) + if (m_pOperator->operate(pKey,m_pOperand)) + break; return pKey ? pKey->GetRecord() : NODE_NOTFOUND; } @@ -230,8 +239,8 @@ sal_uInt32 OIndexIterator::GetNull(bool bFirst) m_nCurNode = NODE_NOTFOUND; } - ONDXKey* pKey; - if ( ( ( pKey = GetNextKey() ) == nullptr ) || !pKey->getValue().isNull()) + ONDXKey* pKey = GetNextKey(); + if ( pKey == nullptr || !pKey->getValue().isNull()) { pKey = nullptr; m_aCurLeaf.Clear(); diff --git a/connectivity/source/drivers/dbase/dindexnode.cxx b/connectivity/source/drivers/dbase/dindexnode.cxx index 4b1c24057020..ae105b8be512 100644 --- a/connectivity/source/drivers/dbase/dindexnode.cxx +++ b/connectivity/source/drivers/dbase/dindexnode.cxx @@ -1002,8 +1002,13 @@ void ONDXPage::SearchAndReplace(const ONDXKey& rSearch, sal_uInt16 nPos = NODE_NOTFOUND; ONDXPage* pPage = this; - while (pPage && (nPos = pPage->Search(rSearch)) == NODE_NOTFOUND) + while (pPage) + { + nPos = pPage->Search(rSearch); + if (nPos != NODE_NOTFOUND) + break; pPage = pPage->aParent; + } if (pPage) { diff --git a/connectivity/source/drivers/firebird/Util.cxx b/connectivity/source/drivers/firebird/Util.cxx index 090e34ca2781..7befd4b99014 100644 --- a/connectivity/source/drivers/firebird/Util.cxx +++ b/connectivity/source/drivers/firebird/Util.cxx @@ -394,11 +394,13 @@ OUString firebird::escapeWith( const OUString& sText, const char aKey, const cha { OUString sRet(sText); sal_Int32 aIndex = 0; - while( (aIndex = sRet.indexOf(aKey, aIndex)) > 0 && - aIndex < sRet.getLength()) + for (;;) { - sRet = sRet.replaceAt(aIndex, 1, OUStringChar(aEscapeChar) + OUStringChar(aKey) ); - aIndex+= 2; + aIndex = sRet.indexOf(aKey, aIndex); + if ( aIndex <= 0 || aIndex >= sRet.getLength()) + break; + sRet = sRet.replaceAt(aIndex, 1, OUStringChar(aEscapeChar) + OUStringChar(aKey) ); + aIndex += 2; } return sRet; diff --git a/connectivity/source/drivers/mysql_jdbc/YTable.cxx b/connectivity/source/drivers/mysql_jdbc/YTable.cxx index c5e9f9a5891f..aec19f36ddcf 100644 --- a/connectivity/source/drivers/mysql_jdbc/YTable.cxx +++ b/connectivity/source/drivers/mysql_jdbc/YTable.cxx @@ -203,13 +203,15 @@ void SAL_CALL OMySQLTable::alterColumnByName(const OUString& colName, } else { - sal_Int32 nIndex = 0; - if (!sTypeName.isEmpty() - && (nIndex = sTypeName.indexOf(s_sAutoIncrement)) != -1) + if (!sTypeName.isEmpty()) { - sTypeName = sTypeName.copy(0, nIndex); - descriptor->setPropertyValue(rProp.getNameByIndex(PROPERTY_ID_TYPENAME), - makeAny(sTypeName)); + sal_Int32 nIndex = sTypeName.indexOf(s_sAutoIncrement); + if (nIndex != -1) + { + sTypeName = sTypeName.copy(0, nIndex); + descriptor->setPropertyValue(rProp.getNameByIndex(PROPERTY_ID_TYPENAME), + makeAny(sTypeName)); + } } } } diff --git a/connectivity/source/drivers/odbc/OFunctions.cxx b/connectivity/source/drivers/odbc/OFunctions.cxx index 5b6138d6a7e6..be68fdfcfe56 100644 --- a/connectivity/source/drivers/odbc/OFunctions.cxx +++ b/connectivity/source/drivers/odbc/OFunctions.cxx @@ -110,7 +110,8 @@ bool LoadLibrary_ODBC3(OUString &_rPath) if( !pODBCso) return false; - return bLoaded = LoadFunctions(pODBCso); + bLoaded = LoadFunctions(pODBCso); + return bLoaded; } diff --git a/connectivity/source/drivers/odbc/OResultSetMetaData.cxx b/connectivity/source/drivers/odbc/OResultSetMetaData.cxx index 6eb19d6b945f..21b95c6a7b29 100644 --- a/connectivity/source/drivers/odbc/OResultSetMetaData.cxx +++ b/connectivity/source/drivers/odbc/OResultSetMetaData.cxx @@ -163,7 +163,8 @@ sal_Int32 SAL_CALL OResultSetMetaData::getColumnCount( ) return m_nColCount; sal_Int16 nNumResultCols=0; OTools::ThrowException(m_pConnection,N3SQLNumResultCols(m_aStatementHandle,&nNumResultCols),m_aStatementHandle,SQL_HANDLE_STMT,*this); - return m_nColCount = nNumResultCols; + m_nColCount = nNumResultCols; + return m_nColCount; } diff --git a/connectivity/source/drivers/postgresql/pq_tools.cxx b/connectivity/source/drivers/postgresql/pq_tools.cxx index 558fd6d6a739..37f6cb575c22 100644 --- a/connectivity/source/drivers/postgresql/pq_tools.cxx +++ b/connectivity/source/drivers/postgresql/pq_tools.cxx @@ -847,14 +847,24 @@ css::uno::Sequence< sal_Int32 > string2intarray( const OUString & str ) { sal_Int32 start = 0; sal_uInt32 c; - while ( iswspace( (c=str.iterateCodePoints(&start)) ) ) + for (;;) + { + c = str.iterateCodePoints(&start); + if (!iswspace(c)) + break; if ( start == strlen) return ret; + } if ( c != L'{' ) return ret; - while ( iswspace( c=str.iterateCodePoints(&start) ) ) + for (;;) + { + c = str.iterateCodePoints(&start); + if ( !iswspace(c) ) + break; if ( start == strlen) return ret; + } if ( c == L'}' ) return ret; diff --git a/cppcanvas/source/mtfrenderer/implrenderer.cxx b/cppcanvas/source/mtfrenderer/implrenderer.cxx index 14e3981547d5..db2edcd3bde1 100644 --- a/cppcanvas/source/mtfrenderer/implrenderer.cxx +++ b/cppcanvas/source/mtfrenderer/implrenderer.cxx @@ -1628,9 +1628,11 @@ namespace cppcanvas::internal { MetaGradientExAction* pGradAction = nullptr; bool bDone( false ); - while( !bDone && - (pCurrAct=rMtf.NextAction()) != nullptr ) + while( !bDone ) { + pCurrAct=rMtf.NextAction(); + if (!pCurrAct) + break; switch( pCurrAct->GetType() ) { // extract gradient info diff --git a/cppu/source/typelib/static_types.cxx b/cppu/source/typelib/static_types.cxx index 6bbdf4c63eab..77f443a6a637 100644 --- a/cppu/source/typelib/static_types.cxx +++ b/cppu/source/typelib/static_types.cxx @@ -182,8 +182,9 @@ typelib_TypeDescriptionReference ** SAL_CALL typelib_static_type_getByTypeClass( &pTD, sTypeName.pData, 0, 0, 0, 0, 0, nullptr, 3, pMembers ); ::typelib_typedescription_register( reinterpret_cast<typelib_TypeDescription **>(&pTD) ); + s_aTypes[typelib_TypeClass_INTERFACE] = pTD->aBase.pWeakRef; ::typelib_typedescriptionreference_acquire( - s_aTypes[typelib_TypeClass_INTERFACE] = pTD->aBase.pWeakRef ); + s_aTypes[typelib_TypeClass_INTERFACE] ); // another static ref: ++s_aTypes[typelib_TypeClass_INTERFACE]->nStaticRefCount; ::typelib_typedescription_release( &pTD->aBase ); @@ -212,8 +213,9 @@ typelib_TypeDescriptionReference ** SAL_CALL typelib_static_type_getByTypeClass( ::typelib_typedescription_new( &pTD1, typelib_TypeClass_EXCEPTION, sTypeName1.pData, nullptr, 2, aMembers ); typelib_typedescription_register( &pTD1 ); + s_aTypes[typelib_TypeClass_EXCEPTION] = pTD1->pWeakRef; typelib_typedescriptionreference_acquire( - s_aTypes[typelib_TypeClass_EXCEPTION] = pTD1->pWeakRef ); + s_aTypes[typelib_TypeClass_EXCEPTION]); // another static ref: ++s_aTypes[typelib_TypeClass_EXCEPTION]->nStaticRefCount; // RuntimeException @@ -369,8 +371,9 @@ void init( } for ( sal_Int32 i = 0 ; i < nMembers; ++i ) { + pComp->ppTypeRefs[i] = ppMembers[i]; ::typelib_typedescriptionreference_acquire( - pComp->ppTypeRefs[i] = ppMembers[i] ); + pComp->ppTypeRefs[i] ); // write offset typelib_TypeDescription * pTD = nullptr; TYPELIB_DANGER_GET( &pTD, pComp->ppTypeRefs[i] ); diff --git a/cppu/source/typelib/typelib.cxx b/cppu/source/typelib/typelib.cxx index 1f7af60693e2..92a7e6ca5120 100644 --- a/cppu/source/typelib/typelib.cxx +++ b/cppu/source/typelib/typelib.cxx @@ -661,10 +661,10 @@ extern "C" void typelib_typedescription_newEmpty( pRet->nRefCount = 1; // reference count is initially 1 pRet->nStaticRefCount = 0; pRet->eTypeClass = eTypeClass; - pRet->pTypeName = nullptr; pRet->pUniqueIdentifier = nullptr; pRet->pReserved = nullptr; - rtl_uString_acquire( pRet->pTypeName = pTypeName ); + pRet->pTypeName = pTypeName; + rtl_uString_acquire( pRet->pTypeName ); pRet->pSelf = pRet; pRet->bComplete = true; pRet->nSize = 0; @@ -740,17 +740,17 @@ void newTypeDescription( typelib_typedescriptionreference_new( pTmp->ppTypeRefs +i, pCompoundMembers[i].eTypeClass, pCompoundMembers[i].pTypeName ); - rtl_uString_acquire( - pTmp->ppMemberNames[i] - = pCompoundMembers[i].pMemberName ); + pTmp->ppMemberNames[i] + = pCompoundMembers[i].pMemberName; + rtl_uString_acquire( pTmp->ppMemberNames[i] ); } else { typelib_typedescriptionreference_new( pTmp->ppTypeRefs +i, pStructMembers[i].aBase.eTypeClass, pStructMembers[i].aBase.pTypeName ); - rtl_uString_acquire( - pTmp->ppMemberNames[i] - = pStructMembers[i].aBase.pMemberName ); + pTmp->ppMemberNames[i] + = pStructMembers[i].aBase.pMemberName; + rtl_uString_acquire(pTmp->ppMemberNames[i]); } // write offset sal_Int32 size; @@ -843,7 +843,8 @@ extern "C" void SAL_CALL typelib_typedescription_newEnum( pEnum->ppEnumNames = new rtl_uString * [ nEnumValues ]; for ( sal_Int32 nPos = nEnumValues; nPos--; ) { - rtl_uString_acquire( pEnum->ppEnumNames[nPos] = ppEnumNames[nPos] ); + pEnum->ppEnumNames[nPos] = ppEnumNames[nPos]; + rtl_uString_acquire( pEnum->ppEnumNames[nPos] ); } pEnum->pEnumValues = new sal_Int32[ nEnumValues ]; ::memcpy( pEnum->pEnumValues, pEnumValues, nEnumValues * sizeof(sal_Int32) ); @@ -1127,8 +1128,8 @@ extern "C" void SAL_CALL typelib_typedescription_newInterfaceMethod( for( sal_Int32 i = 0; i < nParams; i++ ) { // get the name of the parameter - (*ppRet)->pParams[ i ].pName = nullptr; - rtl_uString_acquire( (*ppRet)->pParams[ i ].pName = pParams[i].pParamName ); + (*ppRet)->pParams[ i ].pName = pParams[i].pParamName; + rtl_uString_acquire( (*ppRet)->pParams[ i ].pName ); (*ppRet)->pParams[ i ].pTypeRef = nullptr; // get the type name of the parameter and create the weak reference typelib_typedescriptionreference_new( @@ -1722,7 +1723,8 @@ typelib_TypeDescriptionReference ** copyExceptions( typelib_TypeDescriptionReference ** p = new typelib_TypeDescriptionReference *[count]; for (sal_Int32 i = 0; i < count; ++i) { - typelib_typedescriptionreference_acquire(p[i] = source[i]); + p[i] = source[i]; + typelib_typedescriptionreference_acquire(p[i]); } return p; } @@ -1746,21 +1748,25 @@ bool createDerivedInterfaceMemberDescription( = reinterpret_cast< typelib_InterfaceMethodTypeDescription * >(*result); newMethod->aBase.nPosition = position; + newMethod->aBase.pMemberName + = baseMethod->aBase.pMemberName; rtl_uString_acquire( - newMethod->aBase.pMemberName - = baseMethod->aBase.pMemberName); + newMethod->aBase.pMemberName); + newMethod->pReturnTypeRef = baseMethod->pReturnTypeRef; typelib_typedescriptionreference_acquire( - newMethod->pReturnTypeRef = baseMethod->pReturnTypeRef); + newMethod->pReturnTypeRef); newMethod->nParams = baseMethod->nParams; newMethod->pParams = new typelib_MethodParameter[ newMethod->nParams]; for (sal_Int32 i = 0; i < newMethod->nParams; ++i) { + newMethod->pParams[i].pName + = baseMethod->pParams[i].pName; rtl_uString_acquire( - newMethod->pParams[i].pName - = baseMethod->pParams[i].pName); + newMethod->pParams[i].pName); + newMethod->pParams[i].pTypeRef + = baseMethod->pParams[i].pTypeRef; typelib_typedescriptionreference_acquire( - newMethod->pParams[i].pTypeRef - = baseMethod->pParams[i].pTypeRef); + newMethod->pParams[i].pTypeRef); newMethod->pParams[i].bIn = baseMethod->pParams[i].bIn; newMethod->pParams[i].bOut = baseMethod->pParams[i].bOut; } @@ -1787,13 +1793,13 @@ bool createDerivedInterfaceMemberDescription( = reinterpret_cast< typelib_InterfaceAttributeTypeDescription * >(*result); newAttribute->aBase.nPosition = position; - rtl_uString_acquire( - newAttribute->aBase.pMemberName - = baseAttribute->aBase.pMemberName); + newAttribute->aBase.pMemberName + = baseAttribute->aBase.pMemberName; + rtl_uString_acquire(newAttribute->aBase.pMemberName); newAttribute->bReadOnly = baseAttribute->bReadOnly; - typelib_typedescriptionreference_acquire( - newAttribute->pAttributeTypeRef - = baseAttribute->pAttributeTypeRef); + newAttribute->pAttributeTypeRef + = baseAttribute->pAttributeTypeRef; + typelib_typedescriptionreference_acquire(newAttribute->pAttributeTypeRef); newAttribute->pInterface = reinterpret_cast< typelib_InterfaceTypeDescription * >( interface); @@ -2077,7 +2083,8 @@ extern "C" void SAL_CALL typelib_typedescriptionreference_new( pTDR->eTypeClass = eTypeClass; pTDR->pUniqueIdentifier = nullptr; pTDR->pReserved = nullptr; - rtl_uString_acquire( pTDR->pTypeName = pTypeName ); + pTDR->pTypeName = pTypeName; + rtl_uString_acquire( pTDR->pTypeName ); pTDR->pType = nullptr; *ppTDR = pTDR; } diff --git a/cppu/source/uno/copy.hxx b/cppu/source/uno/copy.hxx index b24e0e337bf5..b61973ec0b7f 100644 --- a/cppu/source/uno/copy.hxx +++ b/cppu/source/uno/copy.hxx @@ -244,7 +244,8 @@ inline void _copyConstructAnyFromData( } else { - _acquire( pDestAny->pReserved = *static_cast<void **>(pSource), acquire ); + pDestAny->pReserved = *static_cast<void **>(pSource); + _acquire( pDestAny->pReserved, acquire ); } break; default: @@ -640,7 +641,10 @@ inline void _copyConstructData( if (mapping) *static_cast<void **>(pDest) = _map( *static_cast<void **>(pSource), pType, pTypeDescr, mapping ); else - _acquire( *static_cast<void **>(pDest) = *static_cast<void **>(pSource), acquire ); + { + *static_cast<void **>(pDest) = *static_cast<void **>(pSource); + _acquire( *static_cast<void **>(pDest), acquire ); + } break; default: break; diff --git a/cppu/source/uno/lbenv.cxx b/cppu/source/uno/lbenv.cxx index 8bbd71891247..aeac8b7ddd8a 100644 --- a/cppu/source/uno/lbenv.cxx +++ b/cppu/source/uno/lbenv.cxx @@ -497,7 +497,9 @@ static void defenv_getRegisteredInterfaces( for (const auto& rEntry : that->aPtr2ObjectMap) { - (*pEnv->acquireInterface)( pEnv, ppInterfaces[nPos++] = rEntry.first ); + ppInterfaces[nPos] = rEntry.first; + (*pEnv->acquireInterface)( pEnv, ppInterfaces[nPos] ); + nPos++; } *pppInterfaces = ppInterfaces; @@ -858,7 +860,8 @@ static void unoenv_computeObjectIdentifier( // process;good guid oid.append( unoenv_getStaticOIdPart() ); OUString aStr( oid.makeStringAndClear() ); - ::rtl_uString_acquire( *ppOId = aStr.pData ); + *ppOId = aStr.pData; + ::rtl_uString_acquire( *ppOId ); } } diff --git a/cppu/source/uno/sequence.cxx b/cppu/source/uno/sequence.cxx index 43a5bc0d85f9..72d3e3ead42b 100644 --- a/cppu/source/uno/sequence.cxx +++ b/cppu/source/uno/sequence.cxx @@ -547,8 +547,8 @@ static bool icopyConstructFromElements( void ** pDestElements = reinterpret_cast<void **>(pSeq->elements); for ( sal_Int32 nPos = 0; nPos < nStopIndex; ++nPos ) { - _acquire( pDestElements[nPos] = - static_cast<void **>(pSourceElements)[nPos], acquire ); + pDestElements[nPos] = static_cast<void **>(pSourceElements)[nPos]; + _acquire( pDestElements[nPos], acquire ); } } break; diff --git a/cppuhelper/source/tdmgr.cxx b/cppuhelper/source/tdmgr.cxx index 970688d736b6..99229644fa7c 100644 --- a/cppuhelper/source/tdmgr.cxx +++ b/cppuhelper/source/tdmgr.cxx @@ -93,7 +93,8 @@ static typelib_TypeDescription * createCTD( rInit.eTypeClass = static_cast<typelib_TypeClass>(pMemberTypes[nPos]->getTypeClass()); OUString aMemberTypeName( pMemberTypes[nPos]->getName() ); - rtl_uString_acquire( rInit.pTypeName = aMemberTypeName.pData ); + rInit.pTypeName = aMemberTypeName.pData; + rtl_uString_acquire( rInit.pTypeName ); // string is held by rMemberNames rInit.pMemberName = pMemberNames[nPos].pData; @@ -165,8 +166,8 @@ static typelib_TypeDescription * createCTD( = static_cast<typelib_TypeClass>(pMemberTypes[nPos]->getTypeClass()); OUString aMemberTypeName( pMemberTypes[nPos]->getName() ); - rtl_uString_acquire( - rInit.aBase.pTypeName = aMemberTypeName.pData ); + rInit.aBase.pTypeName = aMemberTypeName.pData; + rtl_uString_acquire( rInit.aBase.pTypeName ); // string is held by rMemberNames rInit.aBase.pMemberName = pMemberNames[nPos].pData; @@ -256,9 +257,11 @@ static typelib_TypeDescription * createCTD( rInit.eTypeClass = static_cast<typelib_TypeClass>(xType->getTypeClass()); OUString aParamTypeName( xType->getName() ); - rtl_uString_acquire( rInit.pTypeName = aParamTypeName.pData ); + rInit.pTypeName = aParamTypeName.pData; + rtl_uString_acquire( rInit.pTypeName ); OUString aParamName( xParam->getName() ); - rtl_uString_acquire( rInit.pParamName = aParamName.pData ); + rInit.pParamName = aParamName.pData; + rtl_uString_acquire( rInit.pParamName ); rInit.bIn = xParam->isIn(); rInit.bOut = xParam->isOut(); } @@ -273,7 +276,8 @@ static typelib_TypeDescription * createCTD( for ( nPos = nExceptions; nPos--; ) { OUString aExceptionTypeName( pExceptions[nPos]->getName() ); - rtl_uString_acquire( ppExceptionNames[nPos] = aExceptionTypeName.pData ); + ppExceptionNames[nPos] = aExceptionTypeName.pData; + rtl_uString_acquire( ppExceptionNames[nPos] ); } OUString aTypeName( xMethod->getName() ); diff --git a/cui/source/dialogs/insdlg.cxx b/cui/source/dialogs/insdlg.cxx index 948a02d88efa..1df80452d056 100644 --- a/cui/source/dialogs/insdlg.cxx +++ b/cui/source/dialogs/insdlg.cxx @@ -462,7 +462,11 @@ short SfxInsertFloatingFrameDialog::run() bOK = m_xStorage.is(); } - if ( bOK && ( nRet = InsertObjectDialog_Impl::run() ) == RET_OK ) + if (!bOK) + return RET_OK; + + nRet = InsertObjectDialog_Impl::run(); + if ( nRet == RET_OK ) { OUString aURL; if (!m_xEDURL->get_text().isEmpty()) diff --git a/cui/source/tabpages/backgrnd.cxx b/cui/source/tabpages/backgrnd.cxx index 4a874605af03..e72ddcb4854b 100644 --- a/cui/source/tabpages/backgrnd.cxx +++ b/cui/source/tabpages/backgrnd.cxx @@ -75,22 +75,19 @@ SvxBkgTabPage::SvxBkgTabPage(weld::Container* pPage, weld::DialogController* pCo m_xBtnPattern->hide(); SfxObjectShell* pDocSh = SfxObjectShell::Current(); - const SfxPoolItem* pItem = nullptr; XColorListRef pColorTable; - if ( pDocSh && ( nullptr != ( pItem = pDocSh->GetItem( SID_COLOR_TABLE ) ) ) ) - { - pColorTable = static_cast<const SvxColorListItem*>(pItem)->GetColorList(); - } + if ( pDocSh ) + if (auto pItem = pDocSh->GetItem( SID_COLOR_TABLE )) + pColorTable = pItem->GetColorList(); if ( !pColorTable.is() ) pColorTable = XColorList::CreateStdColorList(); XBitmapListRef pBitmapList; - if ( pDocSh && ( nullptr != ( pItem = pDocSh->GetItem( SID_BITMAP_LIST ) ) ) ) - { - pBitmapList = static_cast<const SvxBitmapListItem*>(pItem)->GetBitmapList(); - } + if ( pDocSh ) + if (auto pItem = pDocSh->GetItem( SID_BITMAP_LIST ) ) + pBitmapList = pItem->GetBitmapList(); SetColorList(pColorTable); SetBitmapList(pBitmapList); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits