loplugin:expressionalwayszero improvements

Change-Id: I00bdbc58d2295a0be30b47c85eae6b9abfec17b2
Reviewed-on: https://gerrit.libreoffice.org/51868
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

8 files changed, 79 insertions, 33 deletions

diff --git a/compilerplugins/clang/expressionalwayszero.cxx b/compilerplugins/clang/expressionalwayszero.cxx
index c34b73fec105..ff69154c7dc8 100644
--- a/compilerplugins/clang/expressionalwayszero.cxx
+++ b/compilerplugins/clang/expressionalwayszero.cxx
@@ -41,6 +41,22 @@ public:
         // encoding of constant value for binary file format
         if (startswith(fn, SRCDIR "/package/source/zipapi/ZipFile.cxx"))
             return;
+        // some auto-generated static data
+        if (startswith(fn, SRCDIR "/sal/textenc/tables.cxx"))
+            return;
+        // nested conditional defines that are not worth cleaning up
+        if (startswith(fn, SRCDIR "/opencl/source/openclwrapper.cxx"))
+            return;
+        // some kind of matrix calculation, the compiler will optimise it out anyway
+        if (startswith(fn, SRCDIR "/vcl/source/gdi/bitmap4.cxx"))
+            return;
+        // code follows a pattern
+        if (startswith(fn, SRCDIR "/svx/source/svdraw/svdhdl.cxx"))
+            return;
+        // looks like some kind of TODO marker
+        if (startswith(fn, SRCDIR "/chart2/source/view/main/PropertyMapper.cxx")
+            || startswith(fn, SRCDIR "/sc/source/core/data/formulacell.cxx"))
+            return;
         TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
     }
 
@@ -58,17 +74,33 @@ bool ExpressionAlwaysZero::VisitBinaryOperator( BinaryOperator const * binaryOpe
         return true;
     if (binaryOperator->getLocStart().isMacroID())
         return true;
-    if (binaryOperator->getOpcode() != BO_And)
+
+    bool bAndOperator = false;
+    auto op = binaryOperator->getOpcode();
+    if (op == BO_And || op == BO_AndAssign || op == BO_LAnd)
+        bAndOperator = true;
+    else if (op == BO_Mul || op == BO_MulAssign)
+        ; // ok
+    else
         return true;
+
     auto lhsValue = getExprValue(binaryOperator->getLHS());
     auto rhsValue = getExprValue(binaryOperator->getRHS());
-    if (!lhsValue || !rhsValue || (lhsValue->getExtValue() & rhsValue->getExtValue()) != 0)
+    if (lhsValue && lhsValue->getExtValue() == 0)
+        ; // ok
+    else if (rhsValue && rhsValue->getExtValue() == 0)
+        ; // ok
+    else if (bAndOperator && lhsValue && rhsValue && (lhsValue->getExtValue() & rhsValue->getExtValue()) == 0)
+        ; // ok
+    else if (!bAndOperator && lhsValue && rhsValue && (lhsValue->getExtValue() * rhsValue->getExtValue()) == 0)
+        ; // ok
+    else
         return true;
     report(
         DiagnosticsEngine::Warning, "expression always evaluates to zero, lhs=%0 rhs=%1",
         binaryOperator->getLocStart())
-        << lhsValue->toString(10)
-        << rhsValue->toString(10)
+        << (lhsValue ? lhsValue->toString(10) : "unknown")
+        << (rhsValue ? rhsValue->toString(10) : "unknown")
         << binaryOperator->getSourceRange();
     return true;
 }
