[Libreoffice-commits] core.git: compilerplugins/clang include/toolkit toolkit/source

[Noel Grandin]

 compilerplugins/clang/unusedfields.readonly.results |    2 
 include/toolkit/controls/eventcontainer.hxx         |   16 -----
 toolkit/source/controls/eventcontainer.cxx          |   56 ++++++--------------
 3 files changed, 20 insertions(+), 54 deletions(-)

New commits:
commit 85f08e3e34bea01456eaf8989ac4f77d3900d5c5
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date:   Thu Jul 20 12:41:28 2017 +0200

    remove some "optimisation" insanity in ScriptEventContainer
    
    I can only imagine the weird bugs that must have periodically resulted
    when we had a hash value collision.
    In the process, fix hasElements() to return a useful value
    
    Change-Id: I1d9a052e73332b4b2bbc9c1fd8142c13eb22f1be
    Reviewed-on: https://gerrit.libreoffice.org/40226
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
    Tested-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/compilerplugins/clang/unusedfields.readonly.results b/compilerplugins/clang/unusedfields.readonly.results
index f36b53dea1cb..4dbd91675443 100644
--- a/compilerplugins/clang/unusedfields.readonly.results
+++ b/compilerplugins/clang/unusedfields.readonly.results
@@ -686,8 +686,6 @@ include/test/sheet/xdatapilottable.hxx:32
     apitest::XDataPilotTable xCellForCheck css::uno::Reference<css::table::XCell>
 include/test/sheet/xnamedranges.hxx:38
     apitest::XNamedRanges xSheet css::uno::Reference<css::sheet::XSpreadsheet>
-include/toolkit/controls/eventcontainer.hxx:52
-    toolkit::ScriptEventContainer mnElementCount sal_Int32
 include/tools/inetmime.hxx:66
     INetContentTypeParameter m_bConverted _Bool
 include/tools/multisel.hxx:43
