loplugin useuniqueptr improvement

passing owning pointers to constructors

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

2 files changed, 44 insertions, 0 deletions

diff --git a/compilerplugins/clang/test/useuniqueptr.cxx b/compilerplugins/clang/test/useuniqueptr.cxx
index 844f7fb65d7e..7b9870b6f6bf 100644
--- a/compilerplugins/clang/test/useuniqueptr.cxx
+++ b/compilerplugins/clang/test/useuniqueptr.cxx
@@ -8,6 +8,7 @@
  */
 
 #include <array>
+#include <memory>
 #include <vector>
 #include <unordered_map>
 
@@ -180,4 +181,23 @@ void Foo15(int * p)
 {
     delete p; // expected-error {{calling delete on a pointer param, should be either whitelisted here or simplified [loplugin:useuniqueptr]}}
 };
+
+//  ------------------------------------------------------------------------------------------------
+// tests for passing owning pointers to constructors
+
+class Bravo1
+{
+    std::unique_ptr<int> m_field1;
+    Bravo1(int* p)
+        : m_field1(p) // expected-error {{should be passing via std::unique_ptr param [loplugin:useuniqueptr]}}
+    {}
+};
+class Bravo2
+{
+    std::unique_ptr<int> m_field1;
+    Bravo2(std::unique_ptr<int> p)
+        : m_field1(std::move(p)) // no warning expected
+    {}
+};
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/useuniqueptr.cxx b/compilerplugins/clang/useuniqueptr.cxx
index 7095ddff0b62..1192513a17d0 100644
--- a/compilerplugins/clang/useuniqueptr.cxx
+++ b/compilerplugins/clang/useuniqueptr.cxx
@@ -124,6 +124,8 @@ public:
     bool VisitCompoundStmt(const CompoundStmt* );
     bool VisitCXXDeleteExpr(const CXXDeleteExpr* );
     bool TraverseFunctionDecl(FunctionDecl* );
+    bool TraverseConstructorInitializer(CXXCtorInitializer*);
+
 private:
     void CheckCompoundStmt(const CXXMethodDecl*, const CompoundStmt* );
     void CheckForUnconditionalDelete(const CXXMethodDecl*, const CompoundStmt* );
@@ -532,6 +534,28 @@ bool UseUniquePtr::VisitCXXDeleteExpr(const CXXDeleteExpr* deleteExpr)
     return true;
 }
 
+bool UseUniquePtr::TraverseConstructorInitializer(CXXCtorInitializer * ctorInit)
+{
+    if (!ctorInit->getSourceLocation().isValid() || ignoreLocation(ctorInit->getSourceLocation()))
+        return true;
+    if (!ctorInit->getMember())
+        return true;
+    if (!loplugin::TypeCheck(ctorInit->getMember()->getType()).Class("unique_ptr").StdNamespace())
+        return true;
+    auto constructExpr = dyn_cast<CXXConstructExpr>(ctorInit->getInit());
+    if (!constructExpr)
+        return true;
+    auto init = constructExpr->getArg(0)->IgnoreImpCasts();
+    if (!isa<DeclRefExpr>(init))
+        return true;
+    report(
+        DiagnosticsEngine::Warning,
+        "should be passing via std::unique_ptr param",
+        ctorInit->getSourceLocation())
+        << ctorInit->getSourceRange();
+    return RecursiveASTVisitor<UseUniquePtr>::TraverseConstructorInitializer(ctorInit);
+}
+
 loplugin::Plugin::Registration< UseUniquePtr > X("useuniqueptr", false);
 
 }