@@ -79,19 +111,35 @@ bool ExpressionAlwaysZero::VisitCXXOperatorCallExpr( CXXOperatorCallExpr const *
         return true;
     if (cxxOperatorCallExpr->getLocStart().isMacroID())
         return true;
-    if (cxxOperatorCallExpr->getOperator() != clang::OverloadedOperatorKind::OO_Amp)
+
+    bool bAndOperator = false;
+    auto op = cxxOperatorCallExpr->getOperator();
+    if ( op == OO_Amp || op == OO_AmpEqual || op == OO_AmpAmp)
+        bAndOperator = true;
+    else if (op == OO_Star || op == OO_StarEqual)
+        ; // ok
+    else
         return true;
+
     if (cxxOperatorCallExpr->getNumArgs() != 2)
         return true;
     auto lhsValue = getExprValue(cxxOperatorCallExpr->getArg(0));
     auto rhsValue = getExprValue(cxxOperatorCallExpr->getArg(1));
-    if (!lhsValue || !rhsValue || (lhsValue->getExtValue() & rhsValue->getExtValue()) != 0)
+    if (lhsValue && lhsValue->getExtValue() == 0)
+        ; // ok
+    else if (rhsValue && rhsValue->getExtValue() == 0)
+        ; // ok
+    else if (bAndOperator && lhsValue && rhsValue && (lhsValue->getExtValue() & rhsValue->getExtValue()) == 0)
+        ; // ok
+    else if (!bAndOperator && lhsValue && rhsValue && (lhsValue->getExtValue() * rhsValue->getExtValue()) == 0)
+        ; // ok
+    else
         return true;
     report(
         DiagnosticsEngine::Warning, "expression always evaluates to zero, lhs=%0 rhs=%1",
         cxxOperatorCallExpr->getLocStart())
-        << lhsValue->toString(10)
-        << rhsValue->toString(10)
+        << (lhsValue ? lhsValue->toString(10) : "unknown")
+        << (rhsValue ? rhsValue->toString(10) : "unknown")
         << cxxOperatorCallExpr->getSourceRange();
     return true;
 }
diff --git a/compilerplugins/clang/test/expressionalwayszero.cxx b/compilerplugins/clang/test/expressionalwayszero.cxx
index 06bd80a2d90a..4986e5d690f7 100644
--- a/compilerplugins/clang/test/expressionalwayszero.cxx
+++ b/compilerplugins/clang/test/expressionalwayszero.cxx
@@ -20,6 +20,14 @@ namespace o3tl {
     template<> struct typed_flags<Enum1> : is_typed_flags<Enum1, 0x3> {};
 }
 
+enum class Enum2 {
+    ZERO = 0,
+    ONE = 1,
+    TWO = 2,
+};
+namespace o3tl {
+    template<> struct typed_flags<Enum2> : is_typed_flags<Enum2, 0x3> {};
+}
 
 int main()
 {
@@ -27,5 +35,12 @@ int main()
     (void)v1;
     auto v2 = Enum1::ONE & Enum1::TWO; // expected-error {{expression always evaluates to zero, lhs=1 rhs=2 [loplugin:expressionalwayszero]}}
     (void)v2;
+
+    auto v3 = Enum2::ONE;
+    auto v4 = v3 & Enum2::ZERO; // expected-error {{expression always evaluates to zero, lhs=unknown rhs=0 [loplugin:expressionalwayszero]}}
+    (void)v4;
+
+    auto v5 = Enum2::ONE;
+    v5 &= Enum2::ZERO; // expected-error {{expression always evaluates to zero, lhs=unknown rhs=0 [loplugin:expressionalwayszero]}}
 }
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/desktop/source/deployment/misc/dp_update.cxx b/desktop/source/deployment/misc/dp_update.cxx
index fc11d261baef..605ace6a5f44 100644
--- a/desktop/source/deployment/misc/dp_update.cxx
+++ b/desktop/source/deployment/misc/dp_update.cxx
@@ -125,7 +125,7 @@ void getOwnUpdateInfos(
         }
         else
         {
-            bAllHaveOwnUpdateInformation &= false;
+            bAllHaveOwnUpdateInformation = false;
         }
     }
     out_allFound = bAllHaveOwnUpdateInformation;
