[Libreoffice-commits] core.git: compilerplugins/clang
Libreoffice Gerrit user
logerrit at kemper.freedesktop.org
Fri Oct 5 14:19:28 UTC 2018
compilerplugins/clang/test/useuniqueptr.cxx | 46 ++
compilerplugins/clang/useuniqueptr.cxx | 500 +++++++++++++++++-----------
2 files changed, 366 insertions(+), 180 deletions(-)
New commits:
commit 05a337e297eb0cfe88c99503d760bd9eaf495b7d
Author: Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Fri Oct 5 09:49:57 2018 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Fri Oct 5 16:18:57 2018 +0200
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 at collabora.co.uk>
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);
More information about the Libreoffice-commits
mailing list