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>

17 files changed, 95 insertions, 34 deletions

diff --git a/chart2/source/controller/drawinglayer/DrawViewWrapper.cxx b/chart2/source/controller/drawinglayer/DrawViewWrapper.cxx
index 9ed6517cf7df..5efd976dcef5 100644
--- a/chart2/source/controller/drawinglayer/DrawViewWrapper.cxx
+++ b/chart2/source/controller/drawinglayer/DrawViewWrapper.cxx
@@ -72,7 +72,7 @@ SfxObjectShell * lcl_GetParentObjectShell( const uno::Reference< frame::XModel >
             {
                 SvGlobalName aSfxIdent( SFX_GLOBAL_CLASSID );
                 pResult = reinterpret_cast< SfxObjectShell * >(
-                    xParentTunnel->getSomething( uno::Sequence< sal_Int8 >( aSfxIdent.GetByteSequence() ) ) );
+                    xParentTunnel->getSomething( aSfxIdent.GetByteSequence() ) );
             }
         }
     }
diff --git a/chart2/source/controller/main/ChartController.cxx b/chart2/source/controller/main/ChartController.cxx
index 8ad8cb32ce3b..da8d9db101e9 100644
--- a/chart2/source/controller/main/ChartController.cxx
+++ b/chart2/source/controller/main/ChartController.cxx
@@ -1537,7 +1537,7 @@ void ChartController::impl_initializeAccessible( const uno::Reference< lang::XIn
     {
         uno::Sequence< uno::Any > aArguments(5);
         aArguments[0] <<= uno::Reference<view::XSelectionSupplier>(this);
-        aArguments[1] <<= uno::Reference<frame::XModel>(getModel());
+        aArguments[1] <<= getModel();
         aArguments[2] <<= m_xChartView;
         uno::Reference< XAccessible > xParent;
         {
diff --git a/compilerplugins/clang/redundantfcast.cxx b/compilerplugins/clang/redundantfcast.cxx
index 87d656c6d237..811c6d48647c 100644
--- a/compilerplugins/clang/redundantfcast.cxx
+++ b/compilerplugins/clang/redundantfcast.cxx
@@ -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)
     {
         if (ignoreLocation(expr))
-        {
             return true;
-        }
         auto const t1 = expr->getTypeAsWritten();
         auto const t2 = compat::getSubExprAsWritten(expr)->getType();
         if (t1.getCanonicalType().getTypePtr() != t2.getCanonicalType().getTypePtr())
diff --git a/compilerplugins/clang/test/redundantfcast.cxx b/compilerplugins/clang/test/redundantfcast.cxx
index f642098ed00f..609b787347e1 100644
--- a/compilerplugins/clang/test/redundantfcast.cxx
+++ b/compilerplugins/clang/test/redundantfcast.cxx
@@ -14,7 +14,14 @@
 #include "rtl/ustring.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()
 {
@@ -34,7 +41,7 @@ int main()
     OUString s1;
     method1(OUString(
         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;
     s2 = OUString(
         s1); // expected-error@-1 {{redundant functional cast from 'rtl::OUString' to 'rtl::OUString' [loplugin:redundantfcast]}}
@@ -43,6 +50,10 @@ int main()
     Color col2 = Color(
         col1); // expected-error@-1 {{redundant functional cast from 'Color' to 'Color' [loplugin:redundantfcast]}}
     (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: */
diff --git a/sc/source/core/data/olinetab.cxx b/sc/source/core/data/olinetab.cxx
index f43a873556ab..6ec7d071e6d7 100644
--- a/sc/source/core/data/olinetab.cxx
+++ b/sc/source/core/data/olinetab.cxx
@@ -153,7 +153,7 @@ ScOutlineArray::ScOutlineArray( const ScOutlineArray& rArray ) :
         for (; it != itEnd; ++it)
         {
             const ScOutlineEntry *const pEntry = &it->second;
-            aCollections[nLevel].insert(ScOutlineEntry(*pEntry));
+            aCollections[nLevel].insert(*pEntry);
         }
     }
 }
diff --git a/sc/source/core/tool/token.cxx b/sc/source/core/tool/token.cxx
index de5abccd1ae1..868e92928d6e 100644
--- a/sc/source/core/tool/token.cxx
+++ b/sc/source/core/tool/token.cxx
@@ -2139,7 +2139,7 @@ FormulaToken* ScTokenArray::MergeArray( )
                 }
                 else if ( t->GetType() == svString )
                 {
-                    pArray->PutString(svl::SharedString(t->GetString()), nCol, nRow);
+                    pArray->PutString(t->GetString(), nCol, nRow);
                 }
             break;
 
diff --git a/sc/source/ui/unoobj/cellsuno.cxx b/sc/source/ui/unoobj/cellsuno.cxx
index 12c9d94241ad..d68573a3264b 100644
--- a/sc/source/ui/unoobj/cellsuno.cxx
+++ b/sc/source/ui/unoobj/cellsuno.cxx
@@ -2114,7 +2114,7 @@ uno::Any SAL_CALL ScCellRangesBase::getPropertyDefault( const OUString& aPropert
                         break;
                     case SC_WID_UNO_NUMRULES:
                         {
-                            aAny <<= uno::Reference<container::XIndexReplace>(ScStyleObj::CreateEmptyNumberingRules());
+                            aAny <<= ScStyleObj::CreateEmptyNumberingRules();
                         }
                         break;
                 }
@@ -2601,7 +2601,7 @@ void ScCellRangesBase::GetOnePropertyValue( const SfxItemPropertySimpleEntry* pE
                 case SC_WID_UNO_NUMRULES:
                     {
                         // always return empty numbering rules object
-                        rAny <<= uno::Reference<container::XIndexReplace>(ScStyleObj::CreateEmptyNumberingRules());
+                        rAny <<= ScStyleObj::CreateEmptyNumberingRules();
                     }
                     break;
                 case SC_WID_UNO_ABSNAME:
diff --git a/sc/source/ui/unoobj/dapiuno.cxx b/sc/source/ui/unoobj/dapiuno.cxx
index 645d5f416e74..0f4af4a65662 100644
--- a/sc/source/ui/unoobj/dapiuno.cxx
+++ b/sc/source/ui/unoobj/dapiuno.cxx
@@ -1975,7 +1975,7 @@ Any SAL_CALL ScDataPilotFieldObj::getPropertyValue( const OUString& aPropertyNam
     {
         const DataPilotFieldAutoShowInfo* pInfo = getAutoShowInfo();
         if (pInfo)
-            aRet <<= DataPilotFieldAutoShowInfo(*pInfo);
+            aRet <<= *pInfo;
     }
     else if ( aPropertyName == SC_UNONAME_HASLAYOUTINFO )
         aRet <<= (getLayoutInfo() != nullptr);
@@ -1983,7 +1983,7 @@ Any SAL_CALL ScDataPilotFieldObj::getPropertyValue( const OUString& aPropertyNam
     {
         const DataPilotFieldLayoutInfo* pInfo = getLayoutInfo();
         if (pInfo)
-            aRet <<= DataPilotFieldLayoutInfo(*pInfo);
+            aRet <<= *pInfo;
     }
     else if ( aPropertyName == SC_UNONAME_HASREFERENCE )
         aRet <<= (getReference() != nullptr);
@@ -1991,7 +1991,7 @@ Any SAL_CALL ScDataPilotFieldObj::getPropertyValue( const OUString& aPropertyNam
     {
         const DataPilotFieldReference* pRef = getReference();
         if (pRef)
-            aRet <<= DataPilotFieldReference(*pRef);
+            aRet <<= *pRef;
     }
     else if ( aPropertyName == SC_UNONAME_HASSORTINFO )
         aRet <<= (getSortInfo() != nullptr);
@@ -1999,7 +1999,7 @@ Any SAL_CALL ScDataPilotFieldObj::getPropertyValue( const OUString& aPropertyNam
     {
         const DataPilotFieldSortInfo* pInfo = getSortInfo();
         if (pInfo)
-            aRet <<= DataPilotFieldSortInfo(*pInfo);
+            aRet <<= *pInfo;
     }
     else if ( aPropertyName == SC_UNONAME_ISGROUP )
         aRet <<= hasGroupInfo();
diff --git a/sc/source/ui/unoobj/targuno.cxx b/sc/source/ui/unoobj/targuno.cxx
index d3d2deb062de..96ba5208ebb9 100644
--- a/sc/source/ui/unoobj/targuno.cxx
+++ b/sc/source/ui/unoobj/targuno.cxx
@@ -230,7 +230,7 @@ void ScLinkTargetTypeObj::SetLinkTargetBitmap( uno::Any& rRet, sal_uInt16 nType
     if (nImgId != ScContentId::ROOT)
     {
         BitmapEx aBitmapEx(aContentBmps[static_cast<int>(nImgId) -1 ]);
-        rRet <<= uno::Reference< awt::XBitmap > (VCLUnoHelper::CreateBitmap(aBitmapEx));
+        rRet <<= VCLUnoHelper::CreateBitmap(aBitmapEx);
     }
 }
 
diff --git a/sd/source/ui/docshell/docshel4.cxx b/sd/source/ui/docshell/docshel4.cxx
index 2685d68d3c41..33d351e05d15 100644
--- a/sd/source/ui/docshell/docshel4.cxx
+++ b/sd/source/ui/docshell/docshel4.cxx
@@ -318,7 +318,7 @@ bool DrawDocShell::Load( SfxMedium& rMedium )
             SdPage* pPage = mpDoc->GetSdPage( 0, PageKind::Standard );
 
             if( pPage )
-                SetVisArea( ::tools::Rectangle( pPage->GetAllObjBoundRect() ) );
+                SetVisArea( pPage->GetAllObjBoundRect() );
         }
 
         FinishedLoading();
diff --git a/sfx2/source/doc/objembed.cxx b/sfx2/source/doc/objembed.cxx
index 3c0f660b74a5..5af5ee4e4575 100644
--- a/sfx2/source/doc/objembed.cxx
+++ b/sfx2/source/doc/objembed.cxx
@@ -55,7 +55,7 @@ SfxObjectShell* SfxObjectShell::GetParentShellByModel_Impl()
             {
                 SvGlobalName aSfxIdent( SFX_GLOBAL_CLASSID );
                 pResult = reinterpret_cast<SfxObjectShell*>(xParentTunnel->getSomething(
-                                                uno::Sequence< sal_Int8 >( aSfxIdent.GetByteSequence() ) ) );
+                                                aSfxIdent.GetByteSequence() ) );
             }
         }
     }
diff --git a/sfx2/source/sidebar/SidebarController.cxx b/sfx2/source/sidebar/SidebarController.cxx
index 5752120038a4..c57fbe2040c6 100644
--- a/sfx2/source/sidebar/SidebarController.cxx
+++ b/sfx2/source/sidebar/SidebarController.cxx
@@ -863,7 +863,7 @@ Reference<ui::XUIElement> SidebarController::CreateUIElement (
         Reference<ui::XUIElement> xUIElement(
             xUIElementFactory->createUIElement(
                 rsImplementationURL,
-                Sequence<beans::PropertyValue>(aCreationArguments.getPropertyValues())),
+                aCreationArguments.getPropertyValues()),
             UNO_QUERY_THROW);
 
         return xUIElement;
diff --git a/starmath/source/unomodel.cxx b/starmath/source/unomodel.cxx
index 63816bd3c525..39efdb4c8845 100644
--- a/starmath/source/unomodel.cxx
+++ b/starmath/source/unomodel.cxx
@@ -1058,7 +1058,7 @@ void SAL_CALL SmModel::setParent( const uno::Reference< uno::XInterface >& xPare
     {
         SvGlobalName aSfxIdent( SFX_GLOBAL_CLASSID );
         SfxObjectShell* pDoc = reinterpret_cast<SfxObjectShell *>(xParentTunnel->getSomething(
-                                        uno::Sequence< sal_Int8 >( aSfxIdent.GetByteSequence() ) ) );
+                                        aSfxIdent.GetByteSequence() ) );
         if ( pDoc )
             GetObjectShell()->OnDocumentPrinterChanged( pDoc->GetDocumentPrinter() );
     }
diff --git a/stoc/source/javaloader/javaloader.cxx b/stoc/source/javaloader/javaloader.cxx
index cc2b74aae0c7..b51446c639fa 100644
--- a/stoc/source/javaloader/javaloader.cxx
+++ b/stoc/source/javaloader/javaloader.cxx
@@ -259,8 +259,7 @@ const css::uno::Reference<XImplementationLoader> & JavaComponentLoader::getJavaL
         css::uno::Reference<XInitialization> javaLoader_XInitialization(m_javaLoader, UNO_QUERY_THROW);
 
         Any any;
-        any <<= css::uno::Reference<XMultiComponentFactory>(
-            m_xComponentContext->getServiceManager());
+        any <<= m_xComponentContext->getServiceManager();
 
         javaLoader_XInitialization->initialize(Sequence<Any>(&any, 1));
     }
diff --git a/vcl/source/filter/wmf/wmfwr.cxx b/vcl/source/filter/wmf/wmfwr.cxx
index 575f8bf0874e..3c8c1240384e 100644
--- a/vcl/source/filter/wmf/wmfwr.cxx
+++ b/vcl/source/filter/wmf/wmfwr.cxx
@@ -1003,7 +1003,7 @@ void WMFWriter::HandleLineInfoPolyPolygons(const LineInfo& rInfo, const basegfx:
             for(sal_uInt32 a(0); a < aFillPolyPolygon.count(); a++)
             {
                 const tools::Polygon aPolygon(aFillPolyPolygon.getB2DPolygon(a));
-                WMFRecord_Polygon( tools::Polygon(aPolygon) );
+                WMFRecord_Polygon( aPolygon );
             }
 
             aSrcLineColor = aOldLineColor;
diff --git a/vcl/source/gdi/dibtools.cxx b/vcl/source/gdi/dibtools.cxx
index 810fa381b4d5..3b8b9d9b11b2 100644
--- a/vcl/source/gdi/dibtools.cxx
+++ b/vcl/source/gdi/dibtools.cxx
@@ -385,10 +385,10 @@ bool ImplDecodeRLE(sal_uInt8* pBuffer, DIBV5Header const & rHeader, BitmapWriteA
                         cTmp = *pRLE++;
 
                         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 )
-                            rAcc.SetPixelOnData(pScanline, nX++, BitmapColor(SanitizePaletteIndex(cTmp & 0x0f, rPalette, bForceToMonoWhileReading)));
+                            rAcc.SetPixelOnData(pScanline, nX++, SanitizePaletteIndex(cTmp & 0x0f, rPalette, bForceToMonoWhileReading));
                     }
 
                     if( nRunByte & 1 )
@@ -397,7 +397,7 @@ bool ImplDecodeRLE(sal_uInt8* pBuffer, DIBV5Header const & rHeader, BitmapWriteA
                             return false;
 
                         if( nX < nWidth )
-                            rAcc.SetPixelOnData(pScanline, nX++, BitmapColor(SanitizePaletteIndex(*pRLE >> 4, rPalette, bForceToMonoWhileReading)));
+                            rAcc.SetPixelOnData(pScanline, nX++, SanitizePaletteIndex(*pRLE >> 4, rPalette, bForceToMonoWhileReading));
 
                         pRLE++;
                     }
@@ -418,7 +418,7 @@ bool ImplDecodeRLE(sal_uInt8* pBuffer, DIBV5Header const & rHeader, BitmapWriteA
                             return false;
 
                         if( nX < nWidth )
-                            rAcc.SetPixelOnData(pScanline, nX++, BitmapColor(SanitizePaletteIndex(*pRLE, rPalette, bForceToMonoWhileReading)));
+                            rAcc.SetPixelOnData(pScanline, nX++, SanitizePaletteIndex(*pRLE, rPalette, bForceToMonoWhileReading));
 
                         pRLE++;
                     }
@@ -466,19 +466,19 @@ bool ImplDecodeRLE(sal_uInt8* pBuffer, DIBV5Header const & rHeader, BitmapWriteA
                 for( sal_uLong i = 0; i < nRunByte; i++ )
                 {
                     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 )
-                        rAcc.SetPixelOnData(pScanline, nX++, BitmapColor(SanitizePaletteIndex(cTmp & 0x0f, rPalette, bForceToMonoWhileReading)));
+                        rAcc.SetPixelOnData(pScanline, nX++, SanitizePaletteIndex(cTmp & 0x0f, rPalette, bForceToMonoWhileReading));
                 }
 
                 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
             {
                 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;
-                            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;
-                            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++ )
                         {
                             auto nIndex = *pTmp++;
-                            rAcc.SetPixelOnData(pScanline, nX, BitmapColor(SanitizePaletteIndex(nIndex, rPalette, bForceToMonoWhileReading)));
+                            rAcc.SetPixelOnData(pScanline, nX, SanitizePaletteIndex(nIndex, rPalette, bForceToMonoWhileReading));
                         }
                     }
                 }
diff --git a/writerperfect/source/writer/exp/xmlimp.cxx b/writerperfect/source/writer/exp/xmlimp.cxx
index 812b27e7b69b..f64a688de256 100644
--- a/writerperfect/source/writer/exp/xmlimp.cxx
+++ b/writerperfect/source/writer/exp/xmlimp.cxx
@@ -282,7 +282,7 @@ void XMLOfficeDocContext::HandleFixedLayoutPage(const FixedLayoutPage &rPage, bo
 
     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);
     if (!xSVGWriter.is())