ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

loplugin:expressionalwayszero improvements

Browse Source 
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>
This commit is contained in:
Noel Grandin
2018-03-27 08:22:09 +02:00
parent 50af4bf5c6
commit 862dc17e43
8 changed files with 79 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compilerplugins/clang/expressionalwayszero.cxx
+56 -8
View File
@@ -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;
}
compilerplugins/clang/test/expressionalwayszero.cxx
+15
View File
@@ -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: */
desktop/source/deployment/misc/dp_update.cxx
+1 -1
View File
@@ -125,7 +125,7 @@ void getOwnUpdateInfos(
}
else
{
bAllHaveOwnUpdateInformation &= false;
bAllHaveOwnUpdateInformation = false;
}
}
out_allFound = bAllHaveOwnUpdateInformation;
sd/source/ui/func/fucon3d.cxx
+3 -3
View File
@@ -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));
sdext/source/presenter/PresenterToolBar.cxx
-12
View File
@@ -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,
vcl/source/fontsubset/cff.cxx
+2 -6
View File
@@ -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
vcl/source/gdi/animate.cxx
+1 -2
View File
@@ -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();
}
xmlsecurity/source/dialogs/digitalsignaturesdialog.cxx
+1 -1
View File
@@ -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(