[Libreoffice-commits] core.git: 2 commits - compilerplugins/clang filter/source
Julien Nabet
serval2412 at yahoo.fr
Fri Jan 12 06:03:07 UTC 2018
compilerplugins/clang/test/useuniqueptr.cxx | 18 +++
compilerplugins/clang/useuniqueptr.cxx | 158 ++++++++++++++++++----------
filter/source/msfilter/msvbahelper.cxx | 2
3 files changed, 121 insertions(+), 57 deletions(-)
New commits:
commit a4d02f2522dd78c8f6a65b07e16638554dd4da97
Author: Julien Nabet <serval2412 at yahoo.fr>
Date: Thu Jan 11 20:27:28 2018 +0100
Fix OSL_ENSURE condition in msvbahelper.cxx
Change-Id: I13fceac8779e3000217b3d78281b8a351ba5e371
Reviewed-on: https://gerrit.libreoffice.org/47774
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Julien Nabet <serval2412 at yahoo.fr>
diff --git a/filter/source/msfilter/msvbahelper.cxx b/filter/source/msfilter/msvbahelper.cxx
index 286a352705bc..9640b8895c6f 100644
--- a/filter/source/msfilter/msvbahelper.cxx
+++ b/filter/source/msfilter/msvbahelper.cxx
@@ -545,7 +545,7 @@ uno::Sequence< OUString > SAL_CALL VBAMacroResolver::getSupportedServiceNames()
void SAL_CALL VBAMacroResolver::initialize( const uno::Sequence< uno::Any >& rArgs )
{
- OSL_ENSURE( rArgs.getLength() < 2, "VBAMacroResolver::initialize - missing arguments" );
+ OSL_ENSURE( rArgs.getLength() > 1, "VBAMacroResolver::initialize - missing arguments" );
if( rArgs.getLength() < 2 )
throw uno::RuntimeException();
commit 22bba5f377b9261fd2aecf3a20a9fc59e5d9fda3
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Wed Jan 10 16:23:41 2018 +0200
teach useuniqueptr loplugin about "if(field != null) delete field"
Change-Id: I938deef90c8d6ceb0e72ab3f6ee2cbddc6f72b8d
Reviewed-on: https://gerrit.libreoffice.org/47730
Tested-by: Jenkins <ci at libreoffice.org>
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 2ffdecc33a04..43002ec59a68 100644
--- a/compilerplugins/clang/test/useuniqueptr.cxx
+++ b/compilerplugins/clang/test/useuniqueptr.cxx
@@ -94,4 +94,22 @@ class Foo8 {
delete m_pbar2; // expected-error {{unconditional call to delete on a member, should be using std::unique_ptr [loplugin:useuniqueptr]}}
}
};
+class Foo9 {
+ XXX* m_pbar1; // expected-note {{member is here [loplugin:useuniqueptr]}}
+ XXX* m_pbar2; // expected-note {{member is here [loplugin:useuniqueptr]}}
+ XXX* m_pbar3; // expected-note {{member is here [loplugin:useuniqueptr]}}
+ ~Foo9()
+ {
+ if (m_pbar1)
+ {
+ delete m_pbar1; // expected-error {{unconditional call to delete on a member, should be using std::unique_ptr [loplugin:useuniqueptr]}}
+ }
+ if (m_pbar2 != nullptr)
+ {
+ delete m_pbar2; // expected-error {{unconditional call to delete on a member, should be using std::unique_ptr [loplugin:useuniqueptr]}}
+ }
+ if (m_pbar3 != nullptr)
+ delete m_pbar3; // expected-error {{unconditional call to delete on a member, should be using std::unique_ptr [loplugin:useuniqueptr]}}
+ }
+};
/* 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 7524f7662e3c..99ef6928533e 100644
--- a/compilerplugins/clang/useuniqueptr.cxx
+++ b/compilerplugins/clang/useuniqueptr.cxx
@@ -39,6 +39,7 @@ public:
bool VisitCompoundStmt(const CompoundStmt* );
private:
void CheckForUnconditionalDelete(const CXXDestructorDecl*, const CompoundStmt* );
+ void CheckDeleteExpr(const CXXDestructorDecl*, const CXXDeleteExpr* );
void CheckForForLoopDelete(const CXXDestructorDecl*, const CompoundStmt* );
void CheckForRangedLoopDelete(const CXXDestructorDecl*, const CompoundStmt* );
};
@@ -66,69 +67,114 @@ void UseUniquePtr::CheckForUnconditionalDelete(const CXXDestructorDecl* destruct
for (auto i = compoundStmt->body_begin(); i != compoundStmt->body_end(); ++i)
{
auto deleteExpr = dyn_cast<CXXDeleteExpr>(*i);
- if (!deleteExpr)
- continue;
-
- const ImplicitCastExpr* pCastExpr = dyn_cast<ImplicitCastExpr>(deleteExpr->getArgument());
- if (!pCastExpr)
+ if (deleteExpr)
+ {
+ CheckDeleteExpr(destructorDecl, deleteExpr);
continue;
- const MemberExpr* pMemberExpr = dyn_cast<MemberExpr>(pCastExpr->getSubExpr());
- if (!pMemberExpr)
+ }
+ // Check for conditional deletes like:
+ // if (m_pField != nullptr) delete m_pField;
+ auto ifStmt = dyn_cast<IfStmt>(*i);
+ if (!ifStmt)
continue;
-
- // ignore union games
- const FieldDecl* pFieldDecl = dyn_cast<FieldDecl>(pMemberExpr->getMemberDecl());
- if (!pFieldDecl)
+ auto cond = ifStmt->getCond()->IgnoreImpCasts();
+ if (auto ifCondMemberExpr = dyn_cast<MemberExpr>(cond))
+ {
+ // ignore "if (bMine)"
+ if (!loplugin::TypeCheck(ifCondMemberExpr->getType()).Pointer())
+ continue;
+ }
+ else if (auto binaryOp = dyn_cast<BinaryOperator>(cond))
+ {
+ if (!isa<MemberExpr>(binaryOp->getLHS()->IgnoreImpCasts()))
+ continue;
+ }
+ else
continue;
- TagDecl const * td = dyn_cast<TagDecl>(pFieldDecl->getDeclContext());
- if (td->isUnion())
+ deleteExpr = dyn_cast<CXXDeleteExpr>(ifStmt->getThen());
+ if (deleteExpr)
+ {
+ CheckDeleteExpr(destructorDecl, deleteExpr);
continue;
-
- // ignore calling delete on someone else's field
- if (pFieldDecl->getParent() != destructorDecl->getParent() )
+ }
+ auto ifThenCompoundStmt = dyn_cast<CompoundStmt>(ifStmt->getThen());
+ if (!ifThenCompoundStmt)
continue;
+ for (auto j = ifThenCompoundStmt->body_begin(); j != ifThenCompoundStmt->body_end(); ++j)
+ {
+ auto ifDeleteExpr = dyn_cast<CXXDeleteExpr>(*j);
+ if (ifDeleteExpr)
+ CheckDeleteExpr(destructorDecl, ifDeleteExpr);
+ }
+ }
+}
- if (ignoreLocation(pFieldDecl))
- continue;
- // to ignore things like the CPPUNIT macros
- StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(pFieldDecl->getLocStart()));
- if (loplugin::hasPathnamePrefix(aFileName, WORKDIR))
- continue;
- // passes and stores pointers to member fields
- if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sot/source/sdstor/stgdir.hxx"))
- continue;
- // something platform-specific
- if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/hwpfilter/source/htags.h"))
- continue;
- // passes pointers to member fields
- if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sd/inc/sdpptwrp.hxx"))
- continue;
- // @TODO intrusive linked-lists here, with some trickiness
- if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/source/filter/html/parcss1.hxx"))
- continue;
- // @TODO SwDoc has some weird ref-counting going on
- if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/inc/shellio.hxx"))
- continue;
- // @TODO it's sharing pointers with another class
- if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sc/inc/formulacell.hxx"))
- continue;
- // some weird stuff going on here around struct Entity
- if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sax/"))
- continue;
- if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/include/sax/"))
- continue;
+void UseUniquePtr::CheckDeleteExpr(const CXXDestructorDecl* destructorDecl, const CXXDeleteExpr* deleteExpr)
+{
+ const ImplicitCastExpr* pCastExpr = dyn_cast<ImplicitCastExpr>(deleteExpr->getArgument());
+ if (!pCastExpr)
+ return;
+ const MemberExpr* pMemberExpr = dyn_cast<MemberExpr>(pCastExpr->getSubExpr());
+ if (!pMemberExpr)
+ return;
+
+ // ignore union games
+ const FieldDecl* pFieldDecl = dyn_cast<FieldDecl>(pMemberExpr->getMemberDecl());
+ if (!pFieldDecl)
+ return;
+ TagDecl const * td = dyn_cast<TagDecl>(pFieldDecl->getDeclContext());
+ if (td->isUnion())
+ return;
+
+ // ignore calling delete on someone else's field
+ if (pFieldDecl->getParent() != destructorDecl->getParent() )
+ return;
+
+ if (ignoreLocation(pFieldDecl))
+ return;
+ // to ignore things like the CPPUNIT macros
+ StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(pFieldDecl->getLocStart()));
+ if (loplugin::hasPathnamePrefix(aFileName, WORKDIR))
+ return;
+ // passes and stores pointers to member fields
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sot/source/sdstor/stgdir.hxx"))
+ return;
+ // something platform-specific
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/hwpfilter/source/htags.h"))
+ return;
+ // passes pointers to member fields
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sd/inc/sdpptwrp.hxx"))
+ return;
+ // @TODO intrusive linked-lists here, with some trickiness
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/source/filter/html/parcss1.hxx"))
+ return;
+ // @TODO SwDoc has some weird ref-counting going on
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/inc/shellio.hxx"))
+ return;
+ // @TODO it's sharing pointers with another class
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sc/inc/formulacell.hxx"))
+ return;
+ // some weird stuff going on here around struct Entity
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sax/"))
+ return;
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/include/sax/"))
+ return;
+ // manipulation of tree structures ie. StgAvlNode, don't lend themselves to std::unique_ptr
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sot/"))
+ return;
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/include/sot/"))
+ return;
- report(
- DiagnosticsEngine::Warning,
- "unconditional call to delete on a member, should be using std::unique_ptr",
- deleteExpr->getLocStart())
- << deleteExpr->getSourceRange();
- report(
- DiagnosticsEngine::Note,
- "member is here",
- pFieldDecl->getLocStart())
- << pFieldDecl->getSourceRange();
- }
+ report(
+ DiagnosticsEngine::Warning,
+ "unconditional call to delete on a member, should be using std::unique_ptr",
+ deleteExpr->getLocStart())
+ << deleteExpr->getSourceRange();
+ report(
+ DiagnosticsEngine::Note,
+ "member is here",
+ pFieldDecl->getLocStart())
+ << pFieldDecl->getSourceRange();
}
void UseUniquePtr::CheckForForLoopDelete(const CXXDestructorDecl* destructorDecl, const CompoundStmt* compoundStmt)
More information about the Libreoffice-commits
mailing list