loplugin:buriedassign in c*

Change-Id: Id14fed7e5c0f588ad3c927f12251432d12c1a7c8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/92190
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

26 files changed, 654 insertions, 237 deletions

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);