loplugin:useuniqueptr look for deleting in loops with iterators

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

2 files changed, 366 insertions, 180 deletions

diff --git a/compilerplugins/clang/test/useuniqueptr.cxx b/compilerplugins/clang/test/useuniqueptr.cxx
index ad6314986a72..369cfa1e8929 100644
--- a/compilerplugins/clang/test/useuniqueptr.cxx
+++ b/compilerplugins/clang/test/useuniqueptr.cxx
@@ -96,7 +96,7 @@ class Class8 {
     std::unordered_map<int, int*> m_pbar; // expected-note {{member is here [loplugin:useuniqueptr]}}
     ~Class8()
     {
-        for (auto i : m_pbar)
+        for (auto & i : m_pbar)
             delete i.second; // expected-error {{rather manage this with std::some_container<std::unique_ptr<T>> [loplugin:useuniqueptr]}}
     }
 };
@@ -251,7 +251,50 @@ namespace foo20
 };
 
 //  ------------------------------------------------------------------------------------------------
+// tests for deleting when looping via iterators
+//  ------------------------------------------------------------------------------------------------
+
+void foo21()
+{
+    std::vector<bool*> vec; // expected-note {{var is here [loplugin:useuniqueptr]}}
+    for(auto it = vec.begin(); it != vec.end(); ++it)
+        delete *it; // expected-error {{rather manage this var with std::some_container<std::unique_ptr<T>> [loplugin:useuniqueptr]}}
+}
+
+void foo22()
+{
+    std::unordered_map<int, float*> map; // expected-note {{var is here [loplugin:useuniqueptr]}}
+    for(auto it = map.begin(); it != map.end(); ++it)
+        delete it->second; // expected-error {{rather manage this var with std::some_container<std::unique_ptr<T>> [loplugin:useuniqueptr]}}
+}
+
+class Foo23
+{
+    std::unordered_map<int, float*> map; // expected-note {{member is here [loplugin:useuniqueptr]}}
+    ~Foo23()
+    {
+        for(auto it = map.begin(); it != map.end(); ++it)
+            delete it->second; // expected-error {{rather manage with std::some_container<std::unique_ptr<T>> [loplugin:useuniqueptr]}}
+    }
+};
+
+#if CLANG_VERSION >= 50000
+class Foo24
+{
+    typedef std::vector<int*> HTMLAttrs;
+    HTMLAttrs m_aSetAttrTab; // expected-note {{member is here [loplugin:useuniqueptr]}}
+    ~Foo24()
+    {
+        for ( HTMLAttrs::const_iterator it = m_aSetAttrTab.begin(); it != m_aSetAttrTab.end(); ++it )
+            delete *it; // expected-error {{rather manage with std::some_container<std::unique_ptr<T>> [loplugin:useuniqueptr]}}
+    }
+};
+#endif
+
+//  ------------------------------------------------------------------------------------------------
 // tests for passing owning pointers to constructors
+//  ------------------------------------------------------------------------------------------------
+
 
 class Bravo1
 {
@@ -268,4 +311,5 @@ class Bravo2
     {}
 };
 