diff --git a/sd/source/ui/func/fucon3d.cxx b/sd/source/ui/func/fucon3d.cxx
index aec8839e18c6..5fe097765688 100644
--- a/sd/source/ui/func/fucon3d.cxx
+++ b/sd/source/ui/func/fucon3d.cxx
@@ -134,7 +134,7 @@ E3dCompoundObject* FuConstruct3dObject::ImpCreateBasic3DShape()
             aXPoly.Insert(0, Point (500*5, 1250*5), PolyFlags::Normal);
             aXPoly.Insert(0, Point (250*5, 1250*5), PolyFlags::Normal);
             aXPoly.Insert(0, Point (50*5, 1250*5), PolyFlags::Normal);
-            aXPoly.Insert(0, Point (0*5, 1250*5), PolyFlags::Normal);
+            aXPoly.Insert(0, Point (0,    1250*5), PolyFlags::Normal);
 
             ::basegfx::B2DPolygon aB2DPolygon(aXPoly.getB2DPolygon());
             if(aB2DPolygon.areControlPointsUsed())
@@ -175,7 +175,7 @@ E3dCompoundObject* FuConstruct3dObject::ImpCreateBasic3DShape()
             aInnerPoly.append(::basegfx::B2DPoint(200*5, -1000*5));
             aInnerPoly.append(::basegfx::B2DPoint(100*5, -1000*5));
             aInnerPoly.append(::basegfx::B2DPoint(50*5, -1000*5));
-            aInnerPoly.append(::basegfx::B2DPoint(0*5, -1000*5));
+            aInnerPoly.append(::basegfx::B2DPoint(0,    -1000*5));
             aInnerPoly.setClosed(true);
 
             p3DObj = new E3dLatheObj(mpView->Get3DDefaultAttributes(), ::basegfx::B2DPolyPolygon(aInnerPoly));
@@ -199,7 +199,7 @@ E3dCompoundObject* FuConstruct3dObject::ImpCreateBasic3DShape()
             aInnerPoly.append(::basegfx::B2DPoint(200*5, 1000*5));
             aInnerPoly.append(::basegfx::B2DPoint(100*5, 1000*5));
             aInnerPoly.append(::basegfx::B2DPoint(50*5, 1000*5));
-            aInnerPoly.append(::basegfx::B2DPoint(0*5, 1000*5));
+            aInnerPoly.append(::basegfx::B2DPoint(0,    1000*5));
             aInnerPoly.setClosed(true);
 
             p3DObj = new E3dLatheObj(mpView->Get3DDefaultAttributes(), ::basegfx::B2DPolyPolygon(aInnerPoly));
