loplugin:redundantfcast look for unnecessary temporaries

when calling methods that take a const&

Change-Id: Idf45dfd9fea0de6fae0b1f89550f2f7fc302aa15
Reviewed-on: https://gerrit.libreoffice.org/50970
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
This commit is contained in:
Noel Grandin 2018-03-08 19:39:41 +02:00
parent 97e47e7b00
commit 2d40c43e86
17 changed files with 95 additions and 34 deletions

View File

@ -72,7 +72,7 @@ SfxObjectShell * lcl_GetParentObjectShell( const uno::Reference< frame::XModel >
{ {
SvGlobalName aSfxIdent( SFX_GLOBAL_CLASSID ); SvGlobalName aSfxIdent( SFX_GLOBAL_CLASSID );
pResult = reinterpret_cast< SfxObjectShell * >( pResult = reinterpret_cast< SfxObjectShell * >(
xParentTunnel->getSomething( uno::Sequence< sal_Int8 >( aSfxIdent.GetByteSequence() ) ) ); xParentTunnel->getSomething( aSfxIdent.GetByteSequence() ) );
} }
} }
} }

View File

@ -1537,7 +1537,7 @@ void ChartController::impl_initializeAccessible( const uno::Reference< lang::XIn
{ {
uno::Sequence< uno::Any > aArguments(5); uno::Sequence< uno::Any > aArguments(5);
aArguments[0] <<= uno::Reference<view::XSelectionSupplier>(this); aArguments[0] <<= uno::Reference<view::XSelectionSupplier>(this);
aArguments[1] <<= uno::Reference<frame::XModel>(getModel()); aArguments[1] <<= getModel();
aArguments[2] <<= m_xChartView; aArguments[2] <<= m_xChartView;
uno::Reference< XAccessible > xParent; uno::Reference< XAccessible > xParent;
{ {

View File

@ -21,12 +21,63 @@ public:
{ {
} }
/* Check for the creation of unnecessary temporaries when calling a method that takes a param by const & */
bool VisitCallExpr(CallExpr const* callExpr)
{
if (ignoreLocation(callExpr))
return true;
const FunctionDecl* functionDecl;
if (isa<CXXMemberCallExpr>(callExpr))
functionDecl = dyn_cast<CXXMemberCallExpr>(callExpr)->getMethodDecl();
else
functionDecl = callExpr->getDirectCallee();
if (!functionDecl)
return true;
unsigned len = std::min(callExpr->getNumArgs(), functionDecl->getNumParams());
for (unsigned i = 0; i < len; ++i)
{
// check if param is const&
auto param = functionDecl->getParamDecl(i);
auto lvalueType = param->getType()->getAs<LValueReferenceType>();
if (!lvalueType)
continue;
if (!lvalueType->getPointeeType().isConstQualified())
continue;
auto paramClassOrStructType = lvalueType->getPointeeType()->getAs<RecordType>();
if (!paramClassOrStructType)
continue;
// check for temporary and functional cast in argument expression
auto arg = callExpr->getArg(i)->IgnoreImpCasts();
auto materializeTemporaryExpr = dyn_cast<MaterializeTemporaryExpr>(arg);
if (!materializeTemporaryExpr)
continue;
auto functionalCast = dyn_cast<CXXFunctionalCastExpr>(
materializeTemporaryExpr->GetTemporaryExpr()->IgnoreImpCasts());
if (!functionalCast)
continue;
auto const t1 = functionalCast->getTypeAsWritten();
auto const t2 = compat::getSubExprAsWritten(functionalCast)->getType();
if (t1.getCanonicalType().getTypePtr() != t2.getCanonicalType().getTypePtr())
continue;
// Check that the underlying expression is of the same class/struct type as the param i.e. that we are not instantiating
// something useful
if (t1.getCanonicalType().getTypePtr() != paramClassOrStructType)
continue;
report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1",
arg->getExprLoc())
<< t2 << t1 << arg->getSourceRange();
report(DiagnosticsEngine::Note, "in call to method here", param->getLocation())
<< param->getSourceRange();
}
return true;
}
bool VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr const* expr) bool VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr const* expr)
{ {
if (ignoreLocation(expr)) if (ignoreLocation(expr))
{
return true; return true;
}
auto const t1 = expr->getTypeAsWritten(); auto const t1 = expr->getTypeAsWritten();
auto const t2 = compat::getSubExprAsWritten(expr)->getType(); auto const t2 = compat::getSubExprAsWritten(expr)->getType();
if (t1.getCanonicalType().getTypePtr() != t2.getCanonicalType().getTypePtr()) if (t1.getCanonicalType().getTypePtr() != t2.getCanonicalType().getTypePtr())

View File

@ -14,7 +14,14 @@
#include "rtl/ustring.hxx" #include "rtl/ustring.hxx"
#include "tools/color.hxx" #include "tools/color.hxx"
void method1(OUString const&); void method1(OUString const&); // expected-note {{in call to method here [loplugin:redundantfcast]}}
struct Foo
{
Foo(int) {}
};
void func1(Foo const& f); // expected-note {{in call to method here [loplugin:redundantfcast]}}
int main() int main()
{ {
@ -34,7 +41,7 @@ int main()
OUString s1; OUString s1;
method1(OUString( method1(OUString(
s1)); // expected-error@-1 {{redundant functional cast from 'rtl::OUString' to 'rtl::OUString' [loplugin:redundantfcast]}} s1)); // expected-error@-1 {{redundant functional cast from 'rtl::OUString' to 'rtl::OUString' [loplugin:redundantfcast]}}
// expected-error@-2 {{redundant functional cast from 'rtl::OUString' to 'rtl::OUString' [loplugin:redundantfcast]}}
OUString s2; OUString s2;
s2 = OUString( s2 = OUString(
s1); // expected-error@-1 {{redundant functional cast from 'rtl::OUString' to 'rtl::OUString' [loplugin:redundantfcast]}} s1); // expected-error@-1 {{redundant functional cast from 'rtl::OUString' to 'rtl::OUString' [loplugin:redundantfcast]}}
@ -43,6 +50,10 @@ int main()
Color col2 = Color( Color col2 = Color(
col1); // expected-error@-1 {{redundant functional cast from 'Color' to 'Color' [loplugin:redundantfcast]}} col1); // expected-error@-1 {{redundant functional cast from 'Color' to 'Color' [loplugin:redundantfcast]}}
(void)col2; (void)col2;
Foo foo(1);
func1(Foo(
foo)); // expected-error@-1 {{redundant functional cast from 'Foo' to 'Foo' [loplugin:redundantfcast]}}
} }
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

View File

@ -153,7 +153,7 @@ ScOutlineArray::ScOutlineArray( const ScOutlineArray& rArray ) :
for (; it != itEnd; ++it) for (; it != itEnd; ++it)
{ {
const ScOutlineEntry *const pEntry = &it->second; const ScOutlineEntry *const pEntry = &it->second;
aCollections[nLevel].insert(ScOutlineEntry(*pEntry)); aCollections[nLevel].insert(*pEntry);
} }
} }
} }

View File

@ -2139,7 +2139,7 @@ FormulaToken* ScTokenArray::MergeArray( )
} }
else if ( t->GetType() == svString ) else if ( t->GetType() == svString )
{ {
pArray->PutString(svl::SharedString(t->GetString()), nCol, nRow); pArray->PutString(t->GetString(), nCol, nRow);
} }
break; break;

View File

@ -2114,7 +2114,7 @@ uno::Any SAL_CALL ScCellRangesBase::getPropertyDefault( const OUString& aPropert
break; break;
case SC_WID_UNO_NUMRULES: case SC_WID_UNO_NUMRULES:
{ {
aAny <<= uno::Reference<container::XIndexReplace>(ScStyleObj::CreateEmptyNumberingRules()); aAny <<= ScStyleObj::CreateEmptyNumberingRules();
} }
break; break;
} }
@ -2601,7 +2601,7 @@ void ScCellRangesBase::GetOnePropertyValue( const SfxItemPropertySimpleEntry* pE
case SC_WID_UNO_NUMRULES: case SC_WID_UNO_NUMRULES:
{ {
// always return empty numbering rules object // always return empty numbering rules object
rAny <<= uno::Reference<container::XIndexReplace>(ScStyleObj::CreateEmptyNumberingRules()); rAny <<= ScStyleObj::CreateEmptyNumberingRules();
} }
break; break;
case SC_WID_UNO_ABSNAME: case SC_WID_UNO_ABSNAME:

View File

@ -1975,7 +1975,7 @@ Any SAL_CALL ScDataPilotFieldObj::getPropertyValue( const OUString& aPropertyNam
{ {
const DataPilotFieldAutoShowInfo* pInfo = getAutoShowInfo(); const DataPilotFieldAutoShowInfo* pInfo = getAutoShowInfo();
if (pInfo) if (pInfo)
aRet <<= DataPilotFieldAutoShowInfo(*pInfo); aRet <<= *pInfo;
} }
else if ( aPropertyName == SC_UNONAME_HASLAYOUTINFO ) else if ( aPropertyName == SC_UNONAME_HASLAYOUTINFO )
aRet <<= (getLayoutInfo() != nullptr); aRet <<= (getLayoutInfo() != nullptr);
@ -1983,7 +1983,7 @@ Any SAL_CALL ScDataPilotFieldObj::getPropertyValue( const OUString& aPropertyNam
{ {
const DataPilotFieldLayoutInfo* pInfo = getLayoutInfo(); const DataPilotFieldLayoutInfo* pInfo = getLayoutInfo();
if (pInfo) if (pInfo)
aRet <<= DataPilotFieldLayoutInfo(*pInfo); aRet <<= *pInfo;
} }
else if ( aPropertyName == SC_UNONAME_HASREFERENCE ) else if ( aPropertyName == SC_UNONAME_HASREFERENCE )
aRet <<= (getReference() != nullptr); aRet <<= (getReference() != nullptr);
@ -1991,7 +1991,7 @@ Any SAL_CALL ScDataPilotFieldObj::getPropertyValue( const OUString& aPropertyNam
{ {
const DataPilotFieldReference* pRef = getReference(); const DataPilotFieldReference* pRef = getReference();
if (pRef) if (pRef)
aRet <<= DataPilotFieldReference(*pRef); aRet <<= *pRef;
} }
else if ( aPropertyName == SC_UNONAME_HASSORTINFO ) else if ( aPropertyName == SC_UNONAME_HASSORTINFO )
aRet <<= (getSortInfo() != nullptr); aRet <<= (getSortInfo() != nullptr);
@ -1999,7 +1999,7 @@ Any SAL_CALL ScDataPilotFieldObj::getPropertyValue( const OUString& aPropertyNam
{ {
const DataPilotFieldSortInfo* pInfo = getSortInfo(); const DataPilotFieldSortInfo* pInfo = getSortInfo();
if (pInfo) if (pInfo)
aRet <<= DataPilotFieldSortInfo(*pInfo); aRet <<= *pInfo;
} }
else if ( aPropertyName == SC_UNONAME_ISGROUP ) else if ( aPropertyName == SC_UNONAME_ISGROUP )
aRet <<= hasGroupInfo(); aRet <<= hasGroupInfo();

View File

@ -230,7 +230,7 @@ void ScLinkTargetTypeObj::SetLinkTargetBitmap( uno::Any& rRet, sal_uInt16 nType
if (nImgId != ScContentId::ROOT) if (nImgId != ScContentId::ROOT)
{ {
BitmapEx aBitmapEx(aContentBmps[static_cast<int>(nImgId) -1 ]); BitmapEx aBitmapEx(aContentBmps[static_cast<int>(nImgId) -1 ]);
rRet <<= uno::Reference< awt::XBitmap > (VCLUnoHelper::CreateBitmap(aBitmapEx)); rRet <<= VCLUnoHelper::CreateBitmap(aBitmapEx);
} }
} }

View File

@ -318,7 +318,7 @@ bool DrawDocShell::Load( SfxMedium& rMedium )
SdPage* pPage = mpDoc->GetSdPage( 0, PageKind::Standard ); SdPage* pPage = mpDoc->GetSdPage( 0, PageKind::Standard );
if( pPage ) if( pPage )
SetVisArea( ::tools::Rectangle( pPage->GetAllObjBoundRect() ) ); SetVisArea( pPage->GetAllObjBoundRect() );
} }
FinishedLoading(); FinishedLoading();

View File

@ -55,7 +55,7 @@ SfxObjectShell* SfxObjectShell::GetParentShellByModel_Impl()
{ {
SvGlobalName aSfxIdent( SFX_GLOBAL_CLASSID ); SvGlobalName aSfxIdent( SFX_GLOBAL_CLASSID );
pResult = reinterpret_cast<SfxObjectShell*>(xParentTunnel->getSomething( pResult = reinterpret_cast<SfxObjectShell*>(xParentTunnel->getSomething(
uno::Sequence< sal_Int8 >( aSfxIdent.GetByteSequence() ) ) ); aSfxIdent.GetByteSequence() ) );
} }
} }
} }

