basctl/source/basicide/moduldl2.cxx | 2 basctl/source/basicide/scriptdocument.cxx | 6 - basic/source/classes/sbunoobj.cxx | 2 canvas/source/opengl/ogl_spritedevicehelper.cxx | 4 canvas/source/tools/page.cxx | 2 canvas/source/tools/pagemanager.cxx | 4 canvas/source/tools/surfaceproxymanager.cxx | 2 canvas/source/vcl/canvas.cxx | 2 canvas/source/vcl/canvasbitmaphelper.cxx | 2 canvas/source/vcl/canvascustomsprite.cxx | 6 - canvas/source/vcl/canvashelper.cxx | 4 canvas/source/vcl/canvashelper_texturefill.cxx | 4 canvas/source/vcl/spritecanvas.cxx | 2 canvas/source/vcl/spritedevicehelper.cxx | 2 compilerplugins/clang/makeshared.cxx | 137 ++++++++++++++++++++++++ compilerplugins/clang/test/makeshared.cxx | 43 +++++++ solenv/CompilerTest_compilerplugins_clang.mk | 1 17 files changed, 203 insertions(+), 22 deletions(-)
New commits: commit 8d23f9c2c1e0479a95cb44a09066740213b0f99a Author: Noel Grandin <noel.gran...@collabora.co.uk> AuthorDate: Thu Jan 23 15:17:46 2020 +0200 Commit: Noel Grandin <noel.gran...@collabora.co.uk> CommitDate: Fri Jan 24 07:18:28 2020 +0100 loplugin:makeshared in basctl..canvas Change-Id: I1461da594db222abbaeccfb636194b9790f5dbe8 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/87271 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> diff --git a/basctl/source/basicide/moduldl2.cxx b/basctl/source/basicide/moduldl2.cxx index 38e07178d33d..c0ab18750539 100644 --- a/basctl/source/basicide/moduldl2.cxx +++ b/basctl/source/basicide/moduldl2.cxx @@ -637,7 +637,7 @@ void LibPage::InsertLib() // library import dialog if (!xLibDlg) { - xLibDlg.reset(new LibDialog(m_pDialog->getDialog())); + xLibDlg = std::make_shared<LibDialog>(m_pDialog->getDialog()); xLibDlg->SetStorageName( aURLObj.getName() ); } diff --git a/basctl/source/basicide/scriptdocument.cxx b/basctl/source/basicide/scriptdocument.cxx index 25a705cf69df..cedadda510cf 100644 --- a/basctl/source/basicide/scriptdocument.cxx +++ b/basctl/source/basicide/scriptdocument.cxx @@ -1017,19 +1017,19 @@ namespace basctl ScriptDocument::ScriptDocument() - :m_pImpl(new Impl) + :m_pImpl(std::make_shared<Impl>()) { } ScriptDocument::ScriptDocument( ScriptDocument::SpecialDocument _eType ) - :m_pImpl( new Impl( Reference< XModel >() ) ) + :m_pImpl( std::make_shared<Impl>( Reference< XModel >() ) ) { OSL_ENSURE( _eType == NoDocument, "ScriptDocument::ScriptDocument: unknown SpecialDocument type!" ); } ScriptDocument::ScriptDocument( const Reference< XModel >& _rxDocument ) - :m_pImpl( new Impl( _rxDocument ) ) + :m_pImpl( std::make_shared<Impl>( _rxDocument ) ) { OSL_ENSURE( _rxDocument.is(), "ScriptDocument::ScriptDocument: document must not be NULL!" ); // a NULL document results in an uninitialized instance, and for this diff --git a/basic/source/classes/sbunoobj.cxx b/basic/source/classes/sbunoobj.cxx index 4b84fffe5ba8..ab3d5e54be40 100644 --- a/basic/source/classes/sbunoobj.cxx +++ b/basic/source/classes/sbunoobj.cxx @@ -2362,7 +2362,7 @@ SbUnoObject::SbUnoObject( const OUString& aName_, const Any& aUnoObj_ ) bSetClassName = true; } StructRefInfo aThisStruct( maTmpUnoObj, maTmpUnoObj.getValueType(), 0 ); - maStructInfo.reset( new SbUnoStructRefObject( GetName(), aThisStruct ) ); + maStructInfo = std::make_shared<SbUnoStructRefObject>( GetName(), aThisStruct ); } else if( eType == TypeClass_INTERFACE ) { diff --git a/canvas/source/opengl/ogl_spritedevicehelper.cxx b/canvas/source/opengl/ogl_spritedevicehelper.cxx index be838393c5fd..6a1d935c9ba9 100644 --- a/canvas/source/opengl/ogl_spritedevicehelper.cxx +++ b/canvas/source/opengl/ogl_spritedevicehelper.cxx @@ -78,7 +78,7 @@ namespace oglcanvas mpSpriteCanvas(nullptr), maActiveSprites(), maLastUpdate(), - mpTextureCache(new TextureCache()), + mpTextureCache(std::make_shared<TextureCache>()), mnLinearTwoColorGradientProgram(0), mnLinearMultiColorGradientProgram(0), mnRadialTwoColorGradientProgram(0), @@ -543,7 +543,7 @@ namespace oglcanvas IBufferContextSharedPtr SpriteDeviceHelper::createBufferContext(const ::basegfx::B2IVector& rSize) const { - return IBufferContextSharedPtr(new BufferContextImpl(rSize)); + return std::make_shared<BufferContextImpl>(rSize); } TextureCache& SpriteDeviceHelper::getTextureCache() const diff --git a/canvas/source/tools/page.cxx b/canvas/source/tools/page.cxx index 78f9cd3aa671..3537fa0b6873 100644 --- a/canvas/source/tools/page.cxx +++ b/canvas/source/tools/page.cxx @@ -48,7 +48,7 @@ namespace canvas SurfaceRect rect(rSize); if(insert(rect)) { - FragmentSharedPtr pFragment(new PageFragment(rect,this)); + FragmentSharedPtr pFragment = std::make_shared<PageFragment>(rect,this); mpFragments.push_back(pFragment); return pFragment; } diff --git a/canvas/source/tools/pagemanager.cxx b/canvas/source/tools/pagemanager.cxx index 3d08eff21593..c1e874ca498a 100644 --- a/canvas/source/tools/pagemanager.cxx +++ b/canvas/source/tools/pagemanager.cxx @@ -42,7 +42,7 @@ namespace canvas } // otherwise try to create a new page and allocate space there... - PageSharedPtr pPage(new Page(mpRenderModule)); + PageSharedPtr pPage = std::make_shared<Page>(mpRenderModule); if(pPage->isValid()) { maPages.push_back(pPage); @@ -56,7 +56,7 @@ namespace canvas // of videomemory], and all other pages could not take // the new request. we decide to create a 'naked' fragment // which will receive its location later. - FragmentSharedPtr pFragment(new PageFragment(rSize)); + FragmentSharedPtr pFragment = std::make_shared<PageFragment>(rSize); maFragments.push_back(pFragment); return pFragment; } diff --git a/canvas/source/tools/surfaceproxymanager.cxx b/canvas/source/tools/surfaceproxymanager.cxx index 126e1bd8c2ad..87aa953cf213 100644 --- a/canvas/source/tools/surfaceproxymanager.cxx +++ b/canvas/source/tools/surfaceproxymanager.cxx @@ -33,7 +33,7 @@ namespace canvas public: explicit SurfaceProxyManager( const std::shared_ptr<IRenderModule>& rRenderModule ) : - mpPageManager( new PageManager(rRenderModule) ) + mpPageManager( std::make_shared<PageManager>(rRenderModule) ) { } diff --git a/canvas/source/vcl/canvas.cxx b/canvas/source/vcl/canvas.cxx index 9cbb1c0d7467..4713f8ab9d34 100644 --- a/canvas/source/vcl/canvas.cxx +++ b/canvas/source/vcl/canvas.cxx @@ -87,7 +87,7 @@ namespace vclcanvas if( !pOutDev ) throw lang::NoSupportException("Passed OutDev invalid!", nullptr); - OutDevProviderSharedPtr pOutdevProvider( new OutDevHolder(*pOutDev) ); + OutDevProviderSharedPtr pOutdevProvider = std::make_shared<OutDevHolder>(*pOutDev); // setup helper maDeviceHelper.init( pOutdevProvider ); diff --git a/canvas/source/vcl/canvasbitmaphelper.cxx b/canvas/source/vcl/canvasbitmaphelper.cxx index 44c6dcce1230..43fed2754430 100644 --- a/canvas/source/vcl/canvasbitmaphelper.cxx +++ b/canvas/source/vcl/canvasbitmaphelper.cxx @@ -45,7 +45,7 @@ namespace vclcanvas const OutDevProviderSharedPtr& rOutDevReference ) { mpOutDevReference = rOutDevReference; - mpBackBuffer.reset( new BitmapBackBuffer( rBitmap, rOutDevReference->getOutDev() )); + mpBackBuffer = std::make_shared<BitmapBackBuffer>( rBitmap, rOutDevReference->getOutDev() ); // forward new settings to base class (ref device, output // surface, no protection (own backbuffer), alpha depends on diff --git a/canvas/source/vcl/canvascustomsprite.cxx b/canvas/source/vcl/canvascustomsprite.cxx index 5a92bc64ef10..74ed15836004 100644 --- a/canvas/source/vcl/canvascustomsprite.cxx +++ b/canvas/source/vcl/canvascustomsprite.cxx @@ -55,12 +55,12 @@ namespace vclcanvas ceil( rSpriteSize.Height ))) ); // create content backbuffer in screen depth - BackBufferSharedPtr pBackBuffer( new BackBuffer( rOutDevProvider->getOutDev() ) ); + BackBufferSharedPtr pBackBuffer = std::make_shared<BackBuffer>( rOutDevProvider->getOutDev() ); pBackBuffer->setSize( aSize ); // create mask backbuffer, with one bit color depth - BackBufferSharedPtr pBackBufferMask( new BackBuffer( rOutDevProvider->getOutDev(), - true ) ); + BackBufferSharedPtr pBackBufferMask = std::make_shared<BackBuffer>( rOutDevProvider->getOutDev(), + true ); pBackBufferMask->setSize( aSize ); // TODO(F1): Implement alpha vdev (could prolly enable diff --git a/canvas/source/vcl/canvashelper.cxx b/canvas/source/vcl/canvashelper.cxx index d1e1de400687..7a1578bef92b 100644 --- a/canvas/source/vcl/canvashelper.cxx +++ b/canvas/source/vcl/canvashelper.cxx @@ -776,7 +776,7 @@ namespace vclcanvas const double nAngleInTenthOfDegrees (3600.0 - nRotate * 3600.0 / (2*M_PI)); aGrfAttr.SetRotation( static_cast< sal_uInt16 >(::basegfx::fround(nAngleInTenthOfDegrees)) ); - pGrfObj.reset( new GraphicObject( aBmpEx ) ); + pGrfObj = std::make_shared<GraphicObject>( aBmpEx ); } else { @@ -799,7 +799,7 @@ namespace vclcanvas aBmpEx = tools::transformBitmap( aBmpEx, aMatrix ); - pGrfObj.reset( new GraphicObject( aBmpEx ) ); + pGrfObj = std::make_shared<GraphicObject>( aBmpEx ); // clear scale values, generated bitmap already // contains scaling diff --git a/canvas/source/vcl/canvashelper_texturefill.cxx b/canvas/source/vcl/canvashelper_texturefill.cxx index 3650c8317513..bd17a8883d21 100644 --- a/canvas/source/vcl/canvashelper_texturefill.cxx +++ b/canvas/source/vcl/canvashelper_texturefill.cxx @@ -756,7 +756,7 @@ namespace vclcanvas const double nAngleInTenthOfDegrees (3600.0 - nRotate * 3600.0 / (2*M_PI)); aGrfAttr.SetRotation( static_cast< sal_uInt16 >(::basegfx::fround(nAngleInTenthOfDegrees)) ); - pGrfObj.reset( new GraphicObject( aBmpEx ) ); + pGrfObj = std::make_shared<GraphicObject>( aBmpEx ); } else { @@ -779,7 +779,7 @@ namespace vclcanvas aBmpEx = tools::transformBitmap( aBmpEx, aTotalTransform); - pGrfObj.reset( new GraphicObject( aBmpEx ) ); + pGrfObj = std::make_shared<GraphicObject>( aBmpEx ); // clear scale values, generated bitmap already // contains scaling diff --git a/canvas/source/vcl/spritecanvas.cxx b/canvas/source/vcl/spritecanvas.cxx index f40be1a54049..90aa55228b28 100644 --- a/canvas/source/vcl/spritecanvas.cxx +++ b/canvas/source/vcl/spritecanvas.cxx @@ -77,7 +77,7 @@ namespace vclcanvas uno::Reference< awt::XWindow > xParentWindow; maArguments[3] >>= xParentWindow; - OutDevProviderSharedPtr pOutDev( new WindowOutDevHolder(xParentWindow) ); + OutDevProviderSharedPtr pOutDev = std::make_shared<WindowOutDevHolder>(xParentWindow); // setup helper maDeviceHelper.init( pOutDev ); diff --git a/canvas/source/vcl/spritedevicehelper.cxx b/canvas/source/vcl/spritedevicehelper.cxx index cc36193f08ec..01994f55c0d6 100644 --- a/canvas/source/vcl/spritedevicehelper.cxx +++ b/canvas/source/vcl/spritedevicehelper.cxx @@ -41,7 +41,7 @@ namespace vclcanvas // setup back buffer OutputDevice& rOutDev( pOutDev->getOutDev() ); - mpBackBuffer.reset( new BackBuffer( rOutDev )); + mpBackBuffer = std::make_shared<BackBuffer>( rOutDev ); mpBackBuffer->setSize( rOutDev.GetOutputSizePixel() ); // #i95645# commit 8dd247044f976d93e13370738418fcd2ac167e9c Author: Noel Grandin <noel.gran...@collabora.co.uk> AuthorDate: Thu Jan 23 15:06:47 2020 +0200 Commit: Noel Grandin <noel.gran...@collabora.co.uk> CommitDate: Fri Jan 24 07:18:20 2020 +0100 new loplugin:makeshared Change-Id: I4bfcf6e337a6806ab5983a3fa2e2a7b6f2af1c43 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/87270 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> diff --git a/compilerplugins/clang/makeshared.cxx b/compilerplugins/clang/makeshared.cxx new file mode 100644 index 000000000000..93aac06666ba --- /dev/null +++ b/compilerplugins/clang/makeshared.cxx @@ -0,0 +1,137 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +#ifndef LO_CLANG_SHARED_PLUGINS + +#include <cassert> +#include <string> +#include <iostream> +#include <fstream> +#include <set> + +#include <clang/AST/CXXInheritance.h> + +#include "check.hxx" +#include "compat.hxx" +#include "plugin.hxx" + +/** + * look for places we can use std::make_shared + */ + +namespace +{ +class MakeShared : public loplugin::FilteringPlugin<MakeShared> +{ +public: + explicit MakeShared(loplugin::InstantiationData const& data) + : FilteringPlugin(data) + { + } + + virtual bool preRun() override + { + StringRef fn(handler.getMainFileName()); + // uses boost::shared_ptr and we trigger because we're not looking specifically for std::shared_ptr + if (loplugin::isSamePathname(fn, SRCDIR "/ucb/source/ucp/cmis/cmis_repo_content.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/ucb/source/ucp/cmis/cmis_content.cxx")) + return false; + // TODO something weird with protected base classes going on here + if (loplugin::isSamePathname(fn, SRCDIR "/sc/source/filter/excel/xeextlst.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/sc/source/filter/excel/xecontent.cxx")) + return false; + return true; + } + + virtual void run() override + { + if (preRun()) + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } + + bool shouldVisitTemplateInstantiations() const { return true; } + + bool VisitCXXConstructExpr(CXXConstructExpr const*); + bool VisitCXXMemberCallExpr(CXXMemberCallExpr const*); +}; + +bool MakeShared::VisitCXXConstructExpr(CXXConstructExpr const* constructExpr) +{ + if (ignoreLocation(constructExpr)) + return true; + if (!loplugin::TypeCheck(constructExpr->getType()).ClassOrStruct("shared_ptr").StdNamespace()) + return true; + if (constructExpr->getNumArgs() != 1) + return true; + auto cxxNewExpr = dyn_cast<CXXNewExpr>(constructExpr->getArg(0)->IgnoreParenImpCasts()); + if (!cxxNewExpr) + return true; + auto construct2 = cxxNewExpr->getConstructExpr(); + if (construct2) + { + if (construct2->getConstructor()->getAccess() != AS_public) + return true; + if (construct2->getNumArgs() == 1 && isa<CXXStdInitializerListExpr>(construct2->getArg(0))) + return true; + } + + StringRef fn = getFilenameOfLocation( + compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(constructExpr))); + if (loplugin::isSamePathname(fn, SRCDIR "/include/o3tl/make_shared.hxx")) + return true; + + report(DiagnosticsEngine::Warning, "rather use make_shared", compat::getBeginLoc(cxxNewExpr)) + << cxxNewExpr->getSourceRange(); + return true; +} + +bool MakeShared::VisitCXXMemberCallExpr(CXXMemberCallExpr const* cxxMemberCallExpr) +{ + if (ignoreLocation(cxxMemberCallExpr)) + return true; + if (cxxMemberCallExpr->getNumArgs() != 1) + return true; + + // cannot find a way to use the loplugin::DeclCheck stuff here + auto templateDecl + = dyn_cast<ClassTemplateSpecializationDecl>(cxxMemberCallExpr->getRecordDecl()); + if (!templateDecl) + return true; + auto cxxRecordDecl = templateDecl->getSpecializedTemplate()->getTemplatedDecl(); + if (!cxxRecordDecl->getName().contains("shared_ptr")) + return true; + + if (cxxMemberCallExpr->getMethodDecl()->getName() != "reset") + return true; + auto cxxNewExpr = dyn_cast<CXXNewExpr>(cxxMemberCallExpr->getArg(0)->IgnoreParenImpCasts()); + if (!cxxNewExpr) + return true; + if (cxxNewExpr->getConstructExpr() + && cxxNewExpr->getConstructExpr()->getConstructor()->getAccess() != AS_public) + return true; + + StringRef fn = getFilenameOfLocation( + compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(cxxMemberCallExpr))); + if (loplugin::isSamePathname(fn, SRCDIR "/include/o3tl/make_shared.hxx")) + return true; + + report(DiagnosticsEngine::Warning, "rather use make_shared", compat::getBeginLoc(cxxNewExpr)) + << cxxNewExpr->getSourceRange(); + + return true; +} + +loplugin::Plugin::Registration<MakeShared> makeshared("makeshared", false); + +} // namespace + +#endif // LO_CLANG_SHARED_PLUGINS + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/compilerplugins/clang/test/makeshared.cxx b/compilerplugins/clang/test/makeshared.cxx new file mode 100644 index 000000000000..6e388428f8f1 --- /dev/null +++ b/compilerplugins/clang/test/makeshared.cxx @@ -0,0 +1,43 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include <sal/config.h> + +#include <memory> +#include <o3tl/deleter.hxx> +#include <o3tl/sorted_vector.hxx> + +struct S1 +{ + friend void test1(); + +private: + S1() {} +}; + +void test1() +{ + std::shared_ptr<int> x( + new int); // expected-error {{rather use make_shared [loplugin:makeshared]}} + x.reset(new int); // expected-error {{rather use make_shared [loplugin:makeshared]}} + x = std::shared_ptr<int>( + new int); // expected-error {{rather use make_shared [loplugin:makeshared]}} + + // no warning expected + std::shared_ptr<int> y(new int, o3tl::default_delete<int>()); + y.reset(new int, o3tl::default_delete<int>()); + // no warning expected, no public constructor + std::shared_ptr<S1> z(new S1); + z.reset(new S1); + + // no warning expected - this constructor takes an initializer-list, which maked_shared does not support + auto a = std::shared_ptr<o3tl::sorted_vector<int>>(new o3tl::sorted_vector<int>({ 1, 2 })); +}; + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk index a65969a0078e..05c470df005d 100644 --- a/solenv/CompilerTest_compilerplugins_clang.mk +++ b/solenv/CompilerTest_compilerplugins_clang.mk @@ -47,6 +47,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \ compilerplugins/clang/test/logexceptionnicely \ compilerplugins/clang/test/loopvartoosmall \ compilerplugins/clang/test/mapindex \ + compilerplugins/clang/test/makeshared \ compilerplugins/clang/test/noexceptmove \ compilerplugins/clang/test/nullptr \ compilerplugins/clang/test/oncevar \ _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits