From c66568d6b0bbcce26cbdc5a4e5f6e4c0ae748e45 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Wed, 23 Aug 2017 08:49:02 +0200 Subject: loplugin:useuniqueptr, look for containers.. that can use std::unique_ptr, and apply it in i18npool Change-Id: Ib410abaf73d5f392c7a7a9a322872b08c948f9e9 Reviewed-on: https://gerrit.libreoffice.org/41438 Reviewed-by: Noel Grandin Tested-by: Noel Grandin --- compilerplugins/clang/useuniqueptr.cxx | 133 +++++++++++++++++++++++++++++++-- 1 file changed, 128 insertions(+), 5 deletions(-) (limited to 'compilerplugins/clang/useuniqueptr.cxx') diff --git a/compilerplugins/clang/useuniqueptr.cxx b/compilerplugins/clang/useuniqueptr.cxx index 977404ab2ae6..e47ebd23450a 100644 --- a/compilerplugins/clang/useuniqueptr.cxx +++ b/compilerplugins/clang/useuniqueptr.cxx @@ -38,6 +38,8 @@ public: bool VisitCompoundStmt(const CompoundStmt* ); private: void CheckForSingleUnconditionalDelete(const CXXDestructorDecl*, const CompoundStmt* ); + void CheckForForLoopDelete(const CXXDestructorDecl*, const CompoundStmt* ); + void CheckForRangedLoopDelete(const CXXDestructorDecl*, const CompoundStmt* ); void CheckForDeleteOfPOD(const CompoundStmt* ); }; @@ -52,8 +54,13 @@ bool UseUniquePtr::VisitCXXDestructorDecl(const CXXDestructorDecl* destructorDec if (!compoundStmt) return true; - CheckForSingleUnconditionalDelete(destructorDecl, compoundStmt); - CheckForDeleteOfPOD(compoundStmt); + if (compoundStmt->size() > 0) + { + CheckForSingleUnconditionalDelete(destructorDecl, compoundStmt); + CheckForDeleteOfPOD(compoundStmt); + CheckForForLoopDelete(destructorDecl, compoundStmt); + CheckForRangedLoopDelete(destructorDecl, compoundStmt); + } return true; } @@ -78,9 +85,15 @@ void UseUniquePtr::CheckForSingleUnconditionalDelete(const CXXDestructorDecl* de deleteExpr = dyn_cast(compoundStmt->body_front()); } } else { - return; + // look for the following pattern: + // delete m_pbar; + // m_pbar = nullptr; + if (auto binaryOp = dyn_cast(compoundStmt->body_back())) { + if (binaryOp->getOpcode() == BO_Assign) + deleteExpr = dyn_cast(*(++compoundStmt->body_rbegin())); + } } - if (deleteExpr == nullptr) + if (!deleteExpr) return; const ImplicitCastExpr* pCastExpr = dyn_cast(deleteExpr->getArgument()); @@ -145,6 +158,116 @@ void UseUniquePtr::CheckForSingleUnconditionalDelete(const CXXDestructorDecl* de << pFieldDecl->getSourceRange(); } +void UseUniquePtr::CheckForForLoopDelete(const CXXDestructorDecl* destructorDecl, const CompoundStmt* compoundStmt) +{ + auto forStmt = dyn_cast(compoundStmt->body_back()); + if (!forStmt) + return; + auto deleteExpr = dyn_cast(forStmt->getBody()); + if (!deleteExpr) + return; + + const MemberExpr* memberExpr = nullptr; + const Expr* subExpr = deleteExpr->getArgument(); + for (;;) + { + subExpr = subExpr->IgnoreImpCasts(); + if ((memberExpr = dyn_cast(subExpr))) + break; + else if (auto arraySubscriptExpr = dyn_cast(subExpr)) + subExpr = arraySubscriptExpr->getBase(); + else if (auto cxxOperatorCallExpr = dyn_cast(subExpr)) + { + if (cxxOperatorCallExpr->getOperator() == OO_Subscript) + { + memberExpr = dyn_cast(cxxOperatorCallExpr->getArg(0)); + break; + } + return; + } + else + return; + } + + // ignore union games + const FieldDecl* fieldDecl = dyn_cast(memberExpr->getMemberDecl()); + if (!fieldDecl) + return; + TagDecl const * td = dyn_cast(fieldDecl->getDeclContext()); + if (td->isUnion()) + return; + + // ignore calling delete on someone else's field + if (fieldDecl->getParent() != destructorDecl->getParent() ) + return; + + if (ignoreLocation(fieldDecl)) + return; + // to ignore things like the CPPUNIT macros + StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocStart())); + if (loplugin::hasPathnamePrefix(aFileName, WORKDIR)) + return; + + report( + DiagnosticsEngine::Warning, + "rather manage with std::some_container>", + deleteExpr->getLocStart()) + << deleteExpr->getSourceRange(); + report( + DiagnosticsEngine::Note, + "member is here", + fieldDecl->getLocStart()) + << fieldDecl->getSourceRange(); +} + +void UseUniquePtr::CheckForRangedLoopDelete(const CXXDestructorDecl* destructorDecl, const CompoundStmt* compoundStmt) +{ + auto cxxForRangeStmt = dyn_cast(compoundStmt->body_back()); + if (!cxxForRangeStmt) + return; + auto deleteExpr = dyn_cast(cxxForRangeStmt->getBody()); + if (!deleteExpr) + return; + auto memberExpr = dyn_cast(cxxForRangeStmt->getRangeInit()); + if (!memberExpr) + return; + auto fieldDecl = dyn_cast(memberExpr->getMemberDecl()); + if (!fieldDecl) + return; + + // ignore union games + TagDecl const * td = dyn_cast(fieldDecl->getDeclContext()); + if (td->isUnion()) + return; + + // ignore calling delete on someone else's field + if (fieldDecl->getParent() != destructorDecl->getParent() ) + return; + + if (ignoreLocation(fieldDecl)) + return; + + // to ignore things like the CPPUNIT macros + StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocStart())); + if (loplugin::hasPathnamePrefix(aFileName, WORKDIR)) + return; + // ignore std::map and std::unordered_map, MSVC 2015 has problems with mixing these with std::unique_ptr + auto tc = loplugin::TypeCheck(fieldDecl->getType()); + if (tc.Class("map").StdNamespace() || tc.Class("unordered_map").StdNamespace()) + return; + + report( + DiagnosticsEngine::Warning, + "rather manage with std::some_container>", + deleteExpr->getLocStart()) + << deleteExpr->getSourceRange(); + report( + DiagnosticsEngine::Note, + "member is here", + fieldDecl->getLocStart()) + << fieldDecl->getSourceRange(); +} + bool UseUniquePtr::VisitCompoundStmt(const CompoundStmt* compoundStmt) { if (ignoreLocation(compoundStmt)) @@ -255,7 +378,7 @@ void UseUniquePtr::CheckForDeleteOfPOD(const CompoundStmt* compoundStmt) -loplugin::Plugin::Registration< UseUniquePtr > X("useuniqueptr"); +loplugin::Plugin::Registration< UseUniquePtr > X("useuniqueptr", false); } -- cgit