From 862dc17e437f0972223e18555110dc875d2ffa44 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Thu, 22 Mar 2018 10:01:51 +0200 Subject: loplugin:expressionalwayszero improvements Change-Id: I00bdbc58d2295a0be30b47c85eae6b9abfec17b2 Reviewed-on: https://gerrit.libreoffice.org/51868 Tested-by: Jenkins Reviewed-by: Noel Grandin --- compilerplugins/clang/expressionalwayszero.cxx | 64 +++++++++++++++++++--- .../clang/test/expressionalwayszero.cxx | 15 +++++ 2 files changed, 71 insertions(+), 8 deletions(-) (limited to 'compilerplugins/clang') 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 : is_typed_flags {}; } +enum class Enum2 { + ZERO = 0, + ONE = 1, + TWO = 2, +}; +namespace o3tl { + template<> struct typed_flags : is_typed_flags {}; +} 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: */ -- cgit v1.2.3