enhance useuniqueptr loplugin

teach it to look for the following sequence in a destructor:
delete m_pfoo;
m_pfoo = nullptr;

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

2 files changed, 34 insertions, 12 deletions

diff --git a/compilerplugins/clang/test/useuniqueptr.cxx b/compilerplugins/clang/test/useuniqueptr.cxx
new file mode 100644
index 000000000000..c705bbf0f158
--- /dev/null
+++ b/compilerplugins/clang/test/useuniqueptr.cxx
@@ -0,0 +1,20 @@
+/* -*- 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/.
+ */
+
+
+class Foo {
+    char* m_pbar; // expected-note {{member is here [loplugin:useuniqueptr]}}
+    ~Foo() // expected-error {{Unreferenced externally visible function definition [loplugin:unreffun]}}
+    {
+        delete m_pbar; // expected-error {{a destructor with only a single unconditional call to delete on a member, is a sure sign it should be using std::unique_ptr for that field [loplugin:useuniqueptr]}}
+        m_pbar = nullptr;
+    }
+};
+
+/* 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 59d1a3f3ec68..afae4c3c770a 100644
--- a/compilerplugins/clang/useuniqueptr.cxx
+++ b/compilerplugins/clang/useuniqueptr.cxx
@@ -44,30 +44,32 @@ bool UseUniquePtr::VisitCXXDestructorDecl(const CXXDestructorDecl* destructorDec
     if (isInUnoIncludeFile(destructorDecl))
         return true;
 
-    if (destructorDecl->getBody() == nullptr)
+    const CompoundStmt* compoundStmt = dyn_cast_or_null< CompoundStmt >( destructorDecl->getBody() );
+    if (!compoundStmt)
         return true;
-    const CompoundStmt* compoundStmt = dyn_cast< CompoundStmt >( destructorDecl->getBody() );
-    if (compoundStmt == nullptr) {
-        return true;
-    }
 
-    const CXXDeleteExpr* deleteExpr;
+    const CXXDeleteExpr* deleteExpr = nullptr;
     if (compoundStmt->size() == 1) {
         deleteExpr = dyn_cast<CXXDeleteExpr>(compoundStmt->body_front());
     }
     else if (compoundStmt->size() == 2) {
         // ignore SAL_INFO type stuff
-        // TODO should probably be a little more specific here
-        if (!isa<DoStmt>(compoundStmt->body_front())) {
-            return true;
+        // @TODO should probably be a little more specific here
+        if (isa<DoStmt>(compoundStmt->body_front())) {
+            deleteExpr = dyn_cast<CXXDeleteExpr>(compoundStmt->body_back());
+        }
+        // look for the following pattern:
+        // delete m_pbar;
+        // m_pbar = nullptr;
+        else if (auto binaryOp = dyn_cast<BinaryOperator>(compoundStmt->body_back())) {
+            if (binaryOp->getOpcode() == BO_Assign)
+                deleteExpr = dyn_cast<CXXDeleteExpr>(compoundStmt->body_front());
         }
-        deleteExpr = dyn_cast<CXXDeleteExpr>(compoundStmt->body_back());
     } else {
         return true;
     }
-    if (deleteExpr == nullptr) {
+    if (deleteExpr == nullptr)
         return true;
-    }
 
     const ImplicitCastExpr* pCastExpr = dyn_cast<ImplicitCastExpr>(deleteExpr->getArgument());
     if (!pCastExpr)