diff --git a/include/toolkit/controls/eventcontainer.hxx b/include/toolkit/controls/eventcontainer.hxx
index aba5ce9f3aa4..859b06ee45a3 100644
--- a/include/toolkit/controls/eventcontainer.hxx
+++ b/include/toolkit/controls/eventcontainer.hxx
@@ -32,24 +32,12 @@
 namespace toolkit
 {
 
-// Hashtable to optimize
-typedef std::unordered_map
-<
-    OUString,
-    sal_Int32,
-    OUStringHash
->
-NameContainerNameMap;
-
-
 class ScriptEventContainer : public ::cppu::WeakImplHelper<
                                         css::container::XNameContainer,
                                         css::container::XContainer >
 {
-    NameContainerNameMap mHashMap;
-    css::uno::Sequence< OUString > mNames;
-    std::vector< css::uno::Any > mValues;
-    sal_Int32 mnElementCount;
+    std::unordered_map< OUString, css::uno::Any, OUStringHash>
+                   mHashMap;
     css::uno::Type mType;
 
     ContainerListenerMultiplexer maContainerListeners;
diff --git a/toolkit/source/controls/eventcontainer.cxx b/toolkit/source/controls/eventcontainer.cxx
index 3cc2aa802773..f4df038c405a 100644
--- a/toolkit/source/controls/eventcontainer.cxx
+++ b/toolkit/source/controls/eventcontainer.cxx
@@ -46,33 +46,33 @@ Type ScriptEventContainer::getElementType()
 
 sal_Bool ScriptEventContainer::hasElements()
 {
-    bool bRet = (mnElementCount > 0);
-    return bRet;
+    return !mHashMap.empty();
 }
 
 // Methods XNameAccess
 Any ScriptEventContainer::getByName( const OUString& aName )
 {
-    NameContainerNameMap::iterator aIt = mHashMap.find( aName );
+    auto aIt = mHashMap.find( aName );
     if( aIt == mHashMap.end() )
     {
         throw NoSuchElementException();
     }
-    sal_Int32 iHashResult = (*aIt).second;
-    Any aRetAny = mValues[ iHashResult ];
-    return aRetAny;
+    return aIt->second;
 }
 
 Sequence< OUString > ScriptEventContainer::getElementNames()
 {
-    return mNames;
+    Sequence<OUString> aRet(mHashMap.size());
+    int i = 0;
+    for (auto const & pair : mHashMap)
+        aRet[i++] = pair.first;
+    return aRet;
 }
 
 sal_Bool ScriptEventContainer::hasByName( const OUString& aName )
 {
-    NameContainerNameMap::iterator aIt = mHashMap.find( aName );
-    bool bRet = ( aIt != mHashMap.end() );
-    return bRet;
+    auto aIt = mHashMap.find( aName );
+    return aIt != mHashMap.end();
 }
 
 
@@ -83,14 +83,13 @@ void ScriptEventContainer::replaceByName( const OUString& aName, const Any& aEle
     if( mType != aAnyType )
         throw IllegalArgumentException();
 
-    NameContainerNameMap::iterator aIt = mHashMap.find( aName );
+    auto aIt = mHashMap.find( aName );
     if( aIt == mHashMap.end() )
     {
         throw NoSuchElementException();
     }
-    sal_Int32 iHashResult = (*aIt).second;
-    Any aOldElement = mValues[ iHashResult ];
-    mValues[ iHashResult ] = aElement;
+    Any aOldElement = aIt->second;
+    aIt->second = aElement;
 
     // Fire event
     ContainerEvent aEvent;
@@ -109,18 +108,13 @@ void ScriptEventContainer::insertByName( const OUString& aName, const Any& aElem
     if( mType != aAnyType )
         throw IllegalArgumentException();
 
-    NameContainerNameMap::iterator aIt = mHashMap.find( aName );
+    auto aIt = mHashMap.find( aName );
     if( aIt != mHashMap.end() )
     {
         throw ElementExistException();
     }
 
-    sal_Int32 nCount = mNames.getLength();
-    mNames.realloc( nCount + 1 );
-    mValues.resize( nCount + 1 );
-    mNames.getArray()[ nCount ] = aName;
-    mValues[ nCount ] = aElement;
-    mHashMap[ aName ] = nCount;
+    mHashMap[ aName ] = aElement;
 
     // Fire event
     ContainerEvent aEvent;
@@ -132,33 +126,20 @@ void ScriptEventContainer::insertByName( const OUString& aName, const Any& aElem
 
 void ScriptEventContainer::removeByName( const OUString& Name )
 {
-    NameContainerNameMap::iterator aIt = mHashMap.find( Name );
+    auto aIt = mHashMap.find( Name );
     if( aIt == mHashMap.end() )
     {
         throw NoSuchElementException();
     }
 
-    sal_Int32 iHashResult = (*aIt).second;
-    Any aOldElement = mValues[ iHashResult ];
-
     // Fire event
     ContainerEvent aEvent;
     aEvent.Source = *this;
-    aEvent.Element = aOldElement;
+    aEvent.Element = aIt->second;
     aEvent.Accessor <<= Name;
     maContainerListeners.elementRemoved( aEvent );
 
     mHashMap.erase( aIt );
-    sal_Int32 iLast = mNames.getLength() - 1;
-    if( iLast != iHashResult )
-    {
-        OUString* pNames = mNames.getArray();
-        pNames[ iHashResult ] = pNames[ iLast ];
-        mValues[ iHashResult ] = mValues[ iLast ];
-        mHashMap[ pNames[ iHashResult ] ] = iHashResult;
-    }
-    mNames.realloc( iLast );
-    mValues.resize( iLast );
 }
 
 // Methods XContainer
@@ -174,8 +155,7 @@ void ScriptEventContainer::removeContainerListener( const css::uno::Reference< c
 
 
 ScriptEventContainer::ScriptEventContainer()
-    : mnElementCount( 0 ),
-      mType( cppu::UnoType<ScriptEventDescriptor>::get() ),
+    : mType( cppu::UnoType<ScriptEventDescriptor>::get() ),
       maContainerListeners( *this )
 {
 }