View File

@ -863,7 +863,7 @@ Reference<ui::XUIElement> SidebarController::CreateUIElement (
Reference<ui::XUIElement> xUIElement( Reference<ui::XUIElement> xUIElement(
xUIElementFactory->createUIElement( xUIElementFactory->createUIElement(
rsImplementationURL, rsImplementationURL,
Sequence<beans::PropertyValue>(aCreationArguments.getPropertyValues())), aCreationArguments.getPropertyValues()),
UNO_QUERY_THROW); UNO_QUERY_THROW);
return xUIElement; return xUIElement;

View File

@ -1058,7 +1058,7 @@ void SAL_CALL SmModel::setParent( const uno::Reference< uno::XInterface >& xPare
{ {
SvGlobalName aSfxIdent( SFX_GLOBAL_CLASSID ); SvGlobalName aSfxIdent( SFX_GLOBAL_CLASSID );
SfxObjectShell* pDoc = reinterpret_cast<SfxObjectShell *>(xParentTunnel->getSomething( SfxObjectShell* pDoc = reinterpret_cast<SfxObjectShell *>(xParentTunnel->getSomething(
uno::Sequence< sal_Int8 >( aSfxIdent.GetByteSequence() ) ) ); aSfxIdent.GetByteSequence() ) );
if ( pDoc ) if ( pDoc )
GetObjectShell()->OnDocumentPrinterChanged( pDoc->GetDocumentPrinter() ); GetObjectShell()->OnDocumentPrinterChanged( pDoc->GetDocumentPrinter() );
} }

View File

@ -259,8 +259,7 @@ const css::uno::Reference<XImplementationLoader> & JavaComponentLoader::getJavaL
css::uno::Reference<XInitialization> javaLoader_XInitialization(m_javaLoader, UNO_QUERY_THROW); css::uno::Reference<XInitialization> javaLoader_XInitialization(m_javaLoader, UNO_QUERY_THROW);
Any any; Any any;
any <<= css::uno::Reference<XMultiComponentFactory>( any <<= m_xComponentContext->getServiceManager();
m_xComponentContext->getServiceManager());
javaLoader_XInitialization->initialize(Sequence<Any>(&any, 1)); javaLoader_XInitialization->initialize(Sequence<Any>(&any, 1));
} }

