compilerplugins/clang/constfields.cxx | 593 +++++++++++ compilerplugins/clang/constfields.py | 95 + compilerplugins/clang/constfieldsrewrite.cxx | 143 ++ compilerplugins/clang/test/constfields.cxx | 45 compilerplugins/clang/unusedfields.cxx | 28 compilerplugins/clang/unusedfields.py | 32 solenv/CompilerTest_compilerplugins_clang.mk | 1 writerfilter/source/dmapper/DomainMapper.cxx | 2 writerfilter/source/dmapper/DomainMapper_Impl.cxx | 2 writerfilter/source/dmapper/PropertyMap.cxx | 1 writerfilter/source/dmapper/TextEffectsHandler.cxx | 3 writerfilter/source/ooxml/OOXMLDocumentImpl.cxx | 1 xmlhelp/source/cxxhelp/provider/db.hxx | 2 xmlhelp/source/cxxhelp/provider/provider.cxx | 1 xmlhelp/source/cxxhelp/provider/resultsetbase.cxx | 8 xmloff/qa/unit/uxmloff.cxx | 1 xmloff/source/chart/SchXMLSeries2Context.cxx | 1 xmloff/source/chart/transporttypes.hxx | 1 xmloff/source/core/RDFaExportHelper.cxx | 2 xmloff/source/core/xmlexp.cxx | 15 xmloff/source/core/xmlimp.cxx | 1 xmloff/source/draw/shapeimport.cxx | 2 xmloff/source/forms/handler/form_handler_factory.cxx | 6 xmloff/source/forms/officeforms.cxx | 1 xmloff/source/style/MultiPropertySetHelper.cxx | 2 xmloff/source/style/styleexp.cxx | 2 xmloff/source/style/xmlexppr.cxx | 3 xmloff/source/style/xmlnumfe.cxx | 8 xmloff/source/style/xmlstyle.cxx | 3 xmloff/source/text/XMLFootnoteConfigurationImportContext.cxx | 1 xmloff/source/text/txtparae.cxx | 5 xmloff/source/xforms/XFormsBindContext.cxx | 3 xmlscript/source/xml_helper/xml_impctx.cxx | 3 xmlsecurity/source/framework/saxeventkeeperimpl.cxx | 3 xmlsecurity/source/framework/xmlsignaturetemplateimpl.cxx | 3 xmlsecurity/source/helper/xsecverify.cxx | 4 36 files changed, 897 insertions(+), 130 deletions(-)
New commits: commit 5cd976bdef4334a5c28d6c7748ef5d72cf5025ff Author: Noel Grandin <noel.gran...@collabora.co.uk> AuthorDate: Wed Sep 12 17:01:48 2018 +0200 Commit: Noel Grandin <noel.gran...@collabora.co.uk> CommitDate: Thu Sep 13 08:30:22 2018 +0200 loplugin:simplifyconstruct in writerfilter..xmlsecurity Change-Id: Ic2d901ca0dbc2d6fa96611d260c1572da8a783c0 Reviewed-on: https://gerrit.libreoffice.org/60398 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx index d88a4f701ac6..257b6bbad683 100644 --- a/writerfilter/source/dmapper/DomainMapper.cxx +++ b/writerfilter/source/dmapper/DomainMapper.cxx @@ -2543,7 +2543,7 @@ void DomainMapper::sprmWithProps( Sprm& rSprm, const PropertyMapPtr& rContext ) const PropertyMapPtr pParagraphProps = m_pImpl->GetTopContextOfType(CONTEXT_PARAGRAPH); if( pParagraphProps && pParagraphProps->isSet(PROP_PARA_STYLE_NAME) ) { - StyleSheetEntryPtr pStyle = nullptr; + StyleSheetEntryPtr pStyle; OUString sStyleName; pParagraphProps->getProperty(PROP_PARA_STYLE_NAME)->second >>= sStyleName; if( !sStyleName.isEmpty() && GetStyleSheetTable() ) diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx index 9caf6e3c8fd8..c0eea15bdb95 100644 --- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx +++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx @@ -2919,7 +2919,7 @@ void DomainMapper_Impl::ChainTextFrames() sal_Int32 nSeq; OUString s_mso_next_textbox; bool bShapeNameSet; - TextFramesForChaining(): xShape(nullptr), nId(0), nSeq(0), bShapeNameSet(false) {} + TextFramesForChaining(): nId(0), nSeq(0), bShapeNameSet(false) {} } ; typedef std::map <OUString, TextFramesForChaining> ChainMap; diff --git a/writerfilter/source/dmapper/PropertyMap.cxx b/writerfilter/source/dmapper/PropertyMap.cxx index e4fb96f7bfff..83d384844602 100644 --- a/writerfilter/source/dmapper/PropertyMap.cxx +++ b/writerfilter/source/dmapper/PropertyMap.cxx @@ -380,7 +380,6 @@ SectionPropertyMap::SectionPropertyMap( bool bIsFirstSection ) , m_bTitlePage( false ) , m_nColumnCount( 0 ) , m_nColumnDistance( 1249 ) - , m_xColumnContainer( nullptr ) , m_bSeparatorLineIsOn( false ) , m_bEvenlySpaced( false ) , m_nPageNumber( -1 ) diff --git a/writerfilter/source/dmapper/TextEffectsHandler.cxx b/writerfilter/source/dmapper/TextEffectsHandler.cxx index 13e621438c62..7626fb133762 100644 --- a/writerfilter/source/dmapper/TextEffectsHandler.cxx +++ b/writerfilter/source/dmapper/TextEffectsHandler.cxx @@ -480,8 +480,7 @@ void TextEffectsHandler::convertElementIdToPropertyId(sal_Int32 aElementId) } TextEffectsHandler::TextEffectsHandler(sal_uInt32 aElementId) : - LoggedProperties("TextEffectsHandler"), - mpGrabBagStack(nullptr) + LoggedProperties("TextEffectsHandler") { convertElementIdToPropertyId(aElementId); mpGrabBagStack.reset(new oox::GrabBagStack(maElementName)); diff --git a/writerfilter/source/ooxml/OOXMLDocumentImpl.cxx b/writerfilter/source/ooxml/OOXMLDocumentImpl.cxx index 77b958599966..eb4dcbcd41a7 100644 --- a/writerfilter/source/ooxml/OOXMLDocumentImpl.cxx +++ b/writerfilter/source/ooxml/OOXMLDocumentImpl.cxx @@ -58,7 +58,6 @@ OOXMLDocumentImpl::OOXMLDocumentImpl(OOXMLStream::Pointer_t const & pStream, con : mpStream(pStream) , mxStatusIndicator(xStatusIndicator) , mnXNoteId(0) - , mxThemeDom(nullptr) , mbIsSubstream(false) , mbSkipImages(bSkipImages) , mnPercentSize(0) diff --git a/xmlhelp/source/cxxhelp/provider/db.hxx b/xmlhelp/source/cxxhelp/provider/db.hxx index b26797c80593..607b987ff838 100644 --- a/xmlhelp/source/cxxhelp/provider/db.hxx +++ b/xmlhelp/source/cxxhelp/provider/db.hxx @@ -72,8 +72,6 @@ namespace helpdatafileproxy { Hdf( const OUString& rFileURL, css::uno::Reference< css::ucb::XSimpleFileAccess3 > const & xSFA ) : m_aFileURL( rFileURL ) - , m_pStringToDataMap( nullptr ) - , m_pStringToValPosMap( nullptr ) , m_xSFA( xSFA ) , m_pItData( nullptr ) , m_nItRead( -1 ) diff --git a/xmlhelp/source/cxxhelp/provider/provider.cxx b/xmlhelp/source/cxxhelp/provider/provider.cxx index 4c4532569848..24a953e7aabb 100644 --- a/xmlhelp/source/cxxhelp/provider/provider.cxx +++ b/xmlhelp/source/cxxhelp/provider/provider.cxx @@ -46,7 +46,6 @@ ContentProvider::ContentProvider( const uno::Reference< uno::XComponentContext > : ::ucbhelper::ContentProviderImplHelper( rxContext ) , isInitialized( false ) , m_aScheme(MYUCP_URL_SCHEME) - , m_pDatabases( nullptr ) { } diff --git a/xmlhelp/source/cxxhelp/provider/resultsetbase.cxx b/xmlhelp/source/cxxhelp/provider/resultsetbase.cxx index 9a36812d1172..f32c54a56c02 100644 --- a/xmlhelp/source/cxxhelp/provider/resultsetbase.cxx +++ b/xmlhelp/source/cxxhelp/provider/resultsetbase.cxx @@ -38,10 +38,7 @@ ResultSetBase::ResultSetBase( const uno::Reference< uno::XComponentContext >& r m_xProvider( xProvider ), m_nRow( -1 ), m_nWasNull( true ), - m_sProperty( seq ), - m_pDisposeEventListeners( nullptr ), - m_pRowCountListeners( nullptr ), - m_pIsFinalListeners( nullptr ) + m_sProperty( seq ) { } @@ -287,8 +284,7 @@ ResultSetBase::rowDeleted() uno::Reference< uno::XInterface > SAL_CALL ResultSetBase::getStatement() { - uno::Reference< uno::XInterface > test( nullptr ); - return test; + return uno::Reference< uno::XInterface >(); } diff --git a/xmloff/qa/unit/uxmloff.cxx b/xmloff/qa/unit/uxmloff.cxx index c2460dc496d5..707210b92767 100644 --- a/xmloff/qa/unit/uxmloff.cxx +++ b/xmloff/qa/unit/uxmloff.cxx @@ -47,7 +47,6 @@ private: }; Test::Test() - : pExport( nullptr ) { } diff --git a/xmloff/source/chart/SchXMLSeries2Context.cxx b/xmloff/source/chart/SchXMLSeries2Context.cxx index fba8bc84d600..4b934377d085 100644 --- a/xmloff/source/chart/SchXMLSeries2Context.cxx +++ b/xmloff/source/chart/SchXMLSeries2Context.cxx @@ -261,7 +261,6 @@ SchXMLSeries2Context::SchXMLSeries2Context( mrAxes( rAxes ), mrStyleVector( rStyleVector ), mrRegressionStyleVector( rRegressionStyleVector ), - m_xSeries(nullptr), mnSeriesIndex( nSeriesIndex ), mnDataPointIndex( 0 ), m_bStockHasVolume( bStockHasVolume ), diff --git a/xmloff/source/chart/transporttypes.hxx b/xmloff/source/chart/transporttypes.hxx index b1657a82dd37..16985db9022e 100644 --- a/xmloff/source/chart/transporttypes.hxx +++ b/xmloff/source/chart/transporttypes.hxx @@ -179,7 +179,6 @@ struct DataRowPointStyle , sal_Int32 nAttachedAxis = 0 ) : meType( eType ), m_xSeries( xSeries ), - m_xOldAPISeries( nullptr ), m_nPointIndex( nPointIndex ), m_nPointRepeat( nPointRepeat ), msStyleName( sStyleName ), diff --git a/xmloff/source/core/RDFaExportHelper.cxx b/xmloff/source/core/RDFaExportHelper.cxx index 2fcfe6109dcc..60132380d04c 100644 --- a/xmloff/source/core/RDFaExportHelper.cxx +++ b/xmloff/source/core/RDFaExportHelper.cxx @@ -87,7 +87,7 @@ getRelativeReference(SvXMLExport const& rExport, OUString const& rURI) } RDFaExportHelper::RDFaExportHelper(SvXMLExport & i_rExport) - : m_rExport(i_rExport), m_xRepository(nullptr), m_Counter(0) + : m_rExport(i_rExport), m_Counter(0) { const uno::Reference<rdf::XRepositorySupplier> xRS( m_rExport.GetModel(), uno::UNO_QUERY_THROW); diff --git a/xmloff/source/core/xmlexp.cxx b/xmloff/source/core/xmlexp.cxx index 6c3abe7d60ba..86dc9404bed7 100644 --- a/xmloff/source/core/xmlexp.cxx +++ b/xmloff/source/core/xmlexp.cxx @@ -436,11 +436,6 @@ SvXMLExport::SvXMLExport( mxAttrList( new SvXMLAttributeList ), mpNamespaceMap( new SvXMLNamespaceMap ), maUnitConv( xContext, util::MeasureUnit::MM_100TH, eDefaultMeasureUnit ), - mpNumExport(nullptr), - mpProgressBarHelper( nullptr ), - mpEventExport( nullptr ), - mpImageMapExport( nullptr ), - mpXMLErrors( nullptr ), meClass( eClass ), mnExportFlags( nExportFlags ), mnErrorFlags( SvXMLErrorFlags::NO ), @@ -466,11 +461,6 @@ SvXMLExport::SvXMLExport( msOrigFileName( rFileName ), mpNamespaceMap( new SvXMLNamespaceMap ), maUnitConv( xContext, util::MeasureUnit::MM_100TH, eDefaultMeasureUnit ), - mpNumExport(nullptr), - mpProgressBarHelper( nullptr ), - mpEventExport( nullptr ), - mpImageMapExport( nullptr ), - mpXMLErrors( nullptr ), meClass( XML_TOKEN_INVALID ), mnExportFlags( SvXMLExportFlags::NONE ), mnErrorFlags( SvXMLErrorFlags::NO ), @@ -506,11 +496,6 @@ SvXMLExport::SvXMLExport( maUnitConv( xContext, util::MeasureUnit::MM_100TH, SvXMLUnitConverter::GetMeasureUnit(eDefaultFieldUnit) ), - mpNumExport(nullptr), - mpProgressBarHelper( nullptr ), - mpEventExport( nullptr ), - mpImageMapExport( nullptr ), - mpXMLErrors( nullptr ), meClass( XML_TOKEN_INVALID ), mnExportFlags( nExportFlag ), mnErrorFlags( SvXMLErrorFlags::NO ), diff --git a/xmloff/source/core/xmlimp.cxx b/xmloff/source/core/xmlimp.cxx index 1c4099badb70..5f31846b9aa2 100644 --- a/xmloff/source/core/xmlimp.cxx +++ b/xmloff/source/core/xmlimp.cxx @@ -397,7 +397,6 @@ SvXMLImport::SvXMLImport( mnErrorFlags(SvXMLErrorFlags::NO), isFastContext( false ), maNamespaceHandler( new SvXMLImportFastNamespaceHandler() ), - mxFastDocumentHandler( nullptr ), mbIsFormsSupported( true ), mbIsTableShapeSupported( false ) { diff --git a/xmloff/source/draw/shapeimport.cxx b/xmloff/source/draw/shapeimport.cxx index dff86d74b63a..a474cef96aa3 100644 --- a/xmloff/source/draw/shapeimport.cxx +++ b/xmloff/source/draw/shapeimport.cxx @@ -110,8 +110,6 @@ XMLShapeImportHelper::XMLShapeImportHelper( const uno::Reference< frame::XModel>& rModel, SvXMLImportPropertyMapper *pExtMapper ) : mpImpl( new XMLShapeImportHelperImpl ), - mpPropertySetMapper(nullptr), - mpPresPagePropsMapper(nullptr), mrImporter( rImporter ) { mpImpl->mpSortContext = nullptr; diff --git a/xmloff/source/forms/handler/form_handler_factory.cxx b/xmloff/source/forms/handler/form_handler_factory.cxx index 438d76c52fea..8ea62c5fc4c7 100644 --- a/xmloff/source/forms/handler/form_handler_factory.cxx +++ b/xmloff/source/forms/handler/form_handler_factory.cxx @@ -28,14 +28,14 @@ namespace xmloff namespace { - static PPropertyHandler s_pVCLDateHandler = nullptr; - static PPropertyHandler s_pVCLTimeHandler = nullptr; + static PPropertyHandler s_pVCLDateHandler; + static PPropertyHandler s_pVCLTimeHandler; } //= FormHandlerFactory PPropertyHandler FormHandlerFactory::getFormPropertyHandler( const PropertyId i_propertyId ) { - PPropertyHandler pHandler( nullptr ); + PPropertyHandler pHandler; switch ( i_propertyId ) { diff --git a/xmloff/source/forms/officeforms.cxx b/xmloff/source/forms/officeforms.cxx index 57a1f23d0546..c3fd44b49b0a 100644 --- a/xmloff/source/forms/officeforms.cxx +++ b/xmloff/source/forms/officeforms.cxx @@ -117,7 +117,6 @@ namespace xmloff //= OFormsRootExport OFormsRootExport::OFormsRootExport( SvXMLExport& _rExp ) - :m_pImplElement(nullptr) { addModelAttributes(_rExp); diff --git a/xmloff/source/style/MultiPropertySetHelper.cxx b/xmloff/source/style/MultiPropertySetHelper.cxx index 93d06b64dcfc..c7288be476ea 100644 --- a/xmloff/source/style/MultiPropertySetHelper.cxx +++ b/xmloff/source/style/MultiPropertySetHelper.cxx @@ -35,10 +35,8 @@ using ::com::sun::star::uno::UNO_QUERY; MultiPropertySetHelper::MultiPropertySetHelper( const sal_Char** pNames ) : - pPropertyNames( nullptr ), nLength( 0 ), aPropertySequence(), - pSequenceIndex( nullptr ), aValues(), pValues( nullptr ) { diff --git a/xmloff/source/style/styleexp.cxx b/xmloff/source/style/styleexp.cxx index dac2d19e1565..963d127c12ff 100644 --- a/xmloff/source/style/styleexp.cxx +++ b/xmloff/source/style/styleexp.cxx @@ -440,7 +440,7 @@ void XMLStyleExport::exportStyleFamily( // If next styles are supported and used styles should be exported only, // the next style may be unused but has to be exported, too. In this case // the names of all exported styles are remembered. - std::unique_ptr<std::set<OUString> > pExportedStyles(nullptr); + std::unique_ptr<std::set<OUString> > pExportedStyles; bool bFirstStyle = true; const uno::Sequence< OUString> aSeq = xStyleCont->getElementNames(); diff --git a/xmloff/source/style/xmlexppr.cxx b/xmloff/source/style/xmlexppr.cxx index 14f4ff68e1f3..c264376b863e 100644 --- a/xmloff/source/style/xmlexppr.cxx +++ b/xmloff/source/style/xmlexppr.cxx @@ -198,8 +198,7 @@ public: FilterPropertiesInfo_Impl::FilterPropertiesInfo_Impl() : nCount(0), - aPropInfos(), - pApiNames( nullptr ) + aPropInfos() { } diff --git a/xmloff/source/style/xmlnumfe.cxx b/xmloff/source/style/xmlnumfe.cxx index 9598ee88b6c0..69ca3fdc6532 100644 --- a/xmloff/source/style/xmlnumfe.cxx +++ b/xmloff/source/style/xmlnumfe.cxx @@ -219,9 +219,7 @@ SvXMLNumFmtExport::SvXMLNumFmtExport( const uno::Reference< util::XNumberFormatsSupplier >& rSupp ) : rExport( rExp ), sPrefix( OUString("N") ), - pFormatter( nullptr ), - pCharClass( nullptr ), - pLocaleData( nullptr ) + pFormatter( nullptr ) { // supplier must be SvNumberFormatsSupplierObj SvNumberFormatsSupplierObj* pObj = @@ -253,9 +251,7 @@ SvXMLNumFmtExport::SvXMLNumFmtExport( const OUString& rPrefix ) : rExport( rExp ), sPrefix( rPrefix ), - pFormatter( nullptr ), - pCharClass( nullptr ), - pLocaleData( nullptr ) + pFormatter( nullptr ) { // supplier must be SvNumberFormatsSupplierObj SvNumberFormatsSupplierObj* pObj = diff --git a/xmloff/source/style/xmlstyle.cxx b/xmloff/source/style/xmlstyle.cxx index 371f0def7735..642f1830c429 100644 --- a/xmloff/source/style/xmlstyle.cxx +++ b/xmloff/source/style/xmlstyle.cxx @@ -747,8 +747,7 @@ SvXMLStylesContext::SvXMLStylesContext( SvXMLImport& rImport, sal_uInt16 nPrfx, const OUString& rLName, const uno::Reference< xml::sax::XAttributeList > &, bool bAuto ) : SvXMLImportContext( rImport, nPrfx, rLName ), - mpImpl( new SvXMLStylesContext_Impl( bAuto ) ), - mpStyleStylesElemTokenMap( nullptr ) + mpImpl( new SvXMLStylesContext_Impl( bAuto ) ) { } diff --git a/xmloff/source/text/XMLFootnoteConfigurationImportContext.cxx b/xmloff/source/text/XMLFootnoteConfigurationImportContext.cxx index 633a2151c400..169bf130955e 100644 --- a/xmloff/source/text/XMLFootnoteConfigurationImportContext.cxx +++ b/xmloff/source/text/XMLFootnoteConfigurationImportContext.cxx @@ -130,7 +130,6 @@ XMLFootnoteConfigurationImportContext::XMLFootnoteConfigurationImportContext( : SvXMLStyleContext(rImport, nPrfx, rLocalName, xAttrList, XML_STYLE_FAMILY_TEXT_FOOTNOTECONFIG) , sNumFormat("1") , sNumSync("false") -, pAttrTokenMap(nullptr) , nOffset(0) , nNumbering(FootnoteNumbering::PER_PAGE) , bPosition(false) diff --git a/xmloff/source/text/txtparae.cxx b/xmloff/source/text/txtparae.cxx index 569be2c3c707..95a3e89c6f2d 100644 --- a/xmloff/source/text/txtparae.cxx +++ b/xmloff/source/text/txtparae.cxx @@ -1242,12 +1242,7 @@ XMLTextParagraphExport::XMLTextParagraphExport( m_xImpl(new Impl), rAutoStylePool( rASP ), pBoundFrameSets(new BoundFrameSets(GetExport().GetModel())), - pFieldExport( nullptr ), - pListElements( nullptr ), maListAutoPool( GetExport() ), - pSectionExport( nullptr ), - pIndexMarkExport( nullptr ), - pRedlineExport( nullptr ), bProgress( false ), bBlock( false ), bOpenRuby( false ), diff --git a/xmloff/source/xforms/XFormsBindContext.cxx b/xmloff/source/xforms/XFormsBindContext.cxx index 7adf86bb6b89..3be50d4fadf2 100644 --- a/xmloff/source/xforms/XFormsBindContext.cxx +++ b/xmloff/source/xforms/XFormsBindContext.cxx @@ -67,8 +67,7 @@ XFormsBindContext::XFormsBindContext( const OUString& rLocalName, const Reference<XModel2>& xModel ) : TokenContext( rImport, nPrefix, rLocalName, aAttributeMap, aEmptyMap ), - mxModel( xModel ), - mxBinding( nullptr ) + mxModel( xModel ) { // attach binding to model mxBinding = mxModel->createBinding(); diff --git a/xmlscript/source/xml_helper/xml_impctx.cxx b/xmlscript/source/xml_helper/xml_impctx.cxx index 9c1df5f7cd92..face4f5ca01f 100644 --- a/xmlscript/source/xml_helper/xml_impctx.cxx +++ b/xmlscript/source/xml_helper/xml_impctx.cxx @@ -179,8 +179,7 @@ DocumentHandlerImpl::DocumentHandlerImpl( m_aLastURI_lookup( "<<< unknown URI >>>" ), m_nLastPrefix_lookup( UID_UNKNOWN ), m_aLastPrefix_lookup( "<<< unknown URI >>>" ), - m_nSkipElements( 0 ), - m_pMutex( nullptr ) + m_nSkipElements( 0 ) { m_elements.reserve( 10 ); diff --git a/xmlsecurity/source/framework/saxeventkeeperimpl.cxx b/xmlsecurity/source/framework/saxeventkeeperimpl.cxx index 2c60c950f605..cc79b91613ff 100644 --- a/xmlsecurity/source/framework/saxeventkeeperimpl.cxx +++ b/xmlsecurity/source/framework/saxeventkeeperimpl.cxx @@ -39,8 +39,7 @@ namespace cssxs = com::sun::star::xml::sax; #define IMPLEMENTATION_NAME "com.sun.star.xml.security.framework.SAXEventKeeperImpl" SAXEventKeeperImpl::SAXEventKeeperImpl( ) - :m_pRootBufferNode(nullptr), - m_pCurrentBufferNode(nullptr), + :m_pCurrentBufferNode(nullptr), m_nNextElementMarkId(1), m_pNewBlocker(nullptr), m_pCurrentBlockingBufferNode(nullptr), diff --git a/xmlsecurity/source/framework/xmlsignaturetemplateimpl.cxx b/xmlsecurity/source/framework/xmlsignaturetemplateimpl.cxx index 6e3b0939cebf..1ee62df563bf 100644 --- a/xmlsecurity/source/framework/xmlsignaturetemplateimpl.cxx +++ b/xmlsecurity/source/framework/xmlsignaturetemplateimpl.cxx @@ -30,8 +30,7 @@ using ::com::sun::star::xml::wrapper::XXMLElementWrapper ; using ::com::sun::star::xml::crypto::XXMLSignatureTemplate ; XMLSignatureTemplateImpl::XMLSignatureTemplateImpl() - :m_xTemplate( nullptr ), - m_nStatus ( css::xml::crypto::SecurityOperationStatus_UNKNOWN ) + :m_nStatus ( css::xml::crypto::SecurityOperationStatus_UNKNOWN ) { } diff --git a/xmlsecurity/source/helper/xsecverify.cxx b/xmlsecurity/source/helper/xsecverify.cxx index 283c9ac45c6d..edd78902313b 100644 --- a/xmlsecurity/source/helper/xsecverify.cxx +++ b/xmlsecurity/source/helper/xsecverify.cxx @@ -101,7 +101,7 @@ cssu::Reference< cssxc::sax::XReferenceResolvedListener > XSecController::prepar void XSecController::addSignature() { - cssu::Reference< cssxc::sax::XReferenceResolvedListener > xReferenceResolvedListener = nullptr; + cssu::Reference< cssxc::sax::XReferenceResolvedListener > xReferenceResolvedListener; sal_Int32 nSignatureId = 0; @@ -466,7 +466,7 @@ void XSecController::collectToVerify( const OUString& referenceId ) */ { bool bJustChainingOn = false; - cssu::Reference< cssxs::XDocumentHandler > xHandler = nullptr; + cssu::Reference< cssxs::XDocumentHandler > xHandler; int i,j; int sigNum = m_vInternalSignatureInformations.size(); commit 8c29b0837d924d226fd1a48e323771b450f2b9b6 Author: Noel Grandin <noel.gran...@collabora.co.uk> AuthorDate: Wed Sep 12 11:00:48 2018 +0200 Commit: Noel Grandin <noel.gran...@collabora.co.uk> CommitDate: Thu Sep 13 08:30:11 2018 +0200 new loplugin:constfields look for fields which are only assigned to in the constructor, so they can be made const Change-Id: I0b76817c2181227b04f6a29d6a808f5e31999765 Reviewed-on: https://gerrit.libreoffice.org/60393 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> diff --git a/compilerplugins/clang/constfields.cxx b/compilerplugins/clang/constfields.cxx new file mode 100644 index 000000000000..6613c836b892 --- /dev/null +++ b/compilerplugins/clang/constfields.cxx @@ -0,0 +1,593 @@ +/* -*- 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/. + */ + +#if !defined _WIN32 //TODO, #include <sys/file.h> + +#include <cassert> +#include <string> +#include <iostream> +#include <fstream> +#include <unordered_set> +#include <vector> +#include <algorithm> +#include <sys/file.h> +#include <unistd.h> +#include "plugin.hxx" +#include "compat.hxx" +#include "check.hxx" + +/** +Look for fields that are only assigned to in the constructor using field-init, and can therefore be const. + +The process goes something like this: + $ make check + $ make FORCE_COMPILE_ALL=1 COMPILER_PLUGIN_TOOL='constfields' check + $ ./compilerplugins/clang/constfields.py + +and then + $ for dir in *; do make $dir FORCE_COMPILE_ALL=1 UPDATE_FILES=$dir COMPILER_PLUGIN_TOOL='constfieldsrewrite' $dir; done +to auto-remove the method declarations + +*/ + +namespace +{ +struct MyFieldInfo +{ + const RecordDecl* parentRecord; + std::string parentClass; + std::string fieldName; + std::string fieldType; + std::string sourceLocation; + std::string access; +}; +bool operator<(const MyFieldInfo& lhs, const MyFieldInfo& rhs) +{ + return std::tie(lhs.parentClass, lhs.fieldName) < std::tie(rhs.parentClass, rhs.fieldName); +} + +// try to limit the voluminous output a little +static std::set<MyFieldInfo> cannotBeConstSet; +static std::set<MyFieldInfo> definitionSet; + +/** + * Wrap the different kinds of callable and callee objects in the clang AST so I can define methods that handle everything. + */ +class CallerWrapper +{ + const CallExpr* m_callExpr; + const CXXConstructExpr* m_cxxConstructExpr; + +public: + CallerWrapper(const CallExpr* callExpr) + : m_callExpr(callExpr) + , m_cxxConstructExpr(nullptr) + { + } + CallerWrapper(const CXXConstructExpr* cxxConstructExpr) + : m_callExpr(nullptr) + , m_cxxConstructExpr(cxxConstructExpr) + { + } + unsigned getNumArgs() const + { + return m_callExpr ? m_callExpr->getNumArgs() : m_cxxConstructExpr->getNumArgs(); + } + const Expr* getArg(unsigned i) const + { + return m_callExpr ? m_callExpr->getArg(i) : m_cxxConstructExpr->getArg(i); + } +}; +class CalleeWrapper +{ + const FunctionDecl* m_calleeFunctionDecl = nullptr; + const CXXConstructorDecl* m_cxxConstructorDecl = nullptr; + const FunctionProtoType* m_functionPrototype = nullptr; + +public: + explicit CalleeWrapper(const FunctionDecl* calleeFunctionDecl) + : m_calleeFunctionDecl(calleeFunctionDecl) + { + } + explicit CalleeWrapper(const CXXConstructExpr* cxxConstructExpr) + : m_cxxConstructorDecl(cxxConstructExpr->getConstructor()) + { + } + explicit CalleeWrapper(const FunctionProtoType* functionPrototype) + : m_functionPrototype(functionPrototype) + { + } + unsigned getNumParams() const + { + if (m_calleeFunctionDecl) + return m_calleeFunctionDecl->getNumParams(); + else if (m_cxxConstructorDecl) + return m_cxxConstructorDecl->getNumParams(); + else if (m_functionPrototype->param_type_begin() == m_functionPrototype->param_type_end()) + // FunctionProtoType will assert if we call getParamTypes() and it has no params + return 0; + else + return m_functionPrototype->getParamTypes().size(); + } + const QualType getParamType(unsigned i) const + { + if (m_calleeFunctionDecl) + return m_calleeFunctionDecl->getParamDecl(i)->getType(); + else if (m_cxxConstructorDecl) + return m_cxxConstructorDecl->getParamDecl(i)->getType(); + else + return m_functionPrototype->getParamTypes()[i]; + } + std::string getNameAsString() const + { + if (m_calleeFunctionDecl) + return m_calleeFunctionDecl->getNameAsString(); + else if (m_cxxConstructorDecl) + return m_cxxConstructorDecl->getNameAsString(); + else + return ""; + } + CXXMethodDecl const* getAsCXXMethodDecl() const + { + if (m_calleeFunctionDecl) + return dyn_cast<CXXMethodDecl>(m_calleeFunctionDecl); + return nullptr; + } +}; + +class ConstFields : public RecursiveASTVisitor<ConstFields>, public loplugin::Plugin +{ +public: + explicit ConstFields(loplugin::InstantiationData const& data) + : Plugin(data) + { + } + + virtual void run() override; + + bool shouldVisitTemplateInstantiations() const { return true; } + bool shouldVisitImplicitCode() const { return true; } + + bool VisitFieldDecl(const FieldDecl*); + bool VisitMemberExpr(const MemberExpr*); + bool TraverseCXXConstructorDecl(CXXConstructorDecl*); + bool TraverseCXXMethodDecl(CXXMethodDecl*); + bool TraverseFunctionDecl(FunctionDecl*); + bool TraverseIfStmt(IfStmt*); + +private: + MyFieldInfo niceName(const FieldDecl*); + void check(const FieldDecl* fieldDecl, const Expr* memberExpr); + bool isSomeKindOfZero(const Expr* arg); + bool IsPassedByNonConst(const FieldDecl* fieldDecl, const Stmt* child, CallerWrapper callExpr, + CalleeWrapper calleeFunctionDecl); + llvm::Optional<CalleeWrapper> getCallee(CallExpr const*); + + RecordDecl* insideMoveOrCopyDeclParent = nullptr; + // For reasons I do not understand, parentFunctionDecl() is not reliable, so + // we store the parent function on the way down the AST. + FunctionDecl* insideFunctionDecl = nullptr; + std::vector<FieldDecl const*> insideConditionalCheckOfMemberSet; +}; + +void ConstFields::run() +{ + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + + if (!isUnitTestMode()) + { + // dump all our output in one write call - this is to try and limit IO "crosstalk" between multiple processes + // writing to the same logfile + std::string output; + for (const MyFieldInfo& s : cannotBeConstSet) + output += "write-outside-constructor:\t" + s.parentClass + "\t" + s.fieldName + "\n"; + for (const MyFieldInfo& s : definitionSet) + output += "definition:\t" + s.access + "\t" + s.parentClass + "\t" + s.fieldName + "\t" + + s.fieldType + "\t" + s.sourceLocation + "\n"; + std::ofstream myfile; + myfile.open(WORKDIR "/loplugin.constfields.log", std::ios::app | std::ios::out); + myfile << output; + myfile.close(); + } + else + { + for (const MyFieldInfo& s : cannotBeConstSet) + report(DiagnosticsEngine::Warning, "notconst %0", compat::getBeginLoc(s.parentRecord)) + << s.fieldName; + } +} + +MyFieldInfo ConstFields::niceName(const FieldDecl* fieldDecl) +{ + MyFieldInfo aInfo; + + const RecordDecl* recordDecl = fieldDecl->getParent(); + + if (const CXXRecordDecl* cxxRecordDecl = dyn_cast<CXXRecordDecl>(recordDecl)) + { + if (cxxRecordDecl->getTemplateInstantiationPattern()) + cxxRecordDecl = cxxRecordDecl->getTemplateInstantiationPattern(); + aInfo.parentRecord = cxxRecordDecl; + aInfo.parentClass = cxxRecordDecl->getQualifiedNameAsString(); + } + else + { + aInfo.parentRecord = recordDecl; + aInfo.parentClass = recordDecl->getQualifiedNameAsString(); + } + + aInfo.fieldName = fieldDecl->getNameAsString(); + // sometimes the name (if it's an anonymous thing) contains the full path of the build folder, which we don't need + size_t idx = aInfo.fieldName.find(SRCDIR); + if (idx != std::string::npos) + { + aInfo.fieldName = aInfo.fieldName.replace(idx, strlen(SRCDIR), ""); + } + aInfo.fieldType = fieldDecl->getType().getAsString(); + + SourceLocation expansionLoc + = compiler.getSourceManager().getExpansionLoc(fieldDecl->getLocation()); + StringRef name = compiler.getSourceManager().getFilename(expansionLoc); + aInfo.sourceLocation + = std::string(name.substr(strlen(SRCDIR) + 1)) + ":" + + std::to_string(compiler.getSourceManager().getSpellingLineNumber(expansionLoc)); + loplugin::normalizeDotDotInFilePath(aInfo.sourceLocation); + + switch (fieldDecl->getAccess()) + { + case AS_public: + aInfo.access = "public"; + break; + case AS_private: + aInfo.access = "private"; + break; + case AS_protected: + aInfo.access = "protected"; + break; + default: + aInfo.access = "unknown"; + break; + } + + return aInfo; +} + +bool ConstFields::VisitFieldDecl(const FieldDecl* fieldDecl) +{ + fieldDecl = fieldDecl->getCanonicalDecl(); + if (ignoreLocation(fieldDecl)) + { + return true; + } + // ignore stuff that forms part of the stable URE interface + if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) + { + return true; + } + definitionSet.insert(niceName(fieldDecl)); + return true; +} + +bool ConstFields::TraverseCXXConstructorDecl(CXXConstructorDecl* cxxConstructorDecl) +{ + auto copy = insideMoveOrCopyDeclParent; + if (!ignoreLocation(cxxConstructorDecl) && cxxConstructorDecl->isThisDeclarationADefinition()) + { + if (cxxConstructorDecl->isCopyOrMoveConstructor()) + insideMoveOrCopyDeclParent = cxxConstructorDecl->getParent(); + } + bool ret = RecursiveASTVisitor::TraverseCXXConstructorDecl(cxxConstructorDecl); + insideMoveOrCopyDeclParent = copy; + return ret; +} + +bool ConstFields::TraverseCXXMethodDecl(CXXMethodDecl* cxxMethodDecl) +{ + auto copy1 = insideMoveOrCopyDeclParent; + auto copy2 = insideFunctionDecl; + if (!ignoreLocation(cxxMethodDecl) && cxxMethodDecl->isThisDeclarationADefinition()) + { + if (cxxMethodDecl->isCopyAssignmentOperator() || cxxMethodDecl->isMoveAssignmentOperator()) + insideMoveOrCopyDeclParent = cxxMethodDecl->getParent(); + } + insideFunctionDecl = cxxMethodDecl; + bool ret = RecursiveASTVisitor::TraverseCXXMethodDecl(cxxMethodDecl); + insideMoveOrCopyDeclParent = copy1; + insideFunctionDecl = copy2; + return ret; +} + +bool ConstFields::TraverseFunctionDecl(FunctionDecl* functionDecl) +{ + auto copy2 = insideFunctionDecl; + insideFunctionDecl = functionDecl; + bool ret = RecursiveASTVisitor::TraverseFunctionDecl(functionDecl); + insideFunctionDecl = copy2; + return ret; +} + +bool ConstFields::TraverseIfStmt(IfStmt* ifStmt) +{ + FieldDecl const* memberFieldDecl = nullptr; + Expr const* cond = ifStmt->getCond()->IgnoreParenImpCasts(); + if (auto memberExpr = dyn_cast<MemberExpr>(cond)) + { + if ((memberFieldDecl = dyn_cast<FieldDecl>(memberExpr->getMemberDecl()))) + insideConditionalCheckOfMemberSet.push_back(memberFieldDecl); + } + bool ret = RecursiveASTVisitor::TraverseIfStmt(ifStmt); + if (memberFieldDecl) + insideConditionalCheckOfMemberSet.pop_back(); + return ret; +} + +bool ConstFields::VisitMemberExpr(const MemberExpr* memberExpr) +{ + const ValueDecl* decl = memberExpr->getMemberDecl(); + const FieldDecl* fieldDecl = dyn_cast<FieldDecl>(decl); + if (!fieldDecl) + { + return true; + } + fieldDecl = fieldDecl->getCanonicalDecl(); + if (ignoreLocation(fieldDecl)) + { + return true; + } + // ignore stuff that forms part of the stable URE interface + if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) + { + return true; + } + + check(fieldDecl, memberExpr); + + return true; +} + +void ConstFields::check(const FieldDecl* fieldDecl, const Expr* memberExpr) +{ + auto parentsRange = compiler.getASTContext().getParents(*memberExpr); + const Stmt* child = memberExpr; + const Stmt* parent + = parentsRange.begin() == parentsRange.end() ? nullptr : parentsRange.begin()->get<Stmt>(); + // walk up the tree until we find something interesting + bool bCannotBeConst = false; + bool bDump = false; + auto walkUp = [&]() { + child = parent; + auto parentsRange = compiler.getASTContext().getParents(*parent); + parent = parentsRange.begin() == parentsRange.end() ? nullptr + : parentsRange.begin()->get<Stmt>(); + }; + do + { + if (!parent) + { + // check if we have an expression like + // int& r = m_field; + auto parentsRange = compiler.getASTContext().getParents(*child); + if (parentsRange.begin() != parentsRange.end()) + { + auto varDecl = dyn_cast_or_null<VarDecl>(parentsRange.begin()->get<Decl>()); + // The isImplicit() call is to avoid triggering when we see the vardecl which is part of a for-range statement, + // which is of type 'T&&' and also an l-value-ref ? + if (varDecl && !varDecl->isImplicit() + && loplugin::TypeCheck(varDecl->getType()).LvalueReference().NonConst()) + { + bCannotBeConst = true; + } + } + break; + } + if (isa<CXXReinterpretCastExpr>(parent)) + { + // once we see one of these, there is not much useful we can know + bCannotBeConst = true; + break; + } + else if (isa<CastExpr>(parent) || isa<MemberExpr>(parent) || isa<ParenExpr>(parent) + || isa<ParenListExpr>(parent) +#if CLANG_VERSION >= 40000 + || isa<ArrayInitLoopExpr>(parent) +#endif + || isa<ExprWithCleanups>(parent)) + { + walkUp(); + } + else if (auto unaryOperator = dyn_cast<UnaryOperator>(parent)) + { + UnaryOperator::Opcode op = unaryOperator->getOpcode(); + if (op == UO_AddrOf || op == UO_PostInc || op == UO_PostDec || op == UO_PreInc + || op == UO_PreDec) + { + bCannotBeConst = true; + } + walkUp(); + } + else if (auto operatorCallExpr = dyn_cast<CXXOperatorCallExpr>(parent)) + { + auto callee = getCallee(operatorCallExpr); + if (callee) + { + // if calling a non-const operator on the field + auto calleeMethodDecl = callee->getAsCXXMethodDecl(); + if (calleeMethodDecl && operatorCallExpr->getArg(0) == child + && !calleeMethodDecl->isConst()) + { + bCannotBeConst = true; + } + else if (IsPassedByNonConst(fieldDecl, child, operatorCallExpr, *callee)) + { + bCannotBeConst = true; + } + } + else + bCannotBeConst = true; // conservative, could improve + walkUp(); + } + else if (auto cxxMemberCallExpr = dyn_cast<CXXMemberCallExpr>(parent)) + { + const CXXMethodDecl* calleeMethodDecl = cxxMemberCallExpr->getMethodDecl(); + if (calleeMethodDecl) + { + // if calling a non-const method on the field + const Expr* tmp = dyn_cast<Expr>(child); + if (tmp->isBoundMemberFunction(compiler.getASTContext())) + { + tmp = dyn_cast<MemberExpr>(tmp)->getBase(); + } + if (cxxMemberCallExpr->getImplicitObjectArgument() == tmp + && !calleeMethodDecl->isConst()) + { + bCannotBeConst = true; + break; + } + if (IsPassedByNonConst(fieldDecl, child, cxxMemberCallExpr, + CalleeWrapper(calleeMethodDecl))) + bCannotBeConst = true; + } + else + bCannotBeConst = true; // can happen in templates + walkUp(); + } + else if (auto cxxConstructExpr = dyn_cast<CXXConstructExpr>(parent)) + { + if (IsPassedByNonConst(fieldDecl, child, cxxConstructExpr, + CalleeWrapper(cxxConstructExpr))) + bCannotBeConst = true; + walkUp(); + } + else if (auto callExpr = dyn_cast<CallExpr>(parent)) + { + auto callee = getCallee(callExpr); + if (callee) + { + if (IsPassedByNonConst(fieldDecl, child, callExpr, *callee)) + bCannotBeConst = true; + } + else + bCannotBeConst = true; // conservative, could improve + walkUp(); + } + else if (auto binaryOp = dyn_cast<BinaryOperator>(parent)) + { + BinaryOperator::Opcode op = binaryOp->getOpcode(); + const bool assignmentOp = op == BO_Assign || op == BO_MulAssign || op == BO_DivAssign + || op == BO_RemAssign || op == BO_AddAssign + || op == BO_SubAssign || op == BO_ShlAssign + || op == BO_ShrAssign || op == BO_AndAssign + || op == BO_XorAssign || op == BO_OrAssign; + if (assignmentOp) + { + if (binaryOp->getLHS() == child) + bCannotBeConst = true; + else if (loplugin::TypeCheck(binaryOp->getLHS()->getType()) + .LvalueReference() + .NonConst()) + // if the LHS is a non-const reference, we could write to the field later on + bCannotBeConst = true; + } + walkUp(); + } + else if (isa<ReturnStmt>(parent)) + { + if (insideFunctionDecl) + { + auto tc = loplugin::TypeCheck(insideFunctionDecl->getReturnType()); + if (tc.LvalueReference().NonConst()) + bCannotBeConst = true; + } + break; + } + else if (isa<SwitchStmt>(parent) || isa<WhileStmt>(parent) || isa<ForStmt>(parent) + || isa<IfStmt>(parent) || isa<DoStmt>(parent) || isa<CXXForRangeStmt>(parent) + || isa<DefaultStmt>(parent)) + { + break; + } + else + { + walkUp(); + } + } while (true); + + if (bDump) + { + report(DiagnosticsEngine::Warning, "oh dear, what can the matter be? writtenTo=%0", + compat::getBeginLoc(memberExpr)) + << bCannotBeConst << memberExpr->getSourceRange(); + if (parent) + { + report(DiagnosticsEngine::Note, "parent over here", compat::getBeginLoc(parent)) + << parent->getSourceRange(); + parent->dump(); + } + memberExpr->dump(); + fieldDecl->getType()->dump(); + } + + if (bCannotBeConst) + { + cannotBeConstSet.insert(niceName(fieldDecl)); + } +} + +bool ConstFields::IsPassedByNonConst(const FieldDecl* fieldDecl, const Stmt* child, + CallerWrapper callExpr, CalleeWrapper calleeFunctionDecl) +{ + unsigned len = std::min(callExpr.getNumArgs(), calleeFunctionDecl.getNumParams()); + // if it's an array, passing it by value to a method typically means the + // callee takes a pointer and can modify the array + if (fieldDecl->getType()->isConstantArrayType()) + { + for (unsigned i = 0; i < len; ++i) + if (callExpr.getArg(i) == child) + if (loplugin::TypeCheck(calleeFunctionDecl.getParamType(i)).Pointer().NonConst()) + return true; + } + else + { + for (unsigned i = 0; i < len; ++i) + if (callExpr.getArg(i) == child) + if (loplugin::TypeCheck(calleeFunctionDecl.getParamType(i)) + .LvalueReference() + .NonConst()) + return true; + } + return false; +} + +llvm::Optional<CalleeWrapper> ConstFields::getCallee(CallExpr const* callExpr) +{ + FunctionDecl const* functionDecl = callExpr->getDirectCallee(); + if (functionDecl) + return CalleeWrapper(functionDecl); + + // Extract the functionprototype from a type + clang::Type const* calleeType = callExpr->getCallee()->getType().getTypePtr(); + if (auto pointerType = calleeType->getUnqualifiedDesugaredType()->getAs<clang::PointerType>()) + { + if (auto prototype = pointerType->getPointeeType() + ->getUnqualifiedDesugaredType() + ->getAs<FunctionProtoType>()) + { + return CalleeWrapper(prototype); + } + } + + return llvm::Optional<CalleeWrapper>(); +} + +loplugin::Plugin::Registration<ConstFields> X("constfields", false); +} + +#endif + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/compilerplugins/clang/constfields.py b/compilerplugins/clang/constfields.py new file mode 100755 index 000000000000..9ce5e2747fda --- /dev/null +++ b/compilerplugins/clang/constfields.py @@ -0,0 +1,95 @@ +#!/usr/bin/python + +import sys +import re +import io + +definitionSet = set() +definitionToSourceLocationMap = dict() +definitionToTypeMap = dict() +writeFromOutsideConstructorSet = set() + +# clang does not always use exactly the same numbers in the type-parameter vars it generates +# so I need to substitute them to ensure we can match correctly. +normalizeTypeParamsRegex = re.compile(r"type-parameter-\d+-\d+") +def normalizeTypeParams( line ): + return normalizeTypeParamsRegex.sub("type-parameter-?-?", line) + +def parseFieldInfo( tokens ): + if len(tokens) == 3: + return (normalizeTypeParams(tokens[1]), tokens[2]) + else: + return (normalizeTypeParams(tokens[1]), "") + +with io.open("workdir/loplugin.constfields.log", "rb", buffering=1024*1024) as txt: + for line in txt: + tokens = line.strip().split("\t") + if tokens[0] == "definition:": + access = tokens[1] + fieldInfo = (normalizeTypeParams(tokens[2]), tokens[3]) + srcLoc = tokens[5] + # ignore external source code + if (srcLoc.startswith("external/")): + continue + # ignore build folder + if (srcLoc.startswith("workdir/")): + continue + definitionSet.add(fieldInfo) + definitionToTypeMap[fieldInfo] = tokens[4] + definitionToSourceLocationMap[fieldInfo] = tokens[5] + elif tokens[0] == "write-outside-constructor:": + writeFromOutsideConstructorSet.add(parseFieldInfo(tokens)) + else: + print( "unknown line: " + line) + +# Invert the definitionToSourceLocationMap +# If we see more than one method at the same sourceLocation, it's being autogenerated as part of a template +# and we should just ignore it +sourceLocationToDefinitionMap = {} +for k, v in definitionToSourceLocationMap.iteritems(): + sourceLocationToDefinitionMap[v] = sourceLocationToDefinitionMap.get(v, []) + sourceLocationToDefinitionMap[v].append(k) +for k, definitions in sourceLocationToDefinitionMap.iteritems(): + if len(definitions) > 1: + for d in definitions: + definitionSet.remove(d) + +# Calculate can-be-const-field set +canBeConstFieldSet = set() +for d in definitionSet: + if d in writeFromOutsideConstructorSet: + continue + srcLoc = definitionToSourceLocationMap[d]; + fieldType = definitionToTypeMap[d] + if fieldType.startswith("const "): + continue + if "std::unique_ptr" in fieldType: + continue + if "std::shared_ptr" in fieldType: + continue + if "Reference<" in fieldType: + continue + if "VclPtr<" in fieldType: + continue + if "osl::Mutex" in fieldType: + continue + if "::sfx2::sidebar::ControllerItem" in fieldType: + continue + canBeConstFieldSet.add((d[0] + " " + d[1] + " " + fieldType, srcLoc)) + + +# sort the results using a "natural order" so sequences like [item1,item2,item10] sort nicely +def natural_sort_key(s, _nsre=re.compile('([0-9]+)')): + return [int(text) if text.isdigit() else text.lower() + for text in re.split(_nsre, s)] + +# sort results by name and line number +tmp6list = sorted(canBeConstFieldSet, key=lambda v: natural_sort_key(v[1])) + +# print out the results +with open("compilerplugins/clang/constfields.results", "wt") as f: + for t in tmp6list: + f.write( t[1] + "\n" ) + f.write( " " + t[0] + "\n" ) + + diff --git a/compilerplugins/clang/constfieldsrewrite.cxx b/compilerplugins/clang/constfieldsrewrite.cxx new file mode 100644 index 000000000000..cff211665ddf --- /dev/null +++ b/compilerplugins/clang/constfieldsrewrite.cxx @@ -0,0 +1,143 @@ +/* -*- 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/. + */ + +#if !defined _WIN32 //TODO, #include <sys/mman.h> + +#include <cassert> +#include <string> +#include <iostream> +#include "plugin.hxx" +#include "check.hxx" +#include <sys/mman.h> +#include <sys/types.h> +#include <fcntl.h> +#include <unistd.h> +#include <sys/stat.h> +#include <assert.h> +#include <cstring> + +/** + This is intended to be run as the second stage of the "constfields" clang plugin. +*/ + +namespace +{ +class ConstFieldsRewrite : public RecursiveASTVisitor<ConstFieldsRewrite>, + public loplugin::RewritePlugin +{ +public: + explicit ConstFieldsRewrite(loplugin::InstantiationData const& data); + ~ConstFieldsRewrite(); + + virtual void run() override + { + if (rewriter) + { + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } + } + + bool VisitFieldDecl(const FieldDecl* var); + +private: + // I use a brute-force approach - mmap the results file and do a linear search on it + // It works surprisingly well, because the file is small enough to fit into L2 cache on modern CPU's + size_t mmapFilesize; + int mmapFD; + char* mmappedData; +}; + +size_t getFilesize(const char* filename) +{ + struct stat st; + stat(filename, &st); + return st.st_size; +} + +ConstFieldsRewrite::ConstFieldsRewrite(loplugin::InstantiationData const& data) + : RewritePlugin(data) +{ + static const char sInputFile[] = SRCDIR "/compilerplugins/clang/constfields.results"; + mmapFilesize = getFilesize(sInputFile); + //Open file + mmapFD = open(sInputFile, O_RDONLY, 0); + assert(mmapFD != -1); + //Execute mmap + mmappedData = static_cast<char*>(mmap(NULL, mmapFilesize, PROT_READ, MAP_PRIVATE, mmapFD, 0)); + assert(mmappedData != NULL); +} + +ConstFieldsRewrite::~ConstFieldsRewrite() +{ + //Cleanup + int rc = munmap(mmappedData, mmapFilesize); + assert(rc == 0); + close(mmapFD); +} + +bool ConstFieldsRewrite::VisitFieldDecl(const FieldDecl* fieldDecl) +{ + if (ignoreLocation(fieldDecl)) + return true; + // ignore stuff that forms part of the stable URE interface + if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc( + fieldDecl->getCanonicalDecl()->getLocation()))) + return true; + // in case we've already processed this field + if (fieldDecl->getType().isConstQualified()) + return true; + // in case we've already processed this field + if (fieldDecl->getType().isConstQualified()) + return true; + // TODO rewriting T& is a bit trickier + if (loplugin::TypeCheck(fieldDecl->getType()).LvalueReference()) + return true; + + const RecordDecl* recordDecl = fieldDecl->getParent(); + std::string parentClassName; + if (const CXXRecordDecl* cxxRecordDecl = dyn_cast<CXXRecordDecl>(recordDecl)) + { + if (cxxRecordDecl->getTemplateInstantiationPattern()) + cxxRecordDecl = cxxRecordDecl->getTemplateInstantiationPattern(); + parentClassName = cxxRecordDecl->getQualifiedNameAsString(); + } + else + { + parentClassName = recordDecl->getQualifiedNameAsString(); + } + // the extra spaces match the formatting in the results file, and help avoid false+ + std::string aNiceName = " " + parentClassName + " " + fieldDecl->getNameAsString() + " " + + fieldDecl->getType().getAsString() + "\n"; + + // search mmap'ed file for field + const char* aNiceNameStr = aNiceName.c_str(); + char* found = std::search(mmappedData, mmappedData + mmapFilesize, aNiceNameStr, + aNiceNameStr + aNiceName.size()); + if (!(found < mmappedData + mmapFilesize)) + return true; + + auto endLoc = fieldDecl->getTypeSourceInfo()->getTypeLoc().getEndLoc(); + endLoc = endLoc.getLocWithOffset( + Lexer::MeasureTokenLength(endLoc, compiler.getSourceManager(), compiler.getLangOpts())); + + if (!insertText(endLoc, " const")) + { + report(DiagnosticsEngine::Warning, "Could not mark field as const", + compat::getBeginLoc(fieldDecl)) + << fieldDecl->getSourceRange(); + } + return true; +} + +loplugin::Plugin::Registration<ConstFieldsRewrite> X("constfieldsrewrite", false); +} + +#endif + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/compilerplugins/clang/test/constfields.cxx b/compilerplugins/clang/test/constfields.cxx new file mode 100644 index 000000000000..c045396d5432 --- /dev/null +++ b/compilerplugins/clang/test/constfields.cxx @@ -0,0 +1,45 @@ +/* -*- 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 <vector> +#include <ostream> +#include <com/sun/star/uno/Any.hxx> + +struct Test0 +{ + void method1() {} +}; + +// checking for calling non-const method +struct Test1 +// expected-error@-1 {{notconst m_field1 [loplugin:constfields]}} +{ + Test0* m_field1; + + void method1() + { + if (m_field1) + m_field1->method1(); + } +}; + +// checking for assigning to field +struct Test2 +// expected-error@-1 {{notconst m_field1 [loplugin:constfields]}} +{ + Test0* m_field1; + + Test2() + : m_field1(nullptr) + { + } + + void method1() { m_field1 = nullptr; } +}; +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/compilerplugins/clang/unusedfields.cxx b/compilerplugins/clang/unusedfields.cxx index ac8a4d1aabf2..5ba6ac3c1cd6 100644 --- a/compilerplugins/clang/unusedfields.cxx +++ b/compilerplugins/clang/unusedfields.cxx @@ -70,7 +70,6 @@ static std::set<MyFieldInfo> touchedFromOutsideSet; static std::set<MyFieldInfo> touchedFromOutsideConstructorSet; static std::set<MyFieldInfo> readFromSet; static std::set<MyFieldInfo> writeToSet; -static std::set<MyFieldInfo> writeToOutsideConstructorSet; static std::set<MyFieldInfo> definitionSet; /** @@ -160,7 +159,6 @@ public: private: MyFieldInfo niceName(const FieldDecl*); void checkTouchedFromOutside(const FieldDecl* fieldDecl, const Expr* memberExpr); - void checkWriteFromOutsideConstructor(const FieldDecl* fieldDecl, const Expr* memberExpr); void checkWriteOnly(const FieldDecl* fieldDecl, const Expr* memberExpr); void checkReadOnly(const FieldDecl* fieldDecl, const Expr* memberExpr); bool isSomeKindOfZero(const Expr* arg); @@ -195,8 +193,6 @@ void UnusedFields::run() output += "read:\t" + s.parentClass + "\t" + s.fieldName + "\n"; for (const MyFieldInfo & s : writeToSet) output += "write:\t" + s.parentClass + "\t" + s.fieldName + "\n"; - for (const MyFieldInfo & s : writeToOutsideConstructorSet) - output += "write-outside-constructor:\t" + s.parentClass + "\t" + s.fieldName + "\n"; for (const MyFieldInfo & s : definitionSet) output += "definition:\t" + s.access + "\t" + s.parentClass + "\t" + s.fieldName + "\t" + s.fieldType + "\t" + s.sourceLocation + "\n"; std::ofstream myfile; @@ -676,8 +672,6 @@ void UnusedFields::checkReadOnly(const FieldDecl* fieldDecl, const Expr* memberE // we don't care about writes to a field when inside the copy/move constructor/operator= for that field if (cxxRecordDecl1 && (cxxRecordDecl1 == insideMoveOrCopyOrCloneDeclParent)) { - // ... but they matter to tbe can-be-const analysis - checkWriteFromOutsideConstructor(fieldDecl, memberExpr); return; } } @@ -891,7 +885,6 @@ void UnusedFields::checkReadOnly(const FieldDecl* fieldDecl, const Expr* memberE if (bPotentiallyWrittenTo) { writeToSet.insert(fieldInfo); - checkWriteFromOutsideConstructor(fieldDecl, memberExpr); } } @@ -1021,27 +1014,6 @@ void UnusedFields::checkTouchedFromOutside(const FieldDecl* fieldDecl, const Exp } } -// For the const-field analysis. -// Called when we have a write to a field, and we want to record that write only if it's writing from -// outside the constructor. -void UnusedFields::checkWriteFromOutsideConstructor(const FieldDecl* fieldDecl, const Expr* memberExpr) { - const FunctionDecl* memberExprParentFunction = getParentFunctionDecl(memberExpr); - bool doWrite = false; - - if (!memberExprParentFunction) - // If we are not inside a function - doWrite = true; - else if (memberExprParentFunction->getParent() != fieldDecl->getParent()) - // or we are inside a method from another class (than the one the field belongs to) - doWrite = true; - else if (!isa<CXXConstructorDecl>(memberExprParentFunction)) - // or we are not inside constructor - doWrite = true; - - if (doWrite) - writeToOutsideConstructorSet.insert(niceName(fieldDecl)); -} - llvm::Optional<CalleeWrapper> UnusedFields::getCallee(CallExpr const * callExpr) { FunctionDecl const * functionDecl = callExpr->getDirectCallee(); diff --git a/compilerplugins/clang/unusedfields.py b/compilerplugins/clang/unusedfields.py index a7910bfd9768..dcb37a72017f 100755 --- a/compilerplugins/clang/unusedfields.py +++ b/compilerplugins/clang/unusedfields.py @@ -13,7 +13,6 @@ touchedFromOutsideSet = set() touchedFromOutsideConstructorSet = set() readFromSet = set() writeToSet = set() -writeFromOutsideConstructorSet = set() sourceLocationSet = set() # clang does not always use exactly the same numbers in the type-parameter vars it generates @@ -56,8 +55,6 @@ with io.open("workdir/loplugin.unusedfields.log", "rb", buffering=1024*1024) as readFromSet.add(parseFieldInfo(tokens)) elif tokens[0] == "write:": writeToSet.add(parseFieldInfo(tokens)) - elif tokens[0] == "write-outside-constructor:": - writeFromOutsideConstructorSet.add(parseFieldInfo(tokens)) else: print( "unknown line: " + line) @@ -226,30 +223,6 @@ for d in protectedAndPublicDefinitionSet: canBePrivateSet.add((clazz + " " + definitionToTypeMap[d], srcLoc)) -# Calculate can-be-const-field set -canBeConstFieldSet = set() -for d in definitionSet: - if d in writeFromOutsideConstructorSet: - continue - srcLoc = definitionToSourceLocationMap[d]; - fieldType = definitionToTypeMap[d] - if fieldType.startswith("const "): - continue - if "std::unique_ptr" in fieldType: - continue - if "std::shared_ptr" in fieldType: - continue - if "Reference<" in fieldType: - continue - if "VclPtr<" in fieldType: - continue - if "osl::Mutex" in fieldType: - continue - if "::sfx2::sidebar::ControllerItem" in fieldType: - continue - canBeConstFieldSet.add((d[0] + " " + d[1] + " " + fieldType, srcLoc)) - - # sort the results using a "natural order" so sequences like [item1,item2,item10] sort nicely def natural_sort_key(s, _nsre=re.compile('([0-9]+)')): return [int(text) if text.isdigit() else text.lower() @@ -261,7 +234,6 @@ tmp2list = sorted(writeonlySet, key=lambda v: natural_sort_key(v[1])) tmp3list = sorted(canBePrivateSet, key=lambda v: natural_sort_key(v[1])) tmp4list = sorted(readonlySet, key=lambda v: natural_sort_key(v[1])) tmp5list = sorted(onlyUsedInConstructorSet, key=lambda v: natural_sort_key(v[1])) -tmp6list = sorted(canBeConstFieldSet, key=lambda v: natural_sort_key(v[1])) # print out the results with open("compilerplugins/clang/unusedfields.untouched.results", "wt") as f: @@ -285,9 +257,5 @@ with open("compilerplugins/clang/unusedfields.only-used-in-constructor.results", for t in tmp5list: f.write( t[1] + "\n" ) f.write( " " + t[0] + "\n" ) -with open("compilerplugins/clang/unusedfields.can-be-const.results", "wt") as f: - for t in tmp6list: - f.write( t[1] + "\n" ) - f.write( " " + t[0] + "\n" ) diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk index 6aacf7547c60..ad9b1e3f5192 100644 --- a/solenv/CompilerTest_compilerplugins_clang.mk +++ b/solenv/CompilerTest_compilerplugins_clang.mk @@ -14,6 +14,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \ compilerplugins/clang/test/blockblock \ compilerplugins/clang/test/casttovoid \ compilerplugins/clang/test/commaoperator \ + compilerplugins/clang/test/constfields \ compilerplugins/clang/test/constparams \ compilerplugins/clang/test/conststringfield \ compilerplugins/clang/test/convertlong \ _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits