From 7ab34b51f2d45137191145d31b4b0c7d18f577bf Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Wed, 16 May 2018 10:16:01 +0200 Subject: loplugin:redundantcast improvements for floating-integer conversions Change-Id: I63dbf18f144a792ae775fe6706da81657f790016 Reviewed-on: https://gerrit.libreoffice.org/54416 Reviewed-by: Stephan Bergmann Tested-by: Stephan Bergmann --- basic/source/comp/exprnode.cxx | 1 - compilerplugins/clang/redundantcast.cxx | 17 +++++++++++++++++ compilerplugins/clang/test/redundantcast.cxx | 10 ++++++++++ connectivity/source/drivers/dbase/DTable.cxx | 4 ++-- .../source/primitive3d/sdrextrudeprimitive3d.cxx | 6 +++++- hwpfilter/source/hwpreader.cxx | 14 +++++++------- libreofficekit/qa/tilebench/tilebench.cxx | 4 ++-- reportdesign/source/filter/xml/xmlControlProperty.cxx | 4 +++- sal/rtl/random.cxx | 7 +++++-- sccomp/source/solver/SwarmSolver.cxx | 5 +++-- sd/source/filter/eppt/pptx-epptooxml.cxx | 4 ++-- slideshow/source/engine/transitions/snakewipe.cxx | 13 +++++++++---- tools/source/generic/poly.cxx | 2 +- vcl/opengl/gdiimpl.cxx | 3 ++- vcl/opengl/scale.cxx | 4 +++- writerfilter/source/dmapper/DomainMapper_Impl.cxx | 4 +++- xmloff/source/forms/propertyimport.cxx | 7 ++++++- 17 files changed, 80 insertions(+), 29 deletions(-) diff --git a/basic/source/comp/exprnode.cxx b/basic/source/comp/exprnode.cxx index 55131fc1c14d..8d6db9bd88d8 100644 --- a/basic/source/comp/exprnode.cxx +++ b/basic/source/comp/exprnode.cxx @@ -155,7 +155,6 @@ void SbiExprNode::ConvertToIntConstIfPossible() double n; if( nVal >= SbxMININT && nVal <= SbxMAXINT && modf( nVal, &n ) == 0 ) { - nVal = static_cast(static_cast(nVal)); eType = SbxINTEGER; } } diff --git a/compilerplugins/clang/redundantcast.cxx b/compilerplugins/clang/redundantcast.cxx index 0faadc5a541a..66b81941e579 100644 --- a/compilerplugins/clang/redundantcast.cxx +++ b/compilerplugins/clang/redundantcast.cxx @@ -248,6 +248,23 @@ bool RedundantCast::VisitImplicitCastExpr(const ImplicitCastExpr * expr) { } } break; + case CK_FloatingToIntegral: + case CK_IntegralToFloating: + if (auto e = dyn_cast(expr->getSubExpr()->IgnoreParenImpCasts())) { + if ((isa(e) || isa(e)) + && (e->getSubExprAsWritten()->getType().getCanonicalType().getTypePtr() + == expr->getType().getCanonicalType().getTypePtr())) + { + report( + DiagnosticsEngine::Warning, + ("suspicious %select{static_cast|functional cast}0 from %1 to %2, result is" + " implicitly cast to %3"), + e->getExprLoc()) + << isa(e) << e->getSubExprAsWritten()->getType() + << e->getTypeAsWritten() << expr->getType() << expr->getSourceRange(); + } + } + break; default: break; } diff --git a/compilerplugins/clang/test/redundantcast.cxx b/compilerplugins/clang/test/redundantcast.cxx index 70fcdf3340cb..713a3be1433c 100644 --- a/compilerplugins/clang/test/redundantcast.cxx +++ b/compilerplugins/clang/test/redundantcast.cxx @@ -374,6 +374,15 @@ void testOverloadResolution() { (void) NonOverloadMemFn(&Overload::nonOverload); // expected-error {{redundant functional cast from 'void (Overload::*)()' to 'NonOverloadMemFn' (aka 'void (Overload::*)()') [loplugin:redundantcast]}} }; +void testIntermediaryStaticCast() { + int n = 0; + n = static_cast(n); // expected-error {{suspicious static_cast from 'int' to 'double', result is implicitly cast to 'int' [loplugin:redundantcast]}} + n = double(n); // expected-error {{suspicious functional cast from 'int' to 'double', result is implicitly cast to 'int' [loplugin:redundantcast]}} + double d = 0.0; + d = static_cast(d) + 1.0; // expected-error {{suspicious static_cast from 'double' to 'int', result is implicitly cast to 'double' [loplugin:redundantcast]}} + d = int(d) + 1.0; // expected-error {{suspicious functional cast from 'double' to 'int', result is implicitly cast to 'double' [loplugin:redundantcast]}} +}; + int main() { testConstCast(); testStaticCast(); @@ -382,6 +391,7 @@ int main() { testCStyleCastOfTemplateMethodResult(nullptr); testReinterpretConstCast(); testDynamicCast(); + testIntermediaryStaticCast(); } /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/connectivity/source/drivers/dbase/DTable.cxx b/connectivity/source/drivers/dbase/DTable.cxx index 815d45ba4635..f0112d35a03e 100644 --- a/connectivity/source/drivers/dbase/DTable.cxx +++ b/connectivity/source/drivers/dbase/DTable.cxx @@ -134,9 +134,9 @@ void lcl_CalcJulDate(sal_Int32& _nJulianDate,sal_Int32& _nJulianTime, const css: } // if ( rDateTime.Year <= 0 ) else { - _nJulianDate = static_cast( ((365.25 * iy0) + _nJulianDate = static_cast(365.25 * iy0) + static_cast(30.6001 * (im0 + 1)) - + aDateTime.Day + 1720994)); + + aDateTime.Day + 1720994; } double JD = _nJulianDate + 0.5; _nJulianDate = static_cast( JD + 0.5); diff --git a/drawinglayer/source/primitive3d/sdrextrudeprimitive3d.cxx b/drawinglayer/source/primitive3d/sdrextrudeprimitive3d.cxx index cf00f208b9b8..1921c71dac25 100644 --- a/drawinglayer/source/primitive3d/sdrextrudeprimitive3d.cxx +++ b/drawinglayer/source/primitive3d/sdrextrudeprimitive3d.cxx @@ -17,6 +17,10 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ +#include + +#include + #include #include #include @@ -62,7 +66,7 @@ namespace drawinglayer const double fFrontArea(basegfx::utils::getArea(aFirstPolygon)); const double fSqrtFrontArea(sqrt(fFrontArea)); double fRelativeTextureWidth = basegfx::fTools::equalZero(fSqrtFrontArea) ? 1.0 : fFrontLength / fSqrtFrontArea; - fRelativeTextureWidth = static_cast(static_cast(fRelativeTextureWidth - 0.5)); + fRelativeTextureWidth = std::trunc(fRelativeTextureWidth - 0.5); if(fRelativeTextureWidth < 1.0) { diff --git a/hwpfilter/source/hwpreader.cxx b/hwpfilter/source/hwpreader.cxx index 2b5e5145ae5b..d1e3d8d36691 100644 --- a/hwpfilter/source/hwpreader.cxx +++ b/hwpfilter/source/hwpreader.cxx @@ -4399,21 +4399,21 @@ void HwpReader::makePictureDRAW(HWPDrawingObject *drawobj, Picture * hbox) NaturalSpline(n, tarr, yarr, yb, carr, darr); } - sprintf(buf, "M%d %dC%d %d", WTSM(static_cast(xarr[0])), WTSM(static_cast(yarr[0])), - WTSM(static_cast(xarr[0] + xb[0]/3)), WTSM(static_cast(yarr[0] + yb[0]/3)) ); + sprintf(buf, "M%d %dC%d %d", WTSM(xarr[0]), WTSM(yarr[0]), + WTSM(xarr[0] + xb[0]/3), WTSM(yarr[0] + yb[0]/3) ); oustr += ascii(buf); for( i = 1 ; i < n ; i++ ){ if( i == n -1 ){ sprintf(buf, " %d %d %d %dz", - WTSM(static_cast(xarr[i] - xb[i]/3)), WTSM(static_cast(yarr[i] - yb[i]/3)), - WTSM(static_cast(xarr[i])), WTSM(static_cast(yarr[i])) ); + WTSM(xarr[i] - xb[i]/3), WTSM(yarr[i] - yb[i]/3), + WTSM(xarr[i]), WTSM(yarr[i]) ); } else{ sprintf(buf, " %d %d %d %d %d %d", - WTSM(static_cast(xarr[i] - xb[i]/3)), WTSM(static_cast(yarr[i] - yb[i]/3)), - WTSM(static_cast(xarr[i])), WTSM(static_cast(yarr[i])), - WTSM(static_cast(xarr[i]) + xb[i]/3), WTSM(static_cast(yarr[i] + yb[i]/3)) ); + WTSM(xarr[i] - xb[i]/3), WTSM(yarr[i] - yb[i]/3), + WTSM(xarr[i]), WTSM(yarr[i]), + WTSM(xarr[i] + xb[i]/3), WTSM(yarr[i] + yb[i]/3) ); } oustr += ascii(buf); diff --git a/libreofficekit/qa/tilebench/tilebench.cxx b/libreofficekit/qa/tilebench/tilebench.cxx index 3c62e2e36838..8bb41999f17c 100644 --- a/libreofficekit/qa/tilebench/tilebench.cxx +++ b/libreofficekit/qa/tilebench/tilebench.cxx @@ -155,7 +155,7 @@ int main( int argc, char* argv[] ) // Estimate the maximum tiles based on the number of parts requested, if Writer. if (pDocument->getDocumentType() == LOK_DOCTYPE_TEXT) - max_tiles = static_cast(ceil(max_parts * 16128. / nTilePixelHeight)) * ceil(static_cast(nWidth) / nTilePixelWidth); + max_tiles = static_cast(ceil(max_parts * 16128. / nTilePixelHeight) * ceil(static_cast(nWidth) / nTilePixelWidth)); fprintf(stderr, "Parts to render: %d, Total Parts: %d, Max parts: %d, Max tiles: %d\n", nParts, nTotalParts, max_parts, max_tiles); std::vector vBuffer(nTilePixelWidth * nTilePixelHeight * 4); @@ -210,7 +210,7 @@ int main( int argc, char* argv[] ) aTimes.emplace_back("render sub-regions at scale"); int nMaxTiles = max_tiles; if (pDocument->getDocumentType() == LOK_DOCTYPE_TEXT) - nMaxTiles = static_cast(ceil(max_parts * 16128. / nTileTwipHeight)) * ceil(static_cast(nWidth) / nTileTwipWidth); + nMaxTiles = static_cast(ceil(max_parts * 16128. / nTileTwipHeight) * ceil(static_cast(nWidth) / nTileTwipWidth)); int nTiles = 0; for (int nY = 0; nY < nHeight - 1; nY += nTileTwipHeight) { diff --git a/reportdesign/source/filter/xml/xmlControlProperty.cxx b/reportdesign/source/filter/xml/xmlControlProperty.cxx index 7eae2645c985..c232a44d6019 100644 --- a/reportdesign/source/filter/xml/xmlControlProperty.cxx +++ b/reportdesign/source/filter/xml/xmlControlProperty.cxx @@ -19,6 +19,7 @@ #include +#include #include #include "xmlControlProperty.hxx" @@ -271,7 +272,8 @@ Any OXMLControlProperty::convertString(const css::uno::Type& _rExpectedType, con { case TYPE_DATE: { - OSL_ENSURE((static_cast(nValue)) - nValue == 0, + double dummy; + OSL_ENSURE(std::modf(nValue, &dummy) == 0, "OPropertyImport::convertString: a Date value with a fractional part?"); aReturn <<= implGetDate(nValue); } diff --git a/sal/rtl/random.cxx b/sal/rtl/random.cxx index 158e8fff3cc5..c9269abb837b 100644 --- a/sal/rtl/random.cxx +++ b/sal/rtl/random.cxx @@ -17,6 +17,10 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ +#include + +#include + #include #include #include @@ -81,8 +85,7 @@ static double data(RandomData_Impl *pImpl) (static_cast(pImpl->m_nY) / 30269.0) + (static_cast(pImpl->m_nZ) / 30307.0) ); - random -= static_cast(static_cast(random)); - return random; + return std::modf(random, &random); } static bool initPool(RandomPool_Impl *pImpl) diff --git a/sccomp/source/solver/SwarmSolver.cxx b/sccomp/source/solver/SwarmSolver.cxx index dab7b648b8c0..2a549fb0e0a8 100644 --- a/sccomp/source/solver/SwarmSolver.cxx +++ b/sccomp/source/solver/SwarmSolver.cxx @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -370,7 +371,7 @@ double SwarmSolver::clampVariable(size_t nVarIndex, double fValue) double fResult = std::max(std::min(fValue, rBound.upper), rBound.lower); if (mbInteger) - return sal_Int64(fResult); + return std::trunc(fResult); return fResult; } @@ -389,7 +390,7 @@ double SwarmSolver::boundVariable(size_t nVarIndex, double fValue) } if (mbInteger) - return sal_Int64(fResult); + return std::trunc(fResult); return fResult; } diff --git a/sd/source/filter/eppt/pptx-epptooxml.cxx b/sd/source/filter/eppt/pptx-epptooxml.cxx index f33c62db57dd..13dd072460f0 100644 --- a/sd/source/filter/eppt/pptx-epptooxml.cxx +++ b/sd/source/filter/eppt/pptx-epptooxml.cxx @@ -1898,8 +1898,8 @@ bool PowerPointExport::WriteComments(sal_uInt32 nPageNum) FSEND); pFS->singleElementNS(XML_p, XML_pos, - XML_x, I64S((static_cast(57600*aRealPoint2D.X + 1270)/2540.0)), - XML_y, I64S((static_cast(57600*aRealPoint2D.Y + 1270)/2540.0)), + XML_x, I64S(static_cast((57600*aRealPoint2D.X + 1270)/2540.0)), + XML_y, I64S(static_cast((57600*aRealPoint2D.Y + 1270)/2540.0)), FSEND); pFS->startElementNS(XML_p, XML_text, diff --git a/slideshow/source/engine/transitions/snakewipe.cxx b/slideshow/source/engine/transitions/snakewipe.cxx index dd15d1238deb..b484f36cb8d7 100644 --- a/slideshow/source/engine/transitions/snakewipe.cxx +++ b/slideshow/source/engine/transitions/snakewipe.cxx @@ -17,6 +17,9 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ +#include + +#include #include #include @@ -87,7 +90,7 @@ SnakeWipe::SnakeWipe( sal_Int32 nElements, bool diagonal, bool flipOnYAxis ) if (in) { const double sqrtArea2 = sqrt( t * m_sqrtElements * m_sqrtElements ); const double edge = ::basegfx::pruneScaleValue( - static_cast( static_cast(sqrtArea2) ) / + std::trunc(sqrtArea2) / m_sqrtElements ); ::basegfx::B2DPolygon poly; @@ -99,7 +102,8 @@ SnakeWipe::SnakeWipe( sal_Int32 nElements, bool diagonal, bool flipOnYAxis ) res.append(poly); } const double a = (M_SQRT1_2 / m_sqrtElements); - const double d = (sqrtArea2 - static_cast(sqrtArea2)); + double dummy; + const double d = std::modf(sqrtArea2, &dummy); const double len = (t * M_SQRT2 * d); const double height = ::basegfx::pruneScaleValue( M_SQRT1_2 / m_sqrtElements ); poly.clear(); @@ -130,7 +134,7 @@ SnakeWipe::SnakeWipe( sal_Int32 nElements, bool diagonal, bool flipOnYAxis ) { const double sqrtArea2 = sqrt( t * m_sqrtElements * m_sqrtElements ); const double edge = ::basegfx::pruneScaleValue( - static_cast( static_cast(sqrtArea2) ) / + std::trunc(sqrtArea2) / m_sqrtElements ); ::basegfx::B2DPolygon poly; @@ -143,7 +147,8 @@ SnakeWipe::SnakeWipe( sal_Int32 nElements, bool diagonal, bool flipOnYAxis ) res.append(poly); } const double a = (M_SQRT1_2 / m_sqrtElements); - const double d = (sqrtArea2 - static_cast(sqrtArea2)); + double dummy; + const double d = std::modf(sqrtArea2, &dummy); const double len = ((1.0 - t) * M_SQRT2 * d); const double height = ::basegfx::pruneScaleValue( M_SQRT1_2 / m_sqrtElements ); poly.clear(); diff --git a/tools/source/generic/poly.cxx b/tools/source/generic/poly.cxx index ce9dabefa37e..5b00e6bc56a3 100644 --- a/tools/source/generic/poly.cxx +++ b/tools/source/generic/poly.cxx @@ -1329,7 +1329,7 @@ void Polygon::ImplReduceEdges( tools::Polygon& rPoly, const double& rArea, sal_u else if( fRelLen > 1.0 ) fRelLen = 1.0; - if( ( static_cast( ( ( fLenFact - 1.0 ) * 1000000.0 ) + 0.5 ) < fBound ) && + if( ( std::round( ( fLenFact - 1.0 ) * 1000000.0 ) < fBound ) && ( fabs( fGradB ) <= ( fRelLen * fBound * 0.01 ) ) ) { bDeletePoint = true; diff --git a/vcl/opengl/gdiimpl.cxx b/vcl/opengl/gdiimpl.cxx index df4996a6ca09..6858a2adbae7 100644 --- a/vcl/opengl/gdiimpl.cxx +++ b/vcl/opengl/gdiimpl.cxx @@ -39,6 +39,7 @@ #include #include +#include #include #include @@ -1071,7 +1072,7 @@ void OpenGLSalGraphicsImpl::DrawTransformedTexture( if( ixscale >= 2 && iyscale >= 2 ) // scale ratio less than 50% { areaScaling = true; - fastAreaScaling = ( ixscale == int( ixscale ) && iyscale == int( iyscale )); + fastAreaScaling = ( ixscale == std::trunc( ixscale ) && iyscale == std::trunc( iyscale )); // The generic case has arrays only up to 16 ratio downscaling and is performed in 2 passes, // when the ratio is in the 16-100 range, which is hopefully enough in practice, but protect // against buffer overflows in case such an extreme case happens (and in such case the precision diff --git a/vcl/opengl/scale.cxx b/vcl/opengl/scale.cxx index 433ad19f2e8b..a5280bf70817 100644 --- a/vcl/opengl/scale.cxx +++ b/vcl/opengl/scale.cxx @@ -19,6 +19,8 @@ #include +#include + #include #include @@ -203,7 +205,7 @@ bool OpenGLSalBitmap::ImplScaleArea( const rtl::Reference< OpenGLContext > &xCon double ixscale = 1 / rScaleX; double iyscale = 1 / rScaleY; - bool fast = ( ixscale == int( ixscale ) && iyscale == int( iyscale ) + bool fast = ( ixscale == std::trunc( ixscale ) && iyscale == std::trunc( iyscale ) && int( nNewWidth * ixscale ) == mnWidth && int( nNewHeight * iyscale ) == mnHeight ); bool bTwoPasses = false; diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx index f2274f0c646a..7194879807fd 100644 --- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx +++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx @@ -78,6 +78,7 @@ #include +#include #include #include #include @@ -4586,7 +4587,8 @@ static util::DateTime lcl_dateTimeFromSerial(const double& dSerial) DateTime d(Date(30, 12, 1899)); d.AddDays( static_cast(dSerial) ); - double frac = dSerial - static_cast(dSerial); + double dummy; + double frac = std::modf(dSerial, &dummy); sal_uInt32 seconds = frac * secondsPerDay; util::DateTime date; diff --git a/xmloff/source/forms/propertyimport.cxx b/xmloff/source/forms/propertyimport.cxx index 0851edf31519..679f1d5b3154 100644 --- a/xmloff/source/forms/propertyimport.cxx +++ b/xmloff/source/forms/propertyimport.cxx @@ -17,6 +17,10 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ +#include + +#include + #include "propertyimport.hxx" #include @@ -182,7 +186,8 @@ Any PropertyConversion::convertString( const css::uno::Type& _rExpectedType, { case TYPE_DATE: { - OSL_ENSURE((static_cast(nValue)) - nValue == 0, + double dummy; + OSL_ENSURE(std::modf(nValue, &dummy) == 0, "PropertyConversion::convertString: a Date value with a fractional part?"); aReturn <<= lcl_getDate(nValue); } -- cgit