diff options
17 files changed, 138 insertions, 25 deletions
diff --git a/compilerplugins/clang/sequenceloop.cxx b/compilerplugins/clang/sequenceloop.cxx new file mode 100644 index 000000000000..340412a5137a --- /dev/null +++ b/compilerplugins/clang/sequenceloop.cxx @@ -0,0 +1,76 @@ +/* -*- 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/. + */ + +#include <cassert> +#include <string> +#include <iostream> +#include <fstream> +#include <set> + +#include <clang/AST/CXXInheritance.h> +#include "plugin.hxx" +#include "check.hxx" + +/** + When used in "for" loops, css::uno::Sequence objects tend to end up calling the non-const begin()/end(), + which is considerably more expensive than the const variants because it forces a local copy + of the internal ref-counted impl object. +*/ + +namespace +{ +class SequenceLoop : public loplugin::FilteringPlugin<SequenceLoop> +{ +public: + explicit SequenceLoop(loplugin::InstantiationData const& data) + : FilteringPlugin(data) + { + } + + virtual void run() override + { + if (preRun()) + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } + + bool VisitCXXForRangeStmt(CXXForRangeStmt const*); +}; + +bool SequenceLoop::VisitCXXForRangeStmt(CXXForRangeStmt const* forStmt) +{ + if (ignoreLocation(forStmt)) + return true; + + auto tc = loplugin::TypeCheck(forStmt->getRangeInit()->getType()); + if (tc.Const()) + return true; + if (!tc.Class("Sequence") + .Namespace("uno") + .Namespace("star") + .Namespace("sun") + .Namespace("com") + .GlobalNamespace()) + return true; + const VarDecl* varDecl = forStmt->getLoopVariable(); + auto tc2 = loplugin::TypeCheck(varDecl->getType()); + if (!tc2.LvalueReference().Const()) + return true; + + report(DiagnosticsEngine::Warning, + "use std::as_const, or make range var const, to avoid creating a copy of the Sequence", + compat::getBeginLoc(forStmt)) + << forStmt->getSourceRange(); + return true; +} + +loplugin::Plugin::Registration<SequenceLoop> X("sequenceloop"); + +} // namespace + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/compilerplugins/clang/test/sequenceloop.cxx b/compilerplugins/clang/test/sequenceloop.cxx new file mode 100644 index 000000000000..113e8fd588fe --- /dev/null +++ b/compilerplugins/clang/test/sequenceloop.cxx @@ -0,0 +1,35 @@ +/* -*- 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 <com/sun/star/uno/Sequence.hxx> +#include <com/sun/star/uno/XInterface.hpp> +#include <utility> + +namespace test1 +{ +void foo(css::uno::Sequence<css::uno::Reference<css::uno::XInterface>>& aSeq) +{ + // expected-error@+1 {{use std::as_const, or make range var const, to avoid creating a copy of the Sequence [loplugin:sequenceloop]}} + for (const auto& x : aSeq) + x.get(); + // no warning expected + for (auto& x : aSeq) + x.get(); + for (const auto& x : std::as_const(aSeq)) + x.get(); +} +// no warning expected +void foo2(const css::uno::Sequence<css::uno::Reference<css::uno::XInterface>>& aSeq) +{ + for (const auto& x : aSeq) + x.get(); +} +}; + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/editeng/source/accessibility/AccessibleEditableTextPara.cxx b/editeng/source/accessibility/AccessibleEditableTextPara.cxx index 01c0f274a0b9..79c7ecbc9639 100644 --- a/editeng/source/accessibility/AccessibleEditableTextPara.cxx +++ b/editeng/source/accessibility/AccessibleEditableTextPara.cxx @@ -2424,7 +2424,7 @@ namespace accessibility uno::Sequence< beans::PropertyValue > aOutSequence( aProperties.getLength() ); beans::PropertyValue* pOutSequence = aOutSequence.getArray(); sal_Int32 nOutLen = 0; - for (const beans::Property& rProperty : aProperties) + for (const beans::Property& rProperty : std::as_const(aProperties)) { // calling implementation functions: // _getPropertyState and _getPropertyValue (see below) to provide @@ -2510,7 +2510,7 @@ namespace accessibility uno::Sequence< beans::PropertyValue > aOutSequence( aProperties.getLength() ); beans::PropertyValue* pOutSequence = aOutSequence.getArray(); sal_Int32 nOutLen = 0; - for (const beans::Property& rProperty : aProperties) + for (const beans::Property& rProperty : std::as_const(aProperties)) { // calling 'regular' functions that will operate on the selection PropertyState eState = xPropSet->getPropertyState( rProperty.Name ); diff --git a/filter/source/xsltdialog/xmlfiltersettingsdialog.cxx b/filter/source/xsltdialog/xmlfiltersettingsdialog.cxx index 27502305610d..5f7dcc7b9039 100644 --- a/filter/source/xsltdialog/xmlfiltersettingsdialog.cxx +++ b/filter/source/xsltdialog/xmlfiltersettingsdialog.cxx @@ -340,7 +340,7 @@ OUString XMLFilterSettingsDialog::createUniqueInterfaceName( const OUString& rIn try { - Sequence< OUString > aFilterNames( mxFilterContainer->getElementNames() ); + const Sequence< OUString > aFilterNames( mxFilterContainer->getElementNames() ); Sequence< PropertyValue > aValues; for( OUString const & filterName : aFilterNames) @@ -960,7 +960,7 @@ void XMLFilterSettingsDialog::initFilterList() { if( mxFilterContainer.is() ) { - Sequence< OUString > aFilterNames( mxFilterContainer->getElementNames() ); + const Sequence< OUString > aFilterNames( mxFilterContainer->getElementNames() ); Sequence< PropertyValue > aValues; diff --git a/filter/source/xsltdialog/xmlfiltertestdialog.cxx b/filter/source/xsltdialog/xmlfiltertestdialog.cxx index 684329f438a8..24e3c927fa3b 100644 --- a/filter/source/xsltdialog/xmlfiltertestdialog.cxx +++ b/filter/source/xsltdialog/xmlfiltertestdialog.cxx @@ -290,7 +290,7 @@ void XMLFilterTestDialog::onExportBrowse() Reference< XNameAccess > xTypeDetection( mxContext->getServiceManager()->createInstanceWithContext( "com.sun.star.document.TypeDetection", mxContext ), UNO_QUERY ); if( xFilterContainer.is() && xTypeDetection.is() ) { - Sequence< OUString > aFilterNames( xFilterContainer->getElementNames() ); + const Sequence< OUString > aFilterNames( xFilterContainer->getElementNames() ); for( OUString const & filterName : aFilterNames ) { @@ -306,7 +306,7 @@ void XMLFilterTestDialog::onExportBrowse() int nFound = 0; - for( const PropertyValue& rValue : aValues ) + for( const PropertyValue& rValue : std::as_const(aValues) ) { if ( rValue.Name == "Type" ) { diff --git a/lingucomponent/source/spellcheck/spell/sspellimp.cxx b/lingucomponent/source/spellcheck/spell/sspellimp.cxx index 3bbf8cc66f36..1e88d04874c1 100644 --- a/lingucomponent/source/spellcheck/spell/sspellimp.cxx +++ b/lingucomponent/source/spellcheck/spell/sspellimp.cxx @@ -124,7 +124,7 @@ Sequence< Locale > SAL_CALL SpellChecker::getLocales() uno::Sequence< OUString > aFormatList; aLinguCfg.GetSupportedDictionaryFormatsFor( "SpellCheckers", "org.openoffice.lingu.MySpellSpellChecker", aFormatList ); - for (auto const& format : aFormatList) + for (auto const& format : std::as_const(aFormatList)) { std::vector< SvtLinguConfigDictionaryEntry > aTmpDic( aLinguCfg.GetActiveDictionariesByFormat(format) ); @@ -150,7 +150,7 @@ Sequence< Locale > SAL_CALL SpellChecker::getLocales() std::set<OUString> aLocaleNamesSet; for (auto const& dict : aDics) { - uno::Sequence< OUString > aLocaleNames( dict.aLocaleNames ); + const uno::Sequence< OUString > aLocaleNames( dict.aLocaleNames ); uno::Sequence< OUString > aLocations( dict.aLocations ); SAL_WARN_IF( aLocaleNames.hasElements() && !aLocations.hasElements(), @@ -200,7 +200,7 @@ Sequence< Locale > SAL_CALL SpellChecker::getLocales() if (dict.aLocaleNames.hasElements() && dict.aLocations.hasElements()) { - uno::Sequence< OUString > aLocaleNames( dict.aLocaleNames ); + const uno::Sequence< OUString > aLocaleNames( dict.aLocaleNames ); // currently only one language per dictionary is supported in the actual implementation... // Thus here we work-around this by adding the same dictionary several times. @@ -238,7 +238,7 @@ sal_Bool SAL_CALL SpellChecker::hasLocale(const Locale& rLocale) if (!m_aSuppLocales.hasElements()) getLocales(); - for (auto const& suppLocale : m_aSuppLocales) + for (auto const& suppLocale : std::as_const(m_aSuppLocales)) { if (rLocale == suppLocale) { diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk index dfa82b5dc18f..711137705ba3 100644 --- a/solenv/CompilerTest_compilerplugins_clang.mk +++ b/solenv/CompilerTest_compilerplugins_clang.mk @@ -56,6 +56,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \ compilerplugins/clang/test/salunicodeliteral \ compilerplugins/clang/test/selfinit \ compilerplugins/clang/test/sequentialassign \ + compilerplugins/clang/test/sequenceloop \ compilerplugins/clang/test/shouldreturnbool \ compilerplugins/clang/test/simplifybool \ compilerplugins/clang/test/simplifyconstruct \ diff --git a/xmlscript/source/xmldlg_imexp/xmldlg_expmodels.cxx b/xmlscript/source/xmldlg_imexp/xmldlg_expmodels.cxx index cf0a13e39235..09910dfb6a89 100644 --- a/xmlscript/source/xmldlg_imexp/xmldlg_expmodels.cxx +++ b/xmlscript/source/xmldlg_imexp/xmldlg_expmodels.cxx @@ -318,7 +318,7 @@ void ElementDescriptor::readComboBoxModel( StyleBag * all_styles ) { ElementDescriptor * popup = new ElementDescriptor( _xProps, _xPropState, XMLNS_DIALOGS_PREFIX ":menupopup", _xDocument ); - for ( const auto& rItemValue : itemValues ) + for ( const auto& rItemValue : std::as_const(itemValues) ) { ElementDescriptor * item = new ElementDescriptor( _xProps, _xPropState, XMLNS_DIALOGS_PREFIX ":menuitem", _xDocument ); item->addAttribute( XMLNS_DIALOGS_PREFIX ":value", rItemValue ); @@ -365,7 +365,7 @@ void ElementDescriptor::readListBoxModel( StyleBag * all_styles ) { ElementDescriptor * popup = new ElementDescriptor( _xProps, _xPropState, XMLNS_DIALOGS_PREFIX ":menupopup", _xDocument ); - for ( const auto& rItemValue : itemValues ) + for ( const auto& rItemValue : std::as_const(itemValues) ) { ElementDescriptor * item = new ElementDescriptor(_xProps, _xPropState, XMLNS_DIALOGS_PREFIX ":menuitem", _xDocument ); item->addAttribute( XMLNS_DIALOGS_PREFIX ":value", rItemValue ); @@ -1086,7 +1086,7 @@ void ElementDescriptor::readBullitinBoard( StyleBag * all_styles ) Reference< container::XNameContainer > xDialogModel( _xProps, UNO_QUERY ); if ( !xDialogModel.is() ) return; // #TODO throw??? - Sequence< OUString > aElements( xDialogModel->getElementNames() ); + const Sequence< OUString > aElements( xDialogModel->getElementNames() ); ElementDescriptor * pRadioGroup = nullptr; diff --git a/xmlscript/source/xmldlg_imexp/xmldlg_export.cxx b/xmlscript/source/xmldlg_imexp/xmldlg_export.cxx index 3deca0ba6c04..2ad8ae7b929f 100644 --- a/xmlscript/source/xmldlg_imexp/xmldlg_export.cxx +++ b/xmlscript/source/xmldlg_imexp/xmldlg_export.cxx @@ -1151,7 +1151,7 @@ void ElementDescriptor::readEvents() Reference< container::XNameContainer > xEvents( xSupplier->getEvents() ); if (xEvents.is()) { - Sequence< OUString > aNames( xEvents->getElementNames() ); + const Sequence< OUString > aNames( xEvents->getElementNames() ); for ( const auto& rName : aNames ) { script::ScriptEventDescriptor descr; diff --git a/xmlscript/source/xmlflat_imexp/xmlbas_export.cxx b/xmlscript/source/xmlflat_imexp/xmlbas_export.cxx index 76a20aa3a5a5..6101ec60ecfd 100644 --- a/xmlscript/source/xmlflat_imexp/xmlbas_export.cxx +++ b/xmlscript/source/xmlflat_imexp/xmlbas_export.cxx @@ -153,7 +153,7 @@ sal_Bool XMLBasicExporterBase::filter( const Sequence< beans::PropertyValue >& / if ( xLibContainer.is() ) { - Sequence< OUString > aLibNames = xLibContainer->getElementNames(); + const Sequence< OUString > aLibNames = xLibContainer->getElementNames(); for ( const OUString& rLibName : aLibNames ) { if ( xLibContainer->hasByName( rLibName ) ) @@ -229,7 +229,7 @@ sal_Bool XMLBasicExporterBase::filter( const Sequence< beans::PropertyValue >& / if ( xLib.is() ) { - Sequence< OUString > aModNames = xLib->getElementNames(); + const Sequence< OUString > aModNames = xLib->getElementNames(); for ( const OUString& rModName : aModNames ) { if ( xLib->hasByName( rModName ) ) diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx index da2019c26eba..deb8cf2f2032 100644 --- a/xmlsecurity/qa/unit/signing/signing.cxx +++ b/xmlsecurity/qa/unit/signing/signing.cxx @@ -190,7 +190,7 @@ SigningTest::getCertificate(DocumentSignatureManager& rSignatureManager, { uno::Reference<xml::crypto::XSecurityEnvironment> xSecurityEnvironment = rSignatureManager.getSecurityEnvironment(); - uno::Sequence<uno::Reference<security::XCertificate>> aCertificates + const uno::Sequence<uno::Reference<security::XCertificate>> aCertificates = xSecurityEnvironment->getPersonalCertificates(); for (const auto& xCertificate : aCertificates) diff --git a/xmlsecurity/source/dialogs/certificatechooser.cxx b/xmlsecurity/source/dialogs/certificatechooser.cxx index ec499bc159b0..32c688f2a257 100644 --- a/xmlsecurity/source/dialogs/certificatechooser.cxx +++ b/xmlsecurity/source/dialogs/certificatechooser.cxx @@ -198,7 +198,7 @@ void CertificateChooser::ImplInitialize() // fill list of certificates; the first entry will be selected - for ( const auto& xCert : xCerts ) + for ( const auto& xCert : std::as_const(xCerts) ) { std::shared_ptr<UserData> userData = std::make_shared<UserData>(); userData->xCertificate = xCert; diff --git a/xmlsecurity/source/dialogs/macrosecurity.cxx b/xmlsecurity/source/dialogs/macrosecurity.cxx index 04f8f0f9dd86..4e0dcc76e2fb 100644 --- a/xmlsecurity/source/dialogs/macrosecurity.cxx +++ b/xmlsecurity/source/dialogs/macrosecurity.cxx @@ -357,7 +357,7 @@ MacroSecurityTrustedSourcesTP::MacroSecurityTrustedSourcesTP(weld::Container* pP FillCertLB(); - css::uno::Sequence< OUString > aSecureURLs = m_pDlg->m_aSecOptions.GetSecureURLs(); + const css::uno::Sequence< OUString > aSecureURLs = m_pDlg->m_aSecOptions.GetSecureURLs(); mbURLsReadonly = m_pDlg->m_aSecOptions.IsReadOnly( SvtSecurityOptions::EOption::SecureUrls ); m_xTrustFileROFI->set_visible(mbURLsReadonly); m_xTrustFileLocLB->set_sensitive(!mbURLsReadonly); diff --git a/xmlsecurity/source/helper/documentsignaturehelper.cxx b/xmlsecurity/source/helper/documentsignaturehelper.cxx index ee5fb4989ec1..c097b14a2180 100644 --- a/xmlsecurity/source/helper/documentsignaturehelper.cxx +++ b/xmlsecurity/source/helper/documentsignaturehelper.cxx @@ -86,7 +86,7 @@ static void ImplFillElementList( const OUString& rRootStorageName, const bool bRecursive, const DocumentSignatureAlgorithm mode) { - Sequence< OUString > aElements = rxStore->getElementNames(); + const Sequence< OUString > aElements = rxStore->getElementNames(); for ( const auto& rName : aElements ) { @@ -213,7 +213,7 @@ DocumentSignatureHelper::CreateElementList( xSubStore.clear(); // Object folders... - Sequence< OUString > aElementNames = rxStore->getElementNames(); + const Sequence< OUString > aElementNames = rxStore->getElementNames(); for ( const auto& rName : aElementNames ) { if ( ( rName.match( "Object " ) ) && rxStore->isStorageElement( rName ) ) diff --git a/xmlsecurity/source/helper/documentsignaturemanager.cxx b/xmlsecurity/source/helper/documentsignaturemanager.cxx index 557557f6bae8..19cdc2fb4d1d 100644 --- a/xmlsecurity/source/helper/documentsignaturemanager.cxx +++ b/xmlsecurity/source/helper/documentsignaturemanager.cxx @@ -177,7 +177,7 @@ bool DocumentSignatureManager::isXML(const OUString& rURI) if (readManifest()) { - for (const uno::Sequence<beans::PropertyValue>& entry : m_manifest) + for (const uno::Sequence<beans::PropertyValue>& entry : std::as_const(m_manifest)) { OUString sPath; OUString sMediaType; @@ -393,7 +393,7 @@ bool DocumentSignatureManager::add( eAlgorithmID); } - uno::Sequence<uno::Reference<security::XCertificate>> aCertPath + const uno::Sequence<uno::Reference<security::XCertificate>> aCertPath = xSecurityContext->getSecurityEnvironment()->buildCertificatePath(xCert); OUStringBuffer aStrBuffer; diff --git a/xmlsecurity/source/helper/ooxmlsecexporter.cxx b/xmlsecurity/source/helper/ooxmlsecexporter.cxx index 062c76d5c47b..cf87d6e1ad17 100644 --- a/xmlsecurity/source/helper/ooxmlsecexporter.cxx +++ b/xmlsecurity/source/helper/ooxmlsecexporter.cxx @@ -242,7 +242,7 @@ void OOXMLSecExporter::Impl::writeRelationshipTransform(const OUString& rURI) m_xDocumentHandler->startElement("Transform", uno::Reference<xml::sax::XAttributeList>(pAttributeList.get())); } - uno::Sequence< uno::Sequence<beans::StringPair> > aRelationsInfo = comphelper::OFOPXMLHelper::ReadRelationsInfoSequence(xRelStream, rURI, m_xComponentContext); + const uno::Sequence< uno::Sequence<beans::StringPair> > aRelationsInfo = comphelper::OFOPXMLHelper::ReadRelationsInfoSequence(xRelStream, rURI, m_xComponentContext); for (const uno::Sequence<beans::StringPair>& rPairs : aRelationsInfo) { OUString aId; diff --git a/xmlsecurity/source/xmlsec/nss/xmlsignature_nssimpl.cxx b/xmlsecurity/source/xmlsec/nss/xmlsignature_nssimpl.cxx index 19818823980b..f3a76c76e099 100644 --- a/xmlsecurity/source/xmlsec/nss/xmlsignature_nssimpl.cxx +++ b/xmlsecurity/source/xmlsec/nss/xmlsignature_nssimpl.cxx @@ -288,7 +288,8 @@ OUString SAL_CALL XMLSignature_NssImpl::getImplementationName() /* XServiceInfo */ sal_Bool SAL_CALL XMLSignature_NssImpl::supportsService(const OUString& rServiceName) { - for (OUString const & rCurrentServiceName : getSupportedServiceNames()) + const css::uno::Sequence<OUString> aServiceNames = getSupportedServiceNames(); + for (OUString const & rCurrentServiceName : aServiceNames) { if (rCurrentServiceName == rServiceName) return true; |