+
 /* 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 1e257ae64429..72f74aa445a9 100644
--- a/compilerplugins/clang/useuniqueptr.cxx
+++ b/compilerplugins/clang/useuniqueptr.cxx
@@ -124,11 +124,15 @@ public:
     }
 
     bool VisitFunctionDecl(const FunctionDecl* );
-    bool VisitCompoundStmt(const CompoundStmt* );
     bool VisitCXXDeleteExpr(const CXXDeleteExpr* );
     bool TraverseFunctionDecl(FunctionDecl* );
+#if CLANG_VERSION >= 50000
+    bool TraverseCXXDeductionGuideDecl(CXXDeductionGuideDecl* );
+#endif
     bool TraverseCXXMethodDecl(CXXMethodDecl* );
     bool TraverseCXXConstructorDecl(CXXConstructorDecl* );
+    bool TraverseCXXConversionDecl(CXXConversionDecl* );
+    bool TraverseCXXDestructorDecl(CXXDestructorDecl* );
     bool TraverseConstructorInitializer(CXXCtorInitializer*);
 
 private:
@@ -139,7 +143,7 @@ private:
     void CheckLoopDelete(const FunctionDecl*, const CXXDeleteExpr* );
     void CheckDeleteExpr(const FunctionDecl*, const CXXDeleteExpr*);
     void CheckParenExpr(const FunctionDecl*, const ParenExpr*);
-    void CheckDeleteExpr(const FunctionDecl*, const CXXDeleteExpr*,
+    void CheckMemberDeleteExpr(const FunctionDecl*, const CXXDeleteExpr*,
         const MemberExpr*, StringRef message);
     FunctionDecl const * mpCurrentFunctionDecl = nullptr;
     std::string fn;
@@ -270,7 +274,7 @@ void UseUniquePtr::CheckDeleteExpr(const FunctionDecl* functionDecl, const CXXDe
         if (fn == SRCDIR "/sc/source/filter/html/htmlpars.cxx")
             return;
 
-        CheckDeleteExpr(functionDecl, deleteExpr, memberExpr,
+        CheckMemberDeleteExpr(functionDecl, deleteExpr, memberExpr,
             "unconditional call to delete on a member, should be using std::unique_ptr");
         return;
     }
@@ -422,7 +426,7 @@ void UseUniquePtr::CheckDeleteExpr(const FunctionDecl* functionDecl, const CXXDe
     {
        auto baseMemberExpr = dyn_cast<MemberExpr>(arrayExpr->getBase()->IgnoreParenImpCasts());
        if (baseMemberExpr)
-            CheckDeleteExpr(functionDecl, deleteExpr, baseMemberExpr,
+            CheckMemberDeleteExpr(functionDecl, deleteExpr, baseMemberExpr,
                 "unconditional call to delete on an array member, should be using std::unique_ptr");
     }
 }
@@ -441,89 +445,6 @@ void UseUniquePtr::CheckParenExpr(const FunctionDecl* functionDecl, const ParenE
     CheckDeleteExpr(functionDecl, deleteExpr);
 }
 
-void UseUniquePtr::CheckDeleteExpr(const FunctionDecl* functionDecl, const CXXDeleteExpr* deleteExpr,
-    const MemberExpr* memberExpr, StringRef message)
-{
-    // ignore union games
-    const FieldDecl* fieldDecl = dyn_cast<FieldDecl>(memberExpr->getMemberDecl());
-    if (!fieldDecl)
-        return;
-    TagDecl const * td = dyn_cast<TagDecl>(fieldDecl->getDeclContext());
-    if (td->isUnion())
-        return;
-
-    // ignore calling delete on someone else's field
-    if (auto methodDecl = dyn_cast<CXXMethodDecl>(functionDecl))
-        if (fieldDecl->getParent() != methodDecl->getParent() )
-            return;
-
-    if (ignoreLocation(fieldDecl))
-        return;
-    // to ignore things like the CPPUNIT macros
-    if (startswith(fn, WORKDIR "/"))
-        return;
-    // passes and stores pointers to member fields
-    if (fn == SRCDIR "/sot/source/sdstor/stgdir.hxx")
-        return;
-    // something platform-specific
-    if (fn == SRCDIR "/hwpfilter/source/htags.h")
-        return;
-    // passes pointers to member fields
-    if (fn == SRCDIR "/sd/inc/sdpptwrp.hxx")
-        return;
-    // @TODO intrusive linked-lists here, with some trickiness
-    if (fn == SRCDIR "/sw/source/filter/html/parcss1.hxx")
-        return;
-    // @TODO SwDoc has some weird ref-counting going on
-    if (fn == SRCDIR "/sw/inc/shellio.hxx")
-        return;
-    // @TODO it's sharing pointers with another class
-    if (fn == SRCDIR "/sc/inc/formulacell.hxx")
-        return;
-    // some weird stuff going on here around struct Entity
-    if (startswith(fn, SRCDIR "/sax/"))
-        return;
-    if (startswith(fn, SRCDIR "/include/sax/"))
-        return;
-    // manipulation of tree structures ie. StgAvlNode, don't lend themselves to std::unique_ptr
-    if (startswith(fn, SRCDIR "/sot/"))
-        return;
-    if (startswith(fn, SRCDIR "/include/sot/"))
-        return;
-    // the std::vector is being passed to another class
-    if (fn == SRCDIR "/sfx2/source/explorer/nochaos.cxx")
-        return;
-    auto tc = loplugin::TypeCheck(fieldDecl->getType());
-    // these sw::Ring based classes do not lend themselves to std::unique_ptr management
-    if (tc.Pointer().Class("SwNodeIndex").GlobalNamespace() || tc.Pointer().Class("SwShellTableCursor").GlobalNamespace()
-        || tc.Pointer().Class("SwBlockCursor").GlobalNamespace() || tc.Pointer().Class("SwVisibleCursor").GlobalNamespace()
-        || tc.Pointer().Class("SwShellCursor").GlobalNamespace())
-        return;
-    // there is a loop in ~ImplPrnQueueList deleting stuff on a global data structure
-    if (fn == SRCDIR "/vcl/inc/print.h")
-        return;
-    // painful linked list
-    if (fn == SRCDIR "/basic/source/inc/runtime.hxx")
-        return;
-    // not sure how the node management is working here
-    if (fn == SRCDIR "/i18npool/source/localedata/saxparser.cxx")
-        return;
-    // has a pointer that it only sometimes owns
-    if (fn == SRCDIR "/editeng/source/editeng/impedit.hxx")
-        return;
-
-    report(
-        DiagnosticsEngine::Warning,
-        message,
-        compat::getBeginLoc(deleteExpr))
-        << deleteExpr->getSourceRange();
-    report(
-        DiagnosticsEngine::Note,
-        "member is here",
-        compat::getBeginLoc(fieldDecl))
-        << fieldDecl->getSourceRange();
-}
-
 void UseUniquePtr::CheckLoopDelete(const FunctionDecl* functionDecl, const Stmt* bodyStmt)
 {
     if (auto deleteExpr = dyn_cast<CXXDeleteExpr>(bodyStmt))
@@ -541,13 +462,27 @@ void UseUniquePtr::CheckLoopDelete(const FunctionDecl* functionDecl, const Stmt*
 void UseUniquePtr::CheckLoopDelete(const FunctionDecl* functionDecl, const CXXDeleteExpr* deleteExpr)
 {
     const MemberExpr* memberExpr = nullptr;
+    const VarDecl* varDecl = nullptr;
     const Expr* subExpr = deleteExpr->getArgument();
     // drill down looking for a MemberExpr
     for (;;)
     {
-        subExpr = subExpr->IgnoreImpCasts();
+        subExpr = subExpr->IgnoreParenImpCasts();
         if ((memberExpr = dyn_cast<MemberExpr>(subExpr)))
-            break;
+        {
+            if (memberExpr->getMemberDecl()->getName() == "first" || memberExpr->getMemberDecl()->getName() == "second")
+            {
+                subExpr = memberExpr->getBase();
+                memberExpr = nullptr;
+            }
+            else
+                break;
+        }
+        else if (auto declRefExpr = dyn_cast<DeclRefExpr>(subExpr))
+        {
+            if ((varDecl = dyn_cast<VarDecl>(declRefExpr->getDecl())))
+                break;
+        }
         else if (auto arraySubscriptExpr = dyn_cast<ArraySubscriptExpr>(subExpr))
             subExpr = arraySubscriptExpr->getBase();
         else if (auto cxxOperatorCallExpr = dyn_cast<CXXOperatorCallExpr>(subExpr))
@@ -555,46 +490,171 @@ void UseUniquePtr::CheckLoopDelete(const FunctionDecl* functionDecl, const CXXDe
             // look for deletes of an iterator object where the iterator is over a member field
             if (auto declRefExpr = dyn_cast<DeclRefExpr>(cxxOperatorCallExpr->getArg(0)->IgnoreImpCasts()))
             {
-                if (auto varDecl = dyn_cast<VarDecl>(declRefExpr->getDecl()))
+                if (auto iterVarDecl = dyn_cast<VarDecl>(declRefExpr->getDecl()))
                 {
-                    auto init = varDecl->getInit();
+                    auto init = iterVarDecl->getInit();
                     if (init)
                     {
-                        if (auto x = dyn_cast<ExprWithCleanups>(init))
-                            init = x->getSubExpr();
-                        if (auto x = dyn_cast<CXXBindTemporaryExpr>(init))
-                            init = x->getSubExpr();
+                        init = init->IgnoreImplicit();
+                        if (auto x = dyn_cast<CXXConstructExpr>(init))
+                            if (x->getNumArgs() == 1)
+                                init = x->getArg(0)->IgnoreImplicit();
                         if (auto x = dyn_cast<CXXMemberCallExpr>(init))
                             init = x->getImplicitObjectArgument();
-                        memberExpr = dyn_cast<MemberExpr>(init);
+                        if ((memberExpr = dyn_cast<MemberExpr>(init)))
+                            break;
+                        // look for deletes of an iterator object where the iterator is over a var
+                        if (auto declRefExpr2 = dyn_cast<DeclRefExpr>(init))
+                        {
+                            if ((varDecl = dyn_cast<VarDecl>(declRefExpr2->getDecl())))
+                                break;
+                        }
                     }
-                    break;
                 }
             }
             // look for deletes like "delete m_pField[0]"
             if (cxxOperatorCallExpr->getOperator() == OO_Subscript)
             {
-                memberExpr = dyn_cast<MemberExpr>(cxxOperatorCallExpr->getArg(0));
+                subExpr = cxxOperatorCallExpr->getArg(0)->IgnoreImpCasts();
+                if ((memberExpr = dyn_cast<MemberExpr>(subExpr)))
+                    break;
+                if (auto declRefExpr = dyn_cast<DeclRefExpr>(subExpr))
+                {
+                    if ((varDecl = dyn_cast<VarDecl>(declRefExpr->getDecl())))
+                        break;
+                }
             }
             break;
         }
         else
            break;
     }
-    if (!memberExpr)
-        return;
 
-    // OStorage_Impl::Commit very complicated ownership passing going on
-    if (fn == SRCDIR "/package/source/xstor/xstorage.cxx")
-        return;
-    // complicated
-    if (fn == SRCDIR "/vcl/source/gdi/print.cxx")
-        return;
-    // linked list
-    if (fn == SRCDIR "/basic/source/runtime/runtime.cxx")
-        return;
+    if (memberExpr)
+    {
+        // OStorage_Impl::Commit very complicated ownership passing going on
+        if (fn == SRCDIR "/package/source/xstor/xstorage.cxx")
+            return;
+        // complicated
+        if (fn == SRCDIR "/vcl/source/gdi/print.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/basic/source/runtime/runtime.cxx")
+            return;
+        // complicated
+        if (fn == SRCDIR "/sw/source/core/bastyp/swcache.cxx")
+            return;
+
+        CheckMemberDeleteExpr(functionDecl, deleteExpr, memberExpr, "rather manage with std::some_container<std::unique_ptr<T>>");
+    }
+
+    if (varDecl)
+    {
+        // ignore if the value for the var comes from somewhere else
+        if (varDecl->hasInit() && isa<ExplicitCastExpr>(varDecl->getInit()->IgnoreImpCasts()))
+            return;
+
+        if (startswith(fn, SRCDIR "/vcl/qa/"))
+            return;
+        // linked list
+        if (fn == SRCDIR "/registry/source/reflwrit.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/tools/source/generic/config.cxx")
+            return;
+        // linked lists
+        if (fn == SRCDIR "/vcl/source/gdi/regband.cxx")
+            return;
+        // linked lists
+        if (fn == SRCDIR "/vcl/source/gdi/regionband.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/vcl/source/gdi/octree.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/vcl/source/app/scheduler.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/vcl/source/filter/wmf/wmfwr.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/vcl/source/filter/graphicfilter.cxx")
+            return;
+        // valid code
+        if (fn == SRCDIR "/vcl/source/app/salvtables.cxx")
+            return;
+        // undo code is tricky
+        if (fn == SRCDIR "/svl/source/undo/undo.cxx")
+            return;
+        // subclass that messes with parent class in constructor/destructor, yuck
+        if (fn == SRCDIR "/svtools/source/contnr/imivctl1.cxx")
+            return;
+        // SQLParseNode
+        if (fn == SRCDIR "/connectivity/source/parse/sqlnode.cxx")
+            return;
+        // the whole svx model/contact/view thing confuses me
+        if (fn == SRCDIR "/svx/source/sdr/contact/viewcontact.cxx")
+            return;
+        if (fn == SRCDIR "/svx/source/sdr/contact/objectcontact.cxx")
+            return;
+        // no idea
+        if (fn == SRCDIR "/svx/source/unodialogs/textconversiondlgs/chinese_dictionarydialog.cxx")
+            return;
+        // SdrUndo stuff
+        if (fn == SRCDIR "/svx/source/svdraw/svdundo.cxx")
+            return;
+        // TODO the lazydelete stuff should probably just be ripped out altogether nowthat we have VclPtr
+        if (fn == SRCDIR "/vcl/source/helper/lazydelete.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/filter/source/graphicfilter/idxf/dxfblkrd.cxx")
+            return;
+        if (fn == SRCDIR "/filter/source/graphicfilter/idxf/dxftblrd.cxx")
+            return;
+        if (fn == SRCDIR "/lotuswordpro/source/filter/utlist.cxx")
+            return;
+        if (fn == SRCDIR "/lotuswordpro/source/filter/lwpfribptr.cxx")
+            return;
+        // valid
+        if (fn == SRCDIR "/sd/source/ui/sidebar/MasterPagesSelector.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/sd/source/filter/ppt/pptatom.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/sc/source/core/data/funcdesc.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/sw/source/core/crsr/crsrsh.cxx")
+            return;
+        // no idea
+        if (fn == SRCDIR "/sw/source/core/docnode/nodes.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/sw/source/core/unocore/unocrsr.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/filter/source/graphicfilter/idxf/dxfentrd.cxx")
+            return;
+        // linked list
+        if (fn == SRCDIR "/filter/source/graphicfilter/ios2met/ios2met.cxx")
+            return;
+        // sometimes owning, sometimes not
+        if (fn == SRCDIR "/sw/qa/core/Test-BigPtrArray.cxx")
+            return;
+
 
-    CheckDeleteExpr(functionDecl, deleteExpr, memberExpr, "rather manage with std::some_container<std::unique_ptr<T>>");
+        report(
+            DiagnosticsEngine::Warning,
+            "loopdelete: rather manage this var with std::some_container<std::unique_ptr<T>>",
+            compat::getBeginLoc(deleteExpr))
+            << deleteExpr->getSourceRange();
+        report(
+            DiagnosticsEngine::Note,
+            "var is here",
+            compat::getBeginLoc(varDecl))
+            << varDecl->getSourceRange();
+    }
 }
 
 void UseUniquePtr::CheckCXXForRangeStmt(const FunctionDecl* functionDecl, const CXXForRangeStmt* cxxForRangeStmt)
@@ -611,6 +671,7 @@ void UseUniquePtr::CheckCXXForRangeStmt(const FunctionDecl* functionDecl, const
     if (!deleteExpr)
         return;
 
+    // check for delete of member
     if (auto memberExpr = dyn_cast<MemberExpr>(cxxForRangeStmt->getRangeInit()))
     {
         auto fieldDecl = dyn_cast<FieldDecl>(memberExpr->getMemberDecl());
@@ -623,11 +684,15 @@ void UseUniquePtr::CheckCXXForRangeStmt(const FunctionDecl* functionDecl, const
         // complicated
         if (fn == SRCDIR "/vcl/source/gdi/print.cxx")
             return;
+        // sometimes it's an owning field, sometimes not
+        if (fn == SRCDIR "/i18npool/source/localedata/localedata.cxx")
+            return;
 
-        CheckDeleteExpr(functionDecl, deleteExpr, memberExpr, "rather manage this with std::some_container<std::unique_ptr<T>>");
+        CheckMemberDeleteExpr(functionDecl, deleteExpr, memberExpr, "rather manage this with std::some_container<std::unique_ptr<T>>");
     }
 
-    if (auto declRefExpr = dyn_cast<DeclRefExpr>(cxxForRangeStmt->getRangeInit()))
+    // check for delete of var
+    if (auto declRefExpr = dyn_cast<DeclRefExpr>(cxxForRangeStmt->getRangeInit()->IgnoreParenImpCasts()))
     {
         auto varDecl = dyn_cast<VarDecl>(declRefExpr->getDecl());
         if (!varDecl)
@@ -675,52 +740,87 @@ void UseUniquePtr::CheckCXXForRangeStmt(const FunctionDecl* functionDecl, const
     }
 }
 
-bool UseUniquePtr::VisitCompoundStmt(const CompoundStmt* compoundStmt)
+void UseUniquePtr::CheckMemberDeleteExpr(const FunctionDecl* functionDecl, const CXXDeleteExpr* deleteExpr,
+    const MemberExpr* memberExpr, StringRef message)
 {
-    if (ignoreLocation(compoundStmt))
-        return true;
-    if (isInUnoIncludeFile(compat::getBeginLoc(compoundStmt)))
-        return true;
-    if (compoundStmt->size() == 0) {
-        return true;
-    }
+    // ignore union games
+    const FieldDecl* fieldDecl = dyn_cast<FieldDecl>(memberExpr->getMemberDecl());
+    if (!fieldDecl)
+        return;
+    TagDecl const * td = dyn_cast<TagDecl>(fieldDecl->getDeclContext());
+    if (td->isUnion())
+        return;
 
-    auto lastStmt = compoundStmt->body_back();
-    if (compoundStmt->size() > 1) {
-        if (isa<ReturnStmt>(lastStmt))
-            lastStmt = *(++compoundStmt->body_rbegin());
-    }
-    auto deleteExpr = dyn_cast<CXXDeleteExpr>(lastStmt);
-    if (deleteExpr == nullptr) {
-        return true;
-    }
+    // ignore calling delete on someone else's field
+    if (auto methodDecl = dyn_cast<CXXMethodDecl>(functionDecl))
+        if (fieldDecl->getParent() != methodDecl->getParent() )
+            return;
 
-    auto declRefExpr = dyn_cast<DeclRefExpr>(deleteExpr->getArgument()->IgnoreParenImpCasts());
-    if (!declRefExpr)
-        return true;
-    auto varDecl = dyn_cast<VarDecl>(declRefExpr->getDecl());
-    if (!varDecl)
-        return true;
-    if (!varDecl->hasInit()
-        || !isa<CXXNewExpr>(
-            varDecl->getInit()->IgnoreImplicit()->IgnoreParenImpCasts()))
-        return true;
-    // determine if the var is declared inside the same block as the delete.
-    // @TODO there should surely be a better way to do this
-    if (compat::getBeginLoc(varDecl) < compat::getBeginLoc(compoundStmt))
-        return true;
+    if (ignoreLocation(fieldDecl))
+        return;
+    // to ignore things like the CPPUNIT macros
+    if (startswith(fn, WORKDIR "/"))
+        return;
+    // passes and stores pointers to member fields
+    if (fn == SRCDIR "/sot/source/sdstor/stgdir.hxx")
+        return;
+    // something platform-specific
+    if (fn == SRCDIR "/hwpfilter/source/htags.h")
+        return;
+    // passes pointers to member fields
+    if (fn == SRCDIR "/sd/inc/sdpptwrp.hxx")
+        return;
+    // @TODO intrusive linked-lists here, with some trickiness
+    if (fn == SRCDIR "/sw/source/filter/html/parcss1.hxx")
+        return;
+    // @TODO SwDoc has some weird ref-counting going on
+    if (fn == SRCDIR "/sw/inc/shellio.hxx")
+        return;
+    // @TODO it's sharing pointers with another class
+    if (fn == SRCDIR "/sc/inc/formulacell.hxx")
+        return;
+    // some weird stuff going on here around struct Entity
+    if (startswith(fn, SRCDIR "/sax/"))
+        return;
+    if (startswith(fn, SRCDIR "/include/sax/"))
+        return;
+    // manipulation of tree structures ie. StgAvlNode, don't lend themselves to std::unique_ptr
+    if (startswith(fn, SRCDIR "/sot/"))
+        return;
+    if (startswith(fn, SRCDIR "/include/sot/"))
+        return;
+    // the std::vector is being passed to another class
+    if (fn == SRCDIR "/sfx2/source/explorer/nochaos.cxx")
+        return;
+    auto tc = loplugin::TypeCheck(fieldDecl->getType());
+    // these sw::Ring based classes do not lend themselves to std::unique_ptr management
+    if (tc.Pointer().Class("SwNodeIndex").GlobalNamespace() || tc.Pointer().Class("SwShellTableCursor").GlobalNamespace()
+        || tc.Pointer().Class("SwBlockCursor").GlobalNamespace() || tc.Pointer().Class("SwVisibleCursor").GlobalNamespace()
+        || tc.Pointer().Class("SwShellCursor").GlobalNamespace())
+        return;
+    // there is a loop in ~ImplPrnQueueList deleting stuff on a global data structure
+    if (fn == SRCDIR "/vcl/inc/print.h")
+        return;
+    // painful linked list
+    if (fn == SRCDIR "/basic/source/inc/runtime.hxx")
+        return;
+    // not sure how the node management is working here
+    if (fn == SRCDIR "/i18npool/source/localedata/saxparser.cxx")
+        return;
+    // has a pointer that it only sometimes owns
+    if (fn == SRCDIR "/editeng/source/editeng/impedit.hxx")
+        return;
 
     report(
         DiagnosticsEngine::Warning,
-        "deleting a local variable at the end of a block, is a sure sign it should be using std::unique_ptr for that var",
+        message,
         compat::getBeginLoc(deleteExpr))
         << deleteExpr->getSourceRange();
     report(
         DiagnosticsEngine::Note,
-        "var is here",
-        compat::getBeginLoc(varDecl))
-        << varDecl->getSourceRange();
-    return true;
+        "member is here",
+        compat::getBeginLoc(fieldDecl))
+        << fieldDecl->getSourceRange();
 }
 
 bool UseUniquePtr::TraverseFunctionDecl(FunctionDecl* functionDecl)
@@ -749,6 +849,21 @@ bool UseUniquePtr::TraverseCXXMethodDecl(CXXMethodDecl* methodDecl)
     return ret;
 }
 
+#if CLANG_VERSION >= 50000
+bool UseUniquePtr::TraverseCXXDeductionGuideDecl(CXXDeductionGuideDecl* methodDecl)
+{
+    if (ignoreLocation(methodDecl))
+        return true;
+
+    auto oldCurrent = mpCurrentFunctionDecl;
+    mpCurrentFunctionDecl = methodDecl;
+    bool ret = RecursiveASTVisitor::TraverseCXXDeductionGuideDecl(methodDecl);
+    mpCurrentFunctionDecl = oldCurrent;
+
+    return ret;
+}
+#endif
+
 bool UseUniquePtr::TraverseCXXConstructorDecl(CXXConstructorDecl* methodDecl)
 {
     if (ignoreLocation(methodDecl))
@@ -762,6 +877,64 @@ bool UseUniquePtr::TraverseCXXConstructorDecl(CXXConstructorDecl* methodDecl)
     return ret;
 }
 
+bool UseUniquePtr::TraverseCXXConversionDecl(CXXConversionDecl* methodDecl)
+{
+    if (ignoreLocation(methodDecl))
+        return true;
+
+    auto oldCurrent = mpCurrentFunctionDecl;
+    mpCurrentFunctionDecl = methodDecl;
+    bool ret = RecursiveASTVisitor::TraverseCXXConversionDecl(methodDecl);
+    mpCurrentFunctionDecl = oldCurrent;
+
+    return ret;
+}
+
+bool UseUniquePtr::TraverseCXXDestructorDecl(CXXDestructorDecl* methodDecl)
+{
+    if (ignoreLocation(methodDecl))
+        return true;
+
+    auto oldCurrent = mpCurrentFunctionDecl;
+    mpCurrentFunctionDecl = methodDecl;
+    bool ret = RecursiveASTVisitor::TraverseCXXDestructorDecl(methodDecl);
+    mpCurrentFunctionDecl = oldCurrent;
+
+    return ret;
+}
+
+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_or_null<CXXConstructExpr>(ctorInit->getInit());
+    if (!constructExpr || constructExpr->getNumArgs() == 0)
+        return true;
+    auto init = constructExpr->getArg(0)->IgnoreImpCasts();
+    if (!isa<DeclRefExpr>(init))
+        return true;
+
+    StringRef fn = getFileNameOfSpellingLoc(compiler.getSourceManager().getSpellingLoc(ctorInit->getSourceLocation()));
+    // don't feel like fiddling with the yacc parser
+    if (loplugin::hasPathnamePrefix(fn, SRCDIR "/idlc/"))
+        return true;
+    // cannot change URE
+    if (loplugin::hasPathnamePrefix(fn, SRCDIR "/cppu/source/helper/purpenv/helper_purpenv_Environment.cxx"))
+        return true;
+
+    report(
+        DiagnosticsEngine::Warning,
+        "should be passing via std::unique_ptr param",
+        ctorInit->getSourceLocation())
+        << ctorInit->getSourceRange();
+    return RecursiveASTVisitor<UseUniquePtr>::TraverseConstructorInitializer(ctorInit);
+}
+
+// Only checks for calls to delete on a pointer param
 bool UseUniquePtr::VisitCXXDeleteExpr(const CXXDeleteExpr* deleteExpr)
 {
     if (!mpCurrentFunctionDecl)
@@ -945,37 +1118,6 @@ 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_or_null<CXXConstructExpr>(ctorInit->getInit());
-    if (!constructExpr || constructExpr->getNumArgs() == 0)
-        return true;
-    auto init = constructExpr->getArg(0)->IgnoreImpCasts();
-    if (!isa<DeclRefExpr>(init))
-        return true;
-
-    StringRef fn = getFileNameOfSpellingLoc(compiler.getSourceManager().getSpellingLoc(ctorInit->getSourceLocation()));
-    // don't feel like fiddling with the yacc parser
-    if (loplugin::hasPathnamePrefix(fn, SRCDIR "/idlc/"))
-        return true;
-    // cannot change URE
-    if (loplugin::hasPathnamePrefix(fn, SRCDIR "/cppu/source/helper/purpenv/helper_purpenv_Environment.cxx"))
-        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);