diff --git a/sdext/source/presenter/PresenterToolBar.cxx b/sdext/source/presenter/PresenterToolBar.cxx
index e01fdf096887..c9070fd6f3db 100644
--- a/sdext/source/presenter/PresenterToolBar.cxx
+++ b/sdext/source/presenter/PresenterToolBar.cxx
@@ -57,8 +57,6 @@ using namespace ::com::sun::star::drawing::framework;
 namespace sdext { namespace presenter {
 
 static const sal_Int32 gnGapSize (20);
-static const sal_Int32 gnMinimalSeparatorSize (20);
-static const sal_Int32 gnSeparatorInset (0);
 
 namespace {
 
@@ -1902,11 +1900,6 @@ void VerticalSeparator::Paint (
             PresenterCanvasHelper::SetDeviceColor(aRenderState, pFont->mnColor);
     }
 
-    if (aBBox.Height >= gnMinimalSeparatorSize + 2*gnSeparatorInset)
-    {
-        aBBox.Height -= 2*gnSeparatorInset;
-        aBBox.Y += gnSeparatorInset;
-    }
     rxCanvas->fillPolyPolygon(
         PresenterGeometryHelper::CreatePolygon(aBBox, rxCanvas->getDevice()),
         rViewState,
@@ -1952,11 +1945,6 @@ void HorizontalSeparator::Paint (
             PresenterCanvasHelper::SetDeviceColor(aRenderState, pFont->mnColor);
     }
 
-    if (aBBox.Width >= gnMinimalSeparatorSize+2*gnSeparatorInset)
-    {
-        aBBox.Width -= 2*gnSeparatorInset;
-        aBBox.X += gnSeparatorInset;
-    }
     rxCanvas->fillPolyPolygon(
         PresenterGeometryHelper::CreatePolygon(aBBox, rxCanvas->getDevice()),
         rViewState,
diff --git a/vcl/source/fontsubset/cff.cxx b/vcl/source/fontsubset/cff.cxx
index 3c759cddd067..13ed076d1ff6 100644
--- a/vcl/source/fontsubset/cff.cxx
+++ b/vcl/source/fontsubset/cff.cxx
@@ -533,12 +533,10 @@ void CffSubsetterContext::readDictOp()
         read2push();
     } else if( c == 29 ) {      // longint
         ++mpReadPtr;            // skip 29
-        int nS32 = mpReadPtr[0] << 24;
+        sal_Int32 nS32 = mpReadPtr[0] << 24;
         nS32 += mpReadPtr[1] << 16;
         nS32 += mpReadPtr[2] << 8;
         nS32 += mpReadPtr[3] << 0;
-        if( (sizeof(nS32) != 4) && (nS32 & (1U<<31)))
-            nS32 |= (~0U) << 31;    // assuming 2s complement
         mpReadPtr += 4;
         ValType nVal = static_cast<ValType>(nS32);
         push( nVal );
@@ -558,9 +556,7 @@ void CffSubsetterContext::read2push()
     const U8*& p = mpReadPtr;
     const U8 c = *p;
     if( c == 28 ) {
-        short nS16 = (p[1] << 8) + p[2];
-        if( (sizeof(nS16) != 2) && (nS16 & (1<<15)))
-            nS16 |= (~0U) << 15;    // assuming 2s complement
+        sal_Int16 nS16 = (p[1] << 8) + p[2];
         aVal = nS16;
         p += 3;
     } else if( c <= 246 ) {     // -107..+107
diff --git a/vcl/source/gdi/animate.cxx b/vcl/source/gdi/animate.cxx
index e1a96c7434a0..d3c5031441f7 100644
--- a/vcl/source/gdi/animate.cxx
+++ b/vcl/source/gdi/animate.cxx
@@ -27,7 +27,6 @@
 #include <impanmvw.hxx>
 
 #define MIN_TIMEOUT 2
-#define INC_TIMEOUT 0
 
 sal_uLong Animation::mnAnimCount = 0;
 
@@ -318,7 +317,7 @@ void Animation::Draw( OutputDevice* pOut, const Point& rDestPt, const Size& rDes
 
 void Animation::ImplRestartTimer( sal_uLong nTimeout )
 {
-    maTimer.SetTimeout( std::max( nTimeout, static_cast<sal_uLong>(MIN_TIMEOUT + ( mnAnimCount - 1 ) * INC_TIMEOUT) ) * 10 );
+    maTimer.SetTimeout( std::max( nTimeout, static_cast<sal_uLong>(MIN_TIMEOUT) ) * 10 );
     maTimer.Start();
 }
 
diff --git a/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx b/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx
index 2487d37861fe..aad24e69a544 100644
--- a/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx
+++ b/xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx
@@ -636,7 +636,7 @@ void DigitalSignaturesDialog::ImplFillSignaturesBox()
                 maSignatureManager.maCurrentSignatureInformations[n])))
             {
                 aImage = m_pSigsNotvalidatedImg->GetImage();
-                bAllNewSignatures &= false;
+                bAllNewSignatures = false;
             }
             else if (maSignatureManager.meSignatureMode == DocumentSignatureMode::Content
                 && bSigValid && bCertValid && DocumentSignatureHelper::isOOo3_2_Signature(