summaryrefslogtreecommitdiff
path: root/compilerplugins
diff options
context:
space:
mode:
Diffstat (limited to 'compilerplugins')
-rw-r--r--compilerplugins/clang/test/useuniqueptr.cxx24
-rw-r--r--compilerplugins/clang/useuniqueptr.cxx106
2 files changed, 108 insertions, 22 deletions
diff --git a/compilerplugins/clang/test/useuniqueptr.cxx b/compilerplugins/clang/test/useuniqueptr.cxx
index 564e93ccbdc0..e834123264e0 100644
--- a/compilerplugins/clang/test/useuniqueptr.cxx
+++ b/compilerplugins/clang/test/useuniqueptr.cxx
@@ -8,13 +8,33 @@
*/
-class Foo {
+class Foo1 {
char* m_pbar; // expected-note {{member is here [loplugin:useuniqueptr]}}
- ~Foo()
+ ~Foo1()
{
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;
}
};
+
+class Foo2 {
+ char* m_pbar1; // expected-note {{member is here [loplugin:useuniqueptr]}}
+ char* m_pbar2; // expected-note {{member is here [loplugin:useuniqueptr]}}
+ ~Foo2()
+ {
+ delete[] m_pbar1; // expected-error {{managing array of trival type 'char' manually, rather use std::vector / std::array / std::unique_ptr [loplugin:useuniqueptr]}}
+ delete[] m_pbar2; // expected-error {{managing array of trival type 'char' manually, rather use std::vector / std::array / std::unique_ptr [loplugin:useuniqueptr]}}
+ }
+};
+
+class Foo3 {
+ char* m_pbar;
+ bool bMine;
+ ~Foo3()
+ {
+ if (bMine)
+ delete[] m_pbar;
+ }
+};
/* 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 afae4c3c770a..9e0dd33e900b 100644
--- a/compilerplugins/clang/useuniqueptr.cxx
+++ b/compilerplugins/clang/useuniqueptr.cxx
@@ -33,8 +33,11 @@ public:
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
}
- bool VisitCXXDestructorDecl(const CXXDestructorDecl * );
- bool VisitCompoundStmt(const CompoundStmt * );
+ bool VisitCXXDestructorDecl(const CXXDestructorDecl* );
+ bool VisitCompoundStmt(const CompoundStmt* );
+private:
+ void CheckForSingleUnconditionalDelete(const CXXDestructorDecl*, const CompoundStmt* );
+ void CheckForDeleteArrayOfPOD(const CompoundStmt* );
};
bool UseUniquePtr::VisitCXXDestructorDecl(const CXXDestructorDecl* destructorDecl)
@@ -48,6 +51,14 @@ bool UseUniquePtr::VisitCXXDestructorDecl(const CXXDestructorDecl* destructorDec
if (!compoundStmt)
return true;
+ CheckForSingleUnconditionalDelete(destructorDecl, compoundStmt);
+ CheckForDeleteArrayOfPOD(compoundStmt);
+
+ return true;
+}
+
+void UseUniquePtr::CheckForSingleUnconditionalDelete(const CXXDestructorDecl* destructorDecl, const CompoundStmt* compoundStmt)
+{
const CXXDeleteExpr* deleteExpr = nullptr;
if (compoundStmt->size() == 1) {
deleteExpr = dyn_cast<CXXDeleteExpr>(compoundStmt->body_front());
@@ -66,60 +77,60 @@ bool UseUniquePtr::VisitCXXDestructorDecl(const CXXDestructorDecl* destructorDec
deleteExpr = dyn_cast<CXXDeleteExpr>(compoundStmt->body_front());
}
} else {
- return true;
+ return;
}
if (deleteExpr == nullptr)
- return true;
+ return;
const ImplicitCastExpr* pCastExpr = dyn_cast<ImplicitCastExpr>(deleteExpr->getArgument());
if (!pCastExpr)
- return true;
+ return;
const MemberExpr* pMemberExpr = dyn_cast<MemberExpr>(pCastExpr->getSubExpr());
if (!pMemberExpr)
- return true;
+ return;
// ignore union games
const FieldDecl* pFieldDecl = dyn_cast<FieldDecl>(pMemberExpr->getMemberDecl());
if (!pFieldDecl)
- return true;
+ return;
TagDecl const * td = dyn_cast<TagDecl>(pFieldDecl->getDeclContext());
if (td->isUnion())
- return true;
+ return;
// ignore calling delete on someone else's field
if (pFieldDecl->getParent() != destructorDecl->getParent() )
- return true;
+ return;
if (ignoreLocation(pFieldDecl))
- return true;
+ return;
// to ignore things like the CPPUNIT macros
StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(pFieldDecl->getLocStart()));
if (loplugin::hasPathnamePrefix(aFileName, WORKDIR))
- return true;
+ return;
// passes and stores pointers to member fields
if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sot/source/sdstor/stgdir.hxx"))
- return true;
+ return;
// something platform-specific
if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/hwpfilter/source/htags.h"))
- return true;
+ return;
// @TODO there is clearly a bug in the ownership here, the operator= method cannot be right
if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/include/formula/formdata.hxx"))
- return true;
+ return;
// passes pointers to member fields
if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sd/inc/sdpptwrp.hxx"))
- return true;
+ return;
// @TODO there is clearly a bug in the ownership here, the ScJumpMatrixToken copy constructor cannot be right
if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sc/inc/token.hxx"))
- return true;
+ return;
// @TODO intrusive linked-lists here, with some trickiness
if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/source/filter/html/parcss1.hxx"))
- return true;
+ return;
// @TODO SwDoc has some weird ref-counting going on
if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/inc/shellio.hxx"))
- return true;
+ return;
// @TODO it's sharing pointers with another class
if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sc/inc/formulacell.hxx"))
- return true;
+ return;
report(
DiagnosticsEngine::Warning,
@@ -131,7 +142,6 @@ bool UseUniquePtr::VisitCXXDestructorDecl(const CXXDestructorDecl* destructorDec
"member is here",
pFieldDecl->getLocStart())
<< pFieldDecl->getSourceRange();
- return true;
}
bool UseUniquePtr::VisitCompoundStmt(const CompoundStmt* compoundStmt)
@@ -185,6 +195,62 @@ bool UseUniquePtr::VisitCompoundStmt(const CompoundStmt* compoundStmt)
return true;
}
+void UseUniquePtr::CheckForDeleteArrayOfPOD(const CompoundStmt* compoundStmt)
+{
+ for (auto i = compoundStmt->body_begin();
+ i != compoundStmt->body_end(); ++i)
+ {
+ auto deleteExpr = dyn_cast<CXXDeleteExpr>(*i);
+ if (!deleteExpr || !deleteExpr->isArrayForm())
+ continue;
+
+ const Expr* argExpr = deleteExpr->getArgument();
+ if (auto implicitCastExpr = dyn_cast<ImplicitCastExpr>(deleteExpr->getArgument()))
+ argExpr = implicitCastExpr->getSubExpr();
+
+ auto memberExpr = dyn_cast<MemberExpr>(argExpr);
+ if (!memberExpr)
+ continue;
+ auto fieldDecl = dyn_cast<FieldDecl>(memberExpr->getMemberDecl());
+ if (!fieldDecl)
+ continue;
+ // ignore union games
+ auto * tagDecl = dyn_cast<TagDecl>(fieldDecl->getDeclContext());
+ if (tagDecl->isUnion())
+ continue;
+
+ auto pointerType = dyn_cast<PointerType>(fieldDecl->getType()->getUnqualifiedDesugaredType());
+ QualType elementType = pointerType->getPointeeType();
+ if (!elementType.isTrivialType(compiler.getASTContext()))
+ continue;
+
+ StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocStart()));
+ // TODO ignore this for now, it's just so messy to fix
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/include/tools/stream.hxx"))
+ continue;
+ // TODO this is very performance sensitive code
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/include/svl/itemset.hxx"))
+ continue;
+ // WW8TabBandDesc is playing games with copying/assigning itself
+ if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/source/filter/ww8/ww8par.hxx"))
+ continue;
+
+ report(
+ DiagnosticsEngine::Warning,
+ "managing array of trival type %0 manually, rather use std::vector / std::array / std::unique_ptr",
+ deleteExpr->getLocStart())
+ << elementType
+ << deleteExpr->getSourceRange();
+ report(
+ DiagnosticsEngine::Note,
+ "member is here",
+ fieldDecl->getLocStart())
+ << fieldDecl->getSourceRange();
+ }
+}
+
+
+
loplugin::Plugin::Registration< UseUniquePtr > X("useuniqueptr");
}
generated by cgit (git 2.34.1) at 2025-01-18 19:28:28 +0000