make passstuffbyref plugin ignore std::move'd params

request from vmiklos

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

2 files changed, 66 insertions, 12 deletions

diff --git a/compilerplugins/clang/passstuffbyref.cxx b/compilerplugins/clang/passstuffbyref.cxx
index ad6c10018fdf..8695ae62d622 100644
--- a/compilerplugins/clang/passstuffbyref.cxx
+++ b/compilerplugins/clang/passstuffbyref.cxx
@@ -185,24 +185,50 @@ void PassStuffByRef::checkParams(const FunctionDecl * functionDecl) {
     if (!functionDecl->doesThisDeclarationHaveABody()) {
         return;
     }
+    auto cxxConstructorDecl = dyn_cast<CXXConstructorDecl>(functionDecl);
     unsigned n = functionDecl->getNumParams();
     for (unsigned i = 0; i != n; ++i) {
         const ParmVarDecl * pvDecl = functionDecl->getParamDecl(i);
         auto const t = pvDecl->getType();
-        if (isFat(t)) {
-            report(
-                DiagnosticsEngine::Warning,
-                ("passing %0 by value, rather pass by const lvalue reference"),
-                pvDecl->getLocation())
-                << t << pvDecl->getSourceRange();
-            auto can = functionDecl->getCanonicalDecl();
-            if (can->getLocation() != functionDecl->getLocation()) {
-                report(
-                    DiagnosticsEngine::Note, "function is declared here:",
-                    can->getLocation())
-                    << can->getSourceRange();
+        if (!isFat(t)) {
+            continue;
+        }
+        // Ignore cases where the parameter is std::move'd.
+        // This is a fairly simple check, might need some more complexity if the parameter is std::move'd
+        // somewhere else in the constructor.
+        bool bFoundMove = false;
+        if (cxxConstructorDecl) {
+            for (CXXCtorInitializer const * cxxCtorInitializer : cxxConstructorDecl->inits()) {
+                if (cxxCtorInitializer->isMemberInitializer())
+                {
+                    auto cxxConstructExpr = dyn_cast<CXXConstructExpr>(cxxCtorInitializer->getInit());
+                    if (cxxConstructExpr && cxxConstructExpr->getNumArgs() == 1)
+                    {
+                        if (auto callExpr = dyn_cast<CallExpr>(cxxConstructExpr->getArg(0))) {
+                            if (loplugin::DeclCheck(callExpr->getCalleeDecl()).Function("move").StdNamespace()) {
+                                bFoundMove = true;
+                                break;
+                            }
+                        }
+                    }
+                }
             }
         }
+        if (bFoundMove) {
+            continue;
+        }
+        report(
+            DiagnosticsEngine::Warning,
+            ("passing %0 by value, rather pass by const lvalue reference"),
+            pvDecl->getLocation())
+            << t << pvDecl->getSourceRange();
+        auto can = functionDecl->getCanonicalDecl();
+        if (can->getLocation() != functionDecl->getLocation()) {
+            report(
+                DiagnosticsEngine::Note, "function is declared here:",
+                can->getLocation())
+                << can->getSourceRange();
+        }
     }
     // ignore stuff that forms part of the stable URE interface
     if (isInUnoIncludeFile(functionDecl)) {
diff --git a/compilerplugins/clang/test/passstuffbyref.cxx b/compilerplugins/clang/test/passstuffbyref.cxx
new file mode 100644
index 000000000000..d19b139f485e
--- /dev/null
+++ b/compilerplugins/clang/test/passstuffbyref.cxx
@@ -0,0 +1,28 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <rtl/ustring.hxx>
+
+struct S {
+    OUString mv;
+
+    // request from vmiklos: make sure we ignore cases where the passed in parameter is std::move'd
+    S(OUString v)
+      : mv(std::move(v)) {}
+};
+
+
+void f() // expected-error {{Unreferenced externally visible function definition [loplugin:unreffun]}}
+{
+    S* s;
+    OUString v;
+    s = new S(v);
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */