loplugin:passstuffbyref improved returns

improve the detection of stuff we can return by const &, instead of by
copying

Change-Id: I479ae89d0413125a8295cc3cddbc0017ed61ed69
Reviewed-on: https://gerrit.libreoffice.org/46915
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

24 files changed, 176 insertions, 89 deletions

diff --git a/basegfx/source/polygon/b2dpolygontools.cxx b/basegfx/source/polygon/b2dpolygontools.cxx
index 489b77af92aa..e0a4721505ba 100644
--- a/basegfx/source/polygon/b2dpolygontools.cxx
+++ b/basegfx/source/polygon/b2dpolygontools.cxx
@@ -1645,7 +1645,7 @@ namespace basegfx
             };
         }
 
-        B2DPolygon createUnitPolygon()
+        B2DPolygon const & createUnitPolygon()
         {
             return theUnitPolygon::get();
         }
@@ -1719,7 +1719,7 @@ namespace basegfx
             };
         }
 
-        B2DPolygon createHalfUnitCircle()
+        B2DPolygon const & createHalfUnitCircle()
         {
             return theUnitHalfCircle::get();
         }
@@ -1751,7 +1751,7 @@ namespace basegfx
             };
         }
 
-        B2DPolygon createPolygonFromUnitCircle(sal_uInt32 nStartQuadrant)
+        B2DPolygon const & createPolygonFromUnitCircle(sal_uInt32 nStartQuadrant)
         {
             switch(nStartQuadrant % 4)
             {
diff --git a/basegfx/source/polygon/b3dpolypolygontools.cxx b/basegfx/source/polygon/b3dpolypolygontools.cxx
index d3ae28120e29..9c79bc14bc1f 100644
--- a/basegfx/source/polygon/b3dpolypolygontools.cxx
+++ b/basegfx/source/polygon/b3dpolypolygontools.cxx
@@ -100,7 +100,7 @@ namespace basegfx
             };
         }
 
-        B3DPolyPolygon createUnitCubePolyPolygon()
+        B3DPolyPolygon const & createUnitCubePolyPolygon()
         {
             return theUnitCubePolyPolygon::get();
         }
@@ -182,7 +182,7 @@ namespace basegfx
             };
         }
 
-        B3DPolyPolygon createUnitCubeFillPolyPolygon()
+        B3DPolyPolygon const & createUnitCubeFillPolyPolygon()
         {
             return theUnitCubeFillPolyPolygon::get();
         }
diff --git a/compilerplugins/clang/passstuffbyref.cxx b/compilerplugins/clang/passstuffbyref.cxx
index 8c522efb5dbe..5901a587594e 100644
--- a/compilerplugins/clang/passstuffbyref.cxx
+++ b/compilerplugins/clang/passstuffbyref.cxx
@@ -9,6 +9,7 @@
 
 #include <string>
 #include <set>
+#include <iostream>
 
 #include "check.hxx"
 #include "plugin.hxx"
@@ -211,7 +212,6 @@ static bool startswith(const std::string& rStr, const char* pSubStr) {
 }
 
 void PassStuffByRef::checkReturnValue(const FunctionDecl * functionDecl, const CXXMethodDecl * methodDecl) {
-
     if (methodDecl && (methodDecl->isVirtual() || methodDecl->hasAttr<OverrideAttr>())) {
         return;
     }
@@ -233,6 +233,7 @@ void PassStuffByRef::checkReturnValue(const FunctionDecl * functionDecl, const C
     if (isInUnoIncludeFile(functionDecl)) {
         return;
     }
+
     loplugin::DeclCheck dc(functionDecl);
     // function is passed as parameter to another function
     if (dc.Function("ImplColMonoFnc").Class("GDIMetaFile").GlobalNamespace()
@@ -274,21 +275,22 @@ void PassStuffByRef::checkReturnValue(const FunctionDecl * functionDecl, const C
     if (mbFoundReturnValueDisqualifier)
         return;
 
-    report(
-            DiagnosticsEngine::Warning,
-            "rather return %0 from function %1 by const& than by value, to avoid unnecessary copying",
+    report( DiagnosticsEngine::Warning,
+            "rather return %0 by const& than by value, to avoid unnecessary copying",
             functionDecl->getSourceRange().getBegin())
-        << type.getAsString() << functionDecl->getQualifiedNameAsString() << functionDecl->getSourceRange();
+        << type.getAsString() << functionDecl->getSourceRange();
 
     // display the location of the class member declaration so I don't have to search for it by hand
-    if (functionDecl->getSourceRange().getBegin() != functionDecl->getCanonicalDecl()->getSourceRange().getBegin())
+    auto canonicalDecl = functionDecl->getCanonicalDecl();
+    if (functionDecl != canonicalDecl)
     {
-        report(
-                DiagnosticsEngine::Note,
+        report( DiagnosticsEngine::Note,
                 "decl here",
-                functionDecl->getCanonicalDecl()->getSourceRange().getBegin())
-            << functionDecl->getCanonicalDecl()->getSourceRange();
+                canonicalDecl->getSourceRange().getBegin())
+            << canonicalDecl->getSourceRange();
     }
+
+    //functionDecl->dump();
 }
 
 bool PassStuffByRef::VisitReturnStmt(const ReturnStmt * returnStmt)
@@ -297,53 +299,103 @@ bool PassStuffByRef::VisitReturnStmt(const ReturnStmt * returnStmt)
         return true;
     const Expr* expr = dyn_cast<Expr>(*returnStmt->child_begin())->IgnoreParenCasts();
 
-    if (isReturnExprDisqualified(expr)) {
+    if (isReturnExprDisqualified(expr))
         mbFoundReturnValueDisqualifier = true;
-        return true;
-    }
+
     return true;
 }
 
+/**
+ * Does a return expression disqualify this method from doing return by const & ?
+ */
 bool PassStuffByRef::isReturnExprDisqualified(const Expr* expr)
 {
-    if (isa<ExprWithCleanups>(expr)) {
-        return true;
-    }
-    if (const CXXConstructExpr* constructExpr = dyn_cast<CXXConstructExpr>(expr))
+    while (true)
     {
-        if (constructExpr->getNumArgs()==1
-            && constructExpr->getConstructor()->isCopyOrMoveConstructor())
+        expr = expr->IgnoreParens();
+        if (auto implicitCast = dyn_cast<ImplicitCastExpr>(expr)) {
+            expr = implicitCast->getSubExpr();
+            continue;
+        }
+        if (auto exprWithCleanups = dyn_cast<ExprWithCleanups>(expr)) {
+            expr = exprWithCleanups->getSubExpr();
+            continue;
+        }
+        if (auto constructExpr = dyn_cast<CXXConstructExpr>(expr))
         {
-            expr = constructExpr->getArg(0)->IgnoreParenCasts();
+            if (constructExpr->getNumArgs()==1
+                && constructExpr->getConstructor()->isCopyOrMoveConstructor())
+            {
+                expr = constructExpr->getArg(0);
+                continue;
+            }
+            else
+                return true;
         }
-    }
-    if (isa<CXXConstructExpr>(expr)) {
-        return true;
-    }
-    if (const ArraySubscriptExpr* childExpr = dyn_cast<ArraySubscriptExpr>(expr)) {
-        expr = childExpr->getLHS();
-    }
-    if (const MemberExpr* memberExpr = dyn_cast<MemberExpr>(expr)) {
-        expr = memberExpr->getBase();
-    }
-    if (const DeclRefExpr* declRef = dyn_cast<DeclRefExpr>(expr)) {
-        const VarDecl* varDecl = dyn_cast<VarDecl>(declRef->getDecl());
-        if (varDecl) {
-            if (varDecl->getStorageDuration() == SD_Automatic
-                || varDecl->getStorageDuration() == SD_FullExpression ) {
+        if (isa<MaterializeTemporaryExpr>(expr)) {
+            return true;
+        }
+        if (isa<CXXBindTemporaryExpr>(expr)) {
+            return true;
+        }
+        expr = expr->IgnoreParenCasts();
+        if (auto childExpr = dyn_cast<ArraySubscriptExpr>(expr)) {
+            expr = childExpr->getLHS();
+            continue;
+        }
+        if (auto memberExpr = dyn_cast<MemberExpr>(expr)) {
+            expr = memberExpr->getBase();
+            continue;
+        }
+        if (auto declRef = dyn_cast<DeclRefExpr>(expr)) {
+            // a param might be a temporary
+            if (isa<ParmVarDecl>(declRef->getDecl()))
+                return true;
+            const VarDecl* varDecl = dyn_cast<VarDecl>(declRef->getDecl());
+            if (varDecl) {
+                if (varDecl->getStorageDuration() == SD_Thread
+                    || varDecl->getStorageDuration() == SD_Static ) {
+                    return false;
+                }
                 return true;
             }
-            return false;
         }
+        if (auto condOper = dyn_cast<ConditionalOperator>(expr)) {
+            return isReturnExprDisqualified(condOper->getTrueExpr())
+                || isReturnExprDisqualified(condOper->getFalseExpr());
+        }
+        if (auto operatorCallExpr = dyn_cast<CXXOperatorCallExpr>(expr)) {
+            // TODO could improve this, but sometimes it means we're returning a copy of a temporary.
+            // Same logic as CXXOperatorCallExpr::isAssignmentOp(), which our supported clang
+            // doesn't have yet.
+            auto Opc = operatorCallExpr->getOperator();
+            if (Opc == OO_Equal || Opc == OO_StarEqual ||
+                Opc == OO_SlashEqual || Opc == OO_PercentEqual ||
+                Opc == OO_PlusEqual || Opc == OO_MinusEqual ||
+                Opc == OO_LessLessEqual || Opc == OO_GreaterGreaterEqual ||
+                Opc == OO_AmpEqual || Opc == OO_CaretEqual ||
+                Opc == OO_PipeEqual)
+                return true;
+            if (Opc == OO_Subscript)
+            {
+                if (isReturnExprDisqualified(operatorCallExpr->getArg(0)))
+                    return true;
+                // otherwise fall through to the checking below
+            }
+        }
+        if (auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(expr)) {
+            if (isReturnExprDisqualified(memberCallExpr->getImplicitObjectArgument()))
+                return true;
+            // otherwise fall through to the checking in CallExpr
+        }
+        if (auto callExpr = dyn_cast<CallExpr>(expr)) {
+            FunctionDecl const * calleeFunctionDecl = callExpr->getDirectCallee();
+            if (!calleeFunctionDecl)
+                return true;
+            return !loplugin::TypeCheck(calleeFunctionDecl->getReturnType()).LvalueReference();
+        }
+        return false;
     }
-    if (const ConditionalOperator* condOper = dyn_cast<ConditionalOperator>(expr)) {
-        return isReturnExprDisqualified(condOper->getTrueExpr())
-            || isReturnExprDisqualified(condOper->getFalseExpr());
-    }
-    if (const CallExpr* callExpr = dyn_cast<CallExpr>(expr)) {
-        return !loplugin::TypeCheck(callExpr->getType()).Const().LvalueReference();
-    }
-    return true;
 }
 
 bool PassStuffByRef::VisitVarDecl(const VarDecl * varDecl)
@@ -388,7 +440,7 @@ bool PassStuffByRef::isPrimitiveConstRef(QualType type) {
 }
 
 
-loplugin::Plugin::Registration< PassStuffByRef > X("passstuffbyref");
+loplugin::Plugin::Registration< PassStuffByRef > X("passstuffbyref", false);
 
 }
 
diff --git a/compilerplugins/clang/test/passstuffbyref.cxx b/compilerplugins/clang/test/passstuffbyref.cxx
index 2f076e58e7b7..3178bf7329f2 100644
--- a/compilerplugins/clang/test/passstuffbyref.cxx
+++ b/compilerplugins/clang/test/passstuffbyref.cxx
@@ -8,31 +8,66 @@
  */
 
 #include <rtl/ustring.hxx>
+#include <sys/time.h>
+#include <vector>
 
-struct S {
+struct S1 {
+    OUString mv1;
+    OUString const & get() const { return mv1; }
+};
+struct S2 {
     OUString mv1;
     OUString mv2;
+    OUString mv3[2];
+    S1 child;
+    static OUString gs1;
 
     // make sure we ignore cases where the passed in parameter is std::move'd
-    S(OUString v1, OUString v2)
+    S2(OUString v1, OUString v2)
       : mv1(std::move(v1)), mv2((std::move(v2))) {}
+
+    OUString get1() { return mv1; } // expected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
+    OUString get2(bool b) { return b ? mv1 : mv2; } // expected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
+    OUString get3() { return child.mv1; } // expected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
+    OUString get4() { return mv3[0]; } // expected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
+    OUString get5() { return gs1; } // expected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
+    OUString const & get6() { return gs1; }
+    OUString get7() { return get6(); } // expected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
+    OUString & get8() { return gs1; }
+    OUString get9() { return get8(); } // expected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
+
+    // no warning expected
+    OUString set1() { return OUString("xxx"); }
+    OUString set2() { OUString v1("xxx"); return v1; }
+    OUString set3() { S1 v1; return v1.get(); }
+    OUString set4() { OUString v1[1]; return v1[0]; }
+    OUString set5(OUString const & s) { return s; }
+    OUString set6() { std::vector<OUString> v1(1); return v1[0]; }
+    OUString set7(S1 const & s) { return s.get(); }
 };
 
 
+// no warning expected
+
+timeval &operator -= ( timeval &t1, const timeval &t2 );
+timeval operator-( const timeval &t1, const timeval &t2 )
+{
+    timeval t0 = t1;
+    return t0 -= t2;
+}
+
 void f()
 {
-    S* s;
+    S2* s;
     OUString v1, v2;
-    s = new S(v1, v2);
+    s = new S2(v1, v2);
 }
 
-struct S2 { S2(int); };
+struct S3 { S3(int); };
 
-S2 f2() {
+S3 f2() {
     static int n;
     return n;
 }
 
-// expected-no-diagnostics
-
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/configmgr/source/childaccess.cxx b/configmgr/source/childaccess.cxx
index c45901fac623..a13cbc3b6e14 100644
--- a/configmgr/source/childaccess.cxx
+++ b/configmgr/source/childaccess.cxx
@@ -65,7 +65,7 @@ namespace
     class theChildAccessUnoTunnelId : public rtl::Static< UnoTunnelIdInit, theChildAccessUnoTunnelId > {};
 }
 
-css::uno::Sequence< sal_Int8 > ChildAccess::getTunnelId()
+css::uno::Sequence< sal_Int8 > const & ChildAccess::getTunnelId()
 {
     return theChildAccessUnoTunnelId::get().getSeq();
 }
diff --git a/configmgr/source/childaccess.hxx b/configmgr/source/childaccess.hxx
index 5a0ab480ec0d..eeb7eddeab62 100644
--- a/configmgr/source/childaccess.hxx
+++ b/configmgr/source/childaccess.hxx
@@ -54,7 +54,7 @@ class ChildAccess:
     public css::lang::XUnoTunnel
 {
 public:
-    static css::uno::Sequence< sal_Int8 > getTunnelId();
+    static css::uno::Sequence< sal_Int8 > const & getTunnelId();
 
     ChildAccess(
         Components & components, rtl::Reference< RootAccess > const & root,
diff --git a/configmgr/source/type.cxx b/configmgr/source/type.cxx
index 9924aef4d4d4..f0d109d74fcc 100644
--- a/configmgr/source/type.cxx
+++ b/configmgr/source/type.cxx
@@ -63,7 +63,7 @@ Type elementType(Type type) {
     }
 }
 
-css::uno::Type mapType(Type type) {
+css::uno::Type const & mapType(Type type) {
     switch (type) {
     case TYPE_ANY:
         return cppu::UnoType< css::uno::Any >::get();
diff --git a/configmgr/source/type.hxx b/configmgr/source/type.hxx
index 091c3f353cb4..8550796b22d3 100644
--- a/configmgr/source/type.hxx
+++ b/configmgr/source/type.hxx
@@ -39,7 +39,7 @@ bool isListType(Type type);
 
 Type elementType(Type type);
 
-css::uno::Type mapType(Type type);
+css::uno::Type const & mapType(Type type);
 
 Type getDynamicType(css::uno::Any const & value);
 
diff --git a/cppu/source/threadpool/threadpool.cxx b/cppu/source/threadpool/threadpool.cxx
index a3a3eab51d01..dcbe2ae0734d 100644
--- a/cppu/source/threadpool/threadpool.cxx
+++ b/cppu/source/threadpool/threadpool.cxx
@@ -53,7 +53,7 @@ namespace cppu_threadpool
         }
     };
 
-    DisposedCallerAdminHolder DisposedCallerAdmin::getInstance()
+    DisposedCallerAdminHolder const & DisposedCallerAdmin::getInstance()
     {
         return theDisposedCallerAdmin::get();
     }
diff --git a/cppu/source/threadpool/threadpool.hxx b/cppu/source/threadpool/threadpool.hxx
index 9b4d8cf661d3..85188fc52fd5 100644
--- a/cppu/source/threadpool/threadpool.hxx
+++ b/cppu/source/threadpool/threadpool.hxx
@@ -82,7 +82,7 @@ namespace cppu_threadpool {
     public:
         ~DisposedCallerAdmin();
 
-        static DisposedCallerAdminHolder getInstance();
+        static DisposedCallerAdminHolder const & getInstance();
 
         void dispose( sal_Int64 nDisposeId );
         void destroy( sal_Int64 nDisposeId );
diff --git a/cppuhelper/source/propertysetmixin.cxx b/cppuhelper/source/propertysetmixin.cxx
index 32ede83068c0..8469f1a7c51d 100644
--- a/cppuhelper/source/propertysetmixin.cxx
+++ b/cppuhelper/source/propertysetmixin.cxx
@@ -363,7 +363,7 @@ public:
         css::uno::Sequence< rtl::OUString > const & absentOptional,
         css::uno::Type const & type);
 
-    rtl::OUString translateHandle(
+    rtl::OUString const & translateHandle(
         css::uno::Reference< css::uno::XInterface > const & object,
         sal_Int32 handle) const;
 
@@ -448,7 +448,7 @@ PropertySetMixinImpl::Impl::Impl(
     std::copy(handleNames.begin(), handleNames.end(), handleMap.getArray());
 }
 
-rtl::OUString PropertySetMixinImpl::Impl::translateHandle(
+rtl::OUString const & PropertySetMixinImpl::Impl::translateHandle(
     css::uno::Reference< css::uno::XInterface > const & object,
     sal_Int32 handle) const
 {
diff --git a/include/basegfx/polygon/b2dpolygontools.hxx b/include/basegfx/polygon/b2dpolygontools.hxx
index c20668995d56..8b5ee03c96e1 100644
--- a/include/basegfx/polygon/b2dpolygontools.hxx
+++ b/include/basegfx/polygon/b2dpolygontools.hxx
@@ -224,7 +224,7 @@ namespace basegfx
 
         /** Create the unit polygon
          */
-        BASEGFX_DLLPUBLIC B2DPolygon createUnitPolygon();
+        BASEGFX_DLLPUBLIC B2DPolygon const & createUnitPolygon();
 
         /** Create a circle polygon with given radius.
 
@@ -241,7 +241,7 @@ namespace basegfx
         BASEGFX_DLLPUBLIC B2DPolygon createPolygonFromCircle( const B2DPoint& rCenter, double fRadius );
 
         /// create half circle centered on (0,0) from [0 .. F_PI]
-        B2DPolygon createHalfUnitCircle();
+        B2DPolygon const & createHalfUnitCircle();
 
         /** create a polygon which describes the unit circle and close it
 
@@ -253,7 +253,7 @@ namespace basegfx
             this is the lowest one). This is needed since when lines are dashed, toe old
             geometry started at bottom point, else it would look different.
          */
-        BASEGFX_DLLPUBLIC B2DPolygon createPolygonFromUnitCircle(sal_uInt32 nStartQuadrant = 0);
+        BASEGFX_DLLPUBLIC B2DPolygon const & createPolygonFromUnitCircle(sal_uInt32 nStartQuadrant = 0);
 
         /** Create an ellipse polygon with given radii.
 
diff --git a/include/basegfx/polygon/b3dpolypolygontools.hxx b/include/basegfx/polygon/b3dpolypolygontools.hxx
index 48fad7ecc0c2..b3905137a123 100644
--- a/include/basegfx/polygon/b3dpolypolygontools.hxx
+++ b/include/basegfx/polygon/b3dpolypolygontools.hxx
@@ -44,11 +44,11 @@ namespace basegfx
 
         /** Create a unit 3D line polyPolygon which defines a cube.
          */
-        BASEGFX_DLLPUBLIC B3DPolyPolygon createUnitCubePolyPolygon();
+        BASEGFX_DLLPUBLIC B3DPolyPolygon const & createUnitCubePolyPolygon();
 
         /** Create a unit 3D fill polyPolygon which defines a cube.
          */
-        BASEGFX_DLLPUBLIC B3DPolyPolygon createUnitCubeFillPolyPolygon();
+        BASEGFX_DLLPUBLIC B3DPolyPolygon const & createUnitCubeFillPolyPolygon();
 
         /** Create a 3D line polyPolygon from a B3DRange which defines a cube.
          */
diff --git a/include/sax/fshelper.hxx b/include/sax/fshelper.hxx
index 5be3b3bfc697..70ef1025218b 100644
--- a/include/sax/fshelper.hxx
+++ b/include/sax/fshelper.hxx
@@ -212,7 +212,7 @@ public:
 
     FastSerializerHelper* writeId(sal_Int32 tokenId);
 
-    css::uno::Reference< css::io::XOutputStream > getOutputStream() const;
+    css::uno::Reference< css::io::XOutputStream > const & getOutputStream() const;
 
     static FastAttributeList *createAttrList();
 
diff --git a/include/tools/stream.hxx b/include/tools/stream.hxx
index abf25b1e4a70..640dca026598 100644
--- a/include/tools/stream.hxx
+++ b/include/tools/stream.hxx
@@ -203,7 +203,7 @@ public:
     SvLockBytes*    GetLockBytes() const { return m_xLockBytes.get(); }
 
     ErrCode         GetError() const { return m_nError.IgnoreWarning(); }
-    ErrCode         GetErrorCode() const { return m_nError; }
+    ErrCode const & GetErrorCode() const { return m_nError; }
     void            SetError( ErrCode nErrorCode );
     virtual void    ResetError();
 
diff --git a/include/xmlscript/xml_helper.hxx b/include/xmlscript/xml_helper.hxx
index dad9f599f158..508c5789fe1b 100644
--- a/include/xmlscript/xml_helper.hxx
+++ b/include/xmlscript/xml_helper.hxx
@@ -57,7 +57,7 @@ public:
 
         @param nIndex index of sub element
     */
-    css::uno::Reference< css::xml::sax::XAttributeList > getSubElement( sal_Int32 nIndex );
+    css::uno::Reference< css::xml::sax::XAttributeList > const & getSubElement( sal_Int32 nIndex );
 
     /** Adds an attribute to elements.
 
diff --git a/sax/source/tools/fastserializer.cxx b/sax/source/tools/fastserializer.cxx
index a0ae255b36eb..6257853a64a8 100644
--- a/sax/source/tools/fastserializer.cxx
+++ b/sax/source/tools/fastserializer.cxx
@@ -377,7 +377,7 @@ namespace sax_fastparser {
         writeBytes(sSlashAndClosingBracket, N_CHARS(sSlashAndClosingBracket));
     }
 
-    css::uno::Reference< css::io::XOutputStream > FastSaxSerializer::getOutputStream() const
+    css::uno::Reference< css::io::XOutputStream > const & FastSaxSerializer::getOutputStream() const
     {
         return maCachedOutputStream.getOutputStream();
     }
diff --git a/sax/source/tools/fastserializer.hxx b/sax/source/tools/fastserializer.hxx
index 74e5de223e10..c166f38f0b8c 100644
--- a/sax/source/tools/fastserializer.hxx
+++ b/sax/source/tools/fastserializer.hxx
@@ -51,7 +51,7 @@ public:
     explicit FastSaxSerializer(const css::uno::Reference< css::io::XOutputStream >& xOutputStream);
     ~FastSaxSerializer();
 
-    css::uno::Reference< css::io::XOutputStream > getOutputStream() const;
+    css::uno::Reference< css::io::XOutputStream > const & getOutputStream() const;
     /// called by FSHelper to put data in for writeTokenValueList
     TokenValueList& getTokenValueList() { return maTokenValues; }
 
diff --git a/sax/source/tools/fshelper.cxx b/sax/source/tools/fshelper.cxx
index 0b71769a0fed..bd35903bd7a6 100644
--- a/sax/source/tools/fshelper.cxx
+++ b/sax/source/tools/fshelper.cxx
@@ -125,7 +125,7 @@ FastSerializerHelper* FastSerializerHelper::writeId(sal_Int32 tokenId)
     return this;
 }
 
-css::uno::Reference< css::io::XOutputStream > FastSerializerHelper::getOutputStream() const
+css::uno::Reference< css::io::XOutputStream > const & FastSerializerHelper::getOutputStream() const
 {
     return mpSerializer->getOutputStream();
 }
diff --git a/unotools/source/accessibility/accessiblerelationsethelper.cxx b/unotools/source/accessibility/accessiblerelationsethelper.cxx
index 07456acf43ac..b93192af973e 100644
--- a/unotools/source/accessibility/accessiblerelationsethelper.cxx
+++ b/unotools/source/accessibility/accessiblerelationsethelper.cxx
@@ -39,7 +39,7 @@ public:
     sal_Int32 getRelationCount() const;
     /// @throws lang::IndexOutOfBoundsException
     /// @throws uno::RuntimeException
-    AccessibleRelation getRelation( sal_Int32 nIndex ) const;
+    AccessibleRelation const & getRelation( sal_Int32 nIndex ) const;
     /// @throws uno::RuntimeException
     bool containsRelation( sal_Int16 aRelationType ) const;
     /// @throws uno::RuntimeException
@@ -65,7 +65,7 @@ sal_Int32 AccessibleRelationSetHelperImpl::getRelationCount() const
     return maRelations.size();
 }
 
-AccessibleRelation AccessibleRelationSetHelperImpl::getRelation( sal_Int32 nIndex ) const
+AccessibleRelation const & AccessibleRelationSetHelperImpl::getRelation( sal_Int32 nIndex ) const
 {
     if ((nIndex < 0) || (static_cast<sal_uInt32>(nIndex) >= maRelations.size()))
         throw lang::IndexOutOfBoundsException();
diff --git a/unotools/source/config/eventcfg.cxx b/unotools/source/config/eventcfg.cxx
index 98cfd1a76dae..8451b3a93501 100644
--- a/unotools/source/config/eventcfg.cxx
+++ b/unotools/source/config/eventcfg.cxx
@@ -111,7 +111,7 @@ public:
     /// @throws css::uno::RuntimeException
     bool hasByName( const OUString& aName );
     /// @throws css::uno::RuntimeException
-    static css::uno::Type getElementType(  );
+    static css::uno::Type const & getElementType(  );
     /// @throws css::uno::RuntimeException
     bool hasElements() const;
     OUString const & GetEventName( GlobalEventId nID ) const;
@@ -300,7 +300,7 @@ bool GlobalEventConfig_Impl::hasByName( const OUString& aName )
     return pos != m_supportedEvents.end();
 }
 
-Type GlobalEventConfig_Impl::getElementType(  )
+Type const & GlobalEventConfig_Impl::getElementType(  )
 {
     //DF definitely not sure about this??
     return cppu::UnoType<Sequence<beans::PropertyValue>>::get();
diff --git a/unotools/source/config/moduleoptions.cxx b/unotools/source/config/moduleoptions.cxx
index a9ff6b5eaecc..1e3bcaf5b32d 100644
--- a/unotools/source/config/moduleoptions.cxx
+++ b/unotools/source/config/moduleoptions.cxx
@@ -256,10 +256,10 @@ class SvtModuleOptions_Impl : public ::utl::ConfigItem
 
         bool            IsModuleInstalled         (       SvtModuleOptions::EModule     eModule    ) const;
         css::uno::Sequence < OUString > GetAllServiceNames();
-        OUString        GetFactoryName            (       SvtModuleOptions::EFactory    eFactory   ) const;
-        OUString        GetFactoryStandardTemplate(       SvtModuleOptions::EFactory    eFactory   ) const;
+        OUString const & GetFactoryName           (       SvtModuleOptions::EFactory    eFactory   ) const;
+        OUString const & GetFactoryStandardTemplate(      SvtModuleOptions::EFactory    eFactory   ) const;
         static OUString GetFactoryEmptyDocumentURL(       SvtModuleOptions::EFactory    eFactory   );
-        OUString        GetFactoryDefaultFilter   (       SvtModuleOptions::EFactory    eFactory   ) const;
+        OUString const & GetFactoryDefaultFilter  (       SvtModuleOptions::EFactory    eFactory   ) const;
         bool            IsDefaultFilterReadonly(          SvtModuleOptions::EFactory eFactory      ) const;
         sal_Int32       GetFactoryIcon            (       SvtModuleOptions::EFactory    eFactory   ) const;
         static bool     ClassifyFactoryByName     ( const OUString&              sName      ,
@@ -439,7 +439,7 @@ css::uno::Sequence < OUString > SvtModuleOptions_Impl::GetAllServiceNames()
     return comphelper::containerToSequence(aVec);
 }
 
-OUString SvtModuleOptions_Impl::GetFactoryName( SvtModuleOptions::EFactory eFactory ) const
+OUString const & SvtModuleOptions_Impl::GetFactoryName( SvtModuleOptions::EFactory eFactory ) const
 {
     return m_lFactories[eFactory].getFactory();
 }
@@ -482,7 +482,7 @@ OUString SvtModuleOptions::GetFactoryShortName(SvtModuleOptions::EFactory eFacto
     return sShortName;
 }
 
-OUString SvtModuleOptions_Impl::GetFactoryStandardTemplate( SvtModuleOptions::EFactory eFactory ) const
+OUString const & SvtModuleOptions_Impl::GetFactoryStandardTemplate( SvtModuleOptions::EFactory eFactory ) const
 {
     return m_lFactories[eFactory].getTemplateFile();
 }
@@ -524,7 +524,7 @@ OUString SvtModuleOptions_Impl::GetFactoryEmptyDocumentURL( SvtModuleOptions::EF
     return sURL;
 }
 
-OUString SvtModuleOptions_Impl::GetFactoryDefaultFilter( SvtModuleOptions::EFactory eFactory ) const
+OUString const & SvtModuleOptions_Impl::GetFactoryDefaultFilter( SvtModuleOptions::EFactory eFactory ) const
 {
     return m_lFactories[eFactory].getDefaultFilter();
 }
diff --git a/unotools/source/ucbhelper/ucblockbytes.hxx b/unotools/source/ucbhelper/ucblockbytes.hxx
index 71ebf42ed088..164c551d8dbf 100644
--- a/unotools/source/ucbhelper/ucblockbytes.hxx
+++ b/unotools/source/ucbhelper/ucblockbytes.hxx
@@ -111,7 +111,7 @@ public:
     void                    SetError( ErrCode nError )
                             { m_nError = nError; }
 
-    ErrCode                 GetError() const
+    ErrCode const &         GetError() const
                             { return m_nError; }
 
     // calling this method delegates the responsibility to call closeinput to the caller!
diff --git a/xmlscript/source/xml_helper/xml_element.cxx b/xmlscript/source/xml_helper/xml_element.cxx
index 5b8b6b0326db..02bfdc98245e 100644
--- a/xmlscript/source/xml_helper/xml_element.cxx
+++ b/xmlscript/source/xml_helper/xml_element.cxx
@@ -37,7 +37,7 @@ void XMLElement::addSubElement( Reference< xml::sax::XAttributeList > const & xE
     _subElems.push_back( xElem );
 }
 
-Reference< xml::sax::XAttributeList > XMLElement::getSubElement( sal_Int32 nIndex )
+Reference< xml::sax::XAttributeList > const & XMLElement::getSubElement( sal_Int32 nIndex )
 {
     return _subElems[ (size_t)nIndex ];
 }