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>

2 files changed, 71 insertions, 8 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: */