View File

@ -1003,7 +1003,7 @@ void WMFWriter::HandleLineInfoPolyPolygons(const LineInfo& rInfo, const basegfx:
for(sal_uInt32 a(0); a < aFillPolyPolygon.count(); a++) for(sal_uInt32 a(0); a < aFillPolyPolygon.count(); a++)
{ {
const tools::Polygon aPolygon(aFillPolyPolygon.getB2DPolygon(a)); const tools::Polygon aPolygon(aFillPolyPolygon.getB2DPolygon(a));
WMFRecord_Polygon( tools::Polygon(aPolygon) ); WMFRecord_Polygon( aPolygon );
} }
aSrcLineColor = aOldLineColor; aSrcLineColor = aOldLineColor;

View File

@ -385,10 +385,10 @@ bool ImplDecodeRLE(sal_uInt8* pBuffer, DIBV5Header const & rHeader, BitmapWriteA
cTmp = *pRLE++; cTmp = *pRLE++;
if( nX < nWidth ) if( nX < nWidth )
rAcc.SetPixelOnData(pScanline, nX++, BitmapColor(SanitizePaletteIndex(cTmp >> 4, rPalette, bForceToMonoWhileReading))); rAcc.SetPixelOnData(pScanline, nX++, SanitizePaletteIndex(cTmp >> 4, rPalette, bForceToMonoWhileReading));
if( nX < nWidth ) if( nX < nWidth )
rAcc.SetPixelOnData(pScanline, nX++, BitmapColor(SanitizePaletteIndex(cTmp & 0x0f, rPalette, bForceToMonoWhileReading))); rAcc.SetPixelOnData(pScanline, nX++, SanitizePaletteIndex(cTmp & 0x0f, rPalette, bForceToMonoWhileReading));
} }
if( nRunByte & 1 ) if( nRunByte & 1 )
@ -397,7 +397,7 @@ bool ImplDecodeRLE(sal_uInt8* pBuffer, DIBV5Header const & rHeader, BitmapWriteA
return false; return false;
if( nX < nWidth ) if( nX < nWidth )
rAcc.SetPixelOnData(pScanline, nX++, BitmapColor(SanitizePaletteIndex(*pRLE >> 4, rPalette, bForceToMonoWhileReading))); rAcc.SetPixelOnData(pScanline, nX++, SanitizePaletteIndex(*pRLE >> 4, rPalette, bForceToMonoWhileReading));
pRLE++; pRLE++;
} }
@ -418,7 +418,7 @@ bool ImplDecodeRLE(sal_uInt8* pBuffer, DIBV5Header const & rHeader, BitmapWriteA
return false; return false;
if( nX < nWidth ) if( nX < nWidth )
rAcc.SetPixelOnData(pScanline, nX++, BitmapColor(SanitizePaletteIndex(*pRLE, rPalette, bForceToMonoWhileReading))); rAcc.SetPixelOnData(pScanline, nX++, SanitizePaletteIndex(*pRLE, rPalette, bForceToMonoWhileReading));
pRLE++; pRLE++;
} }
@ -466,19 +466,19 @@ bool ImplDecodeRLE(sal_uInt8* pBuffer, DIBV5Header const & rHeader, BitmapWriteA
for( sal_uLong i = 0; i < nRunByte; i++ ) for( sal_uLong i = 0; i < nRunByte; i++ )
{ {
if( nX < nWidth ) if( nX < nWidth )
rAcc.SetPixelOnData(pScanline, nX++, BitmapColor(SanitizePaletteIndex(cTmp >> 4, rPalette, bForceToMonoWhileReading))); rAcc.SetPixelOnData(pScanline, nX++, SanitizePaletteIndex(cTmp >> 4, rPalette, bForceToMonoWhileReading));
if( nX < nWidth ) if( nX < nWidth )
rAcc.SetPixelOnData(pScanline, nX++, BitmapColor(SanitizePaletteIndex(cTmp & 0x0f, rPalette, bForceToMonoWhileReading))); rAcc.SetPixelOnData(pScanline, nX++, SanitizePaletteIndex(cTmp & 0x0f, rPalette, bForceToMonoWhileReading));
} }
if( ( nCountByte & 1 ) && ( nX < nWidth ) ) if( ( nCountByte & 1 ) && ( nX < nWidth ) )
rAcc.SetPixelOnData(pScanline, nX++, BitmapColor(SanitizePaletteIndex(cTmp >> 4, rPalette, bForceToMonoWhileReading))); rAcc.SetPixelOnData(pScanline, nX++, SanitizePaletteIndex(cTmp >> 4, rPalette, bForceToMonoWhileReading));
} }
else else
{ {
for( sal_uLong i = 0; ( i < nCountByte ) && ( nX < nWidth ); i++ ) for( sal_uLong i = 0; ( i < nCountByte ) && ( nX < nWidth ); i++ )
rAcc.SetPixelOnData(pScanline, nX++, BitmapColor(SanitizePaletteIndex(cTmp, rPalette, bForceToMonoWhileReading))); rAcc.SetPixelOnData(pScanline, nX++, SanitizePaletteIndex(cTmp, rPalette, bForceToMonoWhileReading));
} }
} }
} }
@ -592,7 +592,7 @@ bool ImplReadDIBBits(SvStream& rIStm, DIBV5Header& rHeader, BitmapWriteAccess& r
} }
auto nIndex = (cTmp >> --nShift) & 1; auto nIndex = (cTmp >> --nShift) & 1;
rAcc.SetPixelOnData(pScanline, nX, BitmapColor(SanitizePaletteIndex(nIndex, rPalette, bForceToMonoWhileReading))); rAcc.SetPixelOnData(pScanline, nX, SanitizePaletteIndex(nIndex, rPalette, bForceToMonoWhileReading));
} }
} }
} }
@ -619,7 +619,7 @@ bool ImplReadDIBBits(SvStream& rIStm, DIBV5Header& rHeader, BitmapWriteAccess& r
} }
auto nIndex = (cTmp >> ( --nShift << 2 ) ) & 0x0f; auto nIndex = (cTmp >> ( --nShift << 2 ) ) & 0x0f;
rAcc.SetPixelOnData(pScanline, nX, BitmapColor(SanitizePaletteIndex(nIndex, rPalette, bForceToMonoWhileReading))); rAcc.SetPixelOnData(pScanline, nX, SanitizePaletteIndex(nIndex, rPalette, bForceToMonoWhileReading));
} }
} }
} }
@ -640,7 +640,7 @@ bool ImplReadDIBBits(SvStream& rIStm, DIBV5Header& rHeader, BitmapWriteAccess& r
for( long nX = 0; nX < nWidth; nX++ ) for( long nX = 0; nX < nWidth; nX++ )
{ {
auto nIndex = *pTmp++; auto nIndex = *pTmp++;
rAcc.SetPixelOnData(pScanline, nX, BitmapColor(SanitizePaletteIndex(nIndex, rPalette, bForceToMonoWhileReading))); rAcc.SetPixelOnData(pScanline, nX, SanitizePaletteIndex(nIndex, rPalette, bForceToMonoWhileReading));
} }
} }
} }

View File

@ -282,7 +282,7 @@ void XMLOfficeDocContext::HandleFixedLayoutPage(const FixedLayoutPage &rPage, bo
uno::Sequence<uno::Any> aArguments = uno::Sequence<uno::Any> aArguments =
{ {
uno::makeAny(uno::Sequence<beans::PropertyValue>({comphelper::makePropertyValue("DTDString", false)})) uno::makeAny<uno::Sequence<beans::PropertyValue>>({comphelper::makePropertyValue("DTDString", false)})
}; };
uno::Reference<svg::XSVGWriter> xSVGWriter(xCtx->getServiceManager()->createInstanceWithArgumentsAndContext("com.sun.star.svg.SVGWriter", aArguments, xCtx), uno::UNO_QUERY); uno::Reference<svg::XSVGWriter> xSVGWriter(xCtx->getServiceManager()->createInstanceWithArgumentsAndContext("com.sun.star.svg.SVGWriter", aArguments, xCtx), uno::UNO_QUERY);
if (!xSVGWriter.is()) if (!xSVGWriter.is())