[Libreoffice-commits] core.git: compilerplugins/clang editeng/source filter/source lingucomponent/source solenv/CompilerTest_compilerplugins_clang.mk xmlscript/source xmlsecurity/qa xmlsecurity/source
Noel Grandin (via logerrit)
logerrit at kemper.freedesktop.org
Fri Aug 16 07:05:27 UTC 2019
compilerplugins/clang/sequenceloop.cxx | 76 ++++++++++++
compilerplugins/clang/test/sequenceloop.cxx | 35 +++++
editeng/source/accessibility/AccessibleEditableTextPara.cxx | 4
filter/source/xsltdialog/xmlfiltersettingsdialog.cxx | 4
filter/source/xsltdialog/xmlfiltertestdialog.cxx | 4
lingucomponent/source/spellcheck/spell/sspellimp.cxx | 8 -
solenv/CompilerTest_compilerplugins_clang.mk | 1
xmlscript/source/xmldlg_imexp/xmldlg_expmodels.cxx | 6
xmlscript/source/xmldlg_imexp/xmldlg_export.cxx | 2
xmlscript/source/xmlflat_imexp/xmlbas_export.cxx | 4
xmlsecurity/qa/unit/signing/signing.cxx | 2
xmlsecurity/source/dialogs/certificatechooser.cxx | 2
xmlsecurity/source/dialogs/macrosecurity.cxx | 2
xmlsecurity/source/helper/documentsignaturehelper.cxx | 4
xmlsecurity/source/helper/documentsignaturemanager.cxx | 4
xmlsecurity/source/helper/ooxmlsecexporter.cxx | 2
xmlsecurity/source/xmlsec/nss/xmlsignature_nssimpl.cxx | 3
17 files changed, 138 insertions(+), 25 deletions(-)
New commits:
commit 7d7a7862a7dccb6764f80258f28685a0da3cbbcb
Author: Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Wed Aug 14 11:14:08 2019 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Fri Aug 16 09:04:51 2019 +0200
new loplugin:sequenceloop
look for places we should be using std::as_const on for-range
loops over uno::Sequence, to avoid triggering a copy
Change-Id: I7efb641bf09d37c87946f03428ee4eec90298c8a
Reviewed-on: https://gerrit.libreoffice.org/77441
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
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 at +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;
More information about the Libreoffice-commits
mailing list