loplugin:passstuffbyref more return improvements

slightly less restrictive check when calling functions

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

2 files changed, 7 insertions, 6 deletions

diff --git a/compilerplugins/clang/passstuffbyref.cxx b/compilerplugins/clang/passstuffbyref.cxx
index 54982fdfc045..8cfae946f041 100644
--- a/compilerplugins/clang/passstuffbyref.cxx
+++ b/compilerplugins/clang/passstuffbyref.cxx
@@ -275,7 +275,7 @@ void PassStuffByRef::checkReturnValue(const FunctionDecl * functionDecl, const C
         return;
     }
 
-    //functionDecl->dump();
+    // functionDecl->dump();
 
     mbInsideFunctionDecl = true;
     mbFoundReturnValueDisqualifier = false;
@@ -412,12 +412,14 @@ bool PassStuffByRef::isReturnExprDisqualified(const Expr* expr)
             FunctionDecl const * calleeFunctionDecl = callExpr->getDirectCallee();
             if (!calleeFunctionDecl)
                 return true;
-            // TODO anything takes a param is suspect because it might return the param by ref.
+            // TODO anything takes a non-integral param is suspect because it might return the param by ref.
             // we could tighten this to only reject functions that have a param of the same type
             // as the return type. Or we could check for such functions and disallow them.
             // Or we could force such functions to be annotated somehow.
-            if (calleeFunctionDecl->getNumParams() > 0)
-                return true;
+            for (unsigned i = 0; i != calleeFunctionDecl->getNumParams(); ++i) {
+                if (!calleeFunctionDecl->getParamDecl(i)->getType()->isIntegralOrEnumerationType())
+                    return true;
+            }
             auto tc = loplugin::TypeCheck(calleeFunctionDecl->getReturnType());
             if (!tc.LvalueReference() && !tc.Pointer())
                 return true;
diff --git a/compilerplugins/clang/test/passstuffbyref.cxx b/compilerplugins/clang/test/passstuffbyref.cxx
index 71c219c986ec..f6277b4bbf07 100644
--- a/compilerplugins/clang/test/passstuffbyref.cxx
+++ b/compilerplugins/clang/test/passstuffbyref.cxx
@@ -41,8 +41,7 @@ struct S2 {
     // TODO
     OUString get10() { return OUString(*&get6()); } // todoexpected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
     OUString get11() const { return mxCow->get(); } // expected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
-    // TODO anything takes a param is suspect because it might return the param by ref
-    OUString get12() { return child.get2(false); } // todoexpected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
+    OUString get12() { return child.get2(false); } // 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"); }