[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.0-29' - l10ntools/inc l10ntools/source

Stephan Bergmann (via logerrit) logerrit at kemper.freedesktop.org
Mon Jan 13 04:44:17 UTC 2020


 l10ntools/inc/export.hxx   |   74 ---------------------------------
 l10ntools/source/merge.cxx |   99 +++++----------------------------------------
 2 files changed, 13 insertions(+), 160 deletions(-)

New commits:
commit 968f07ab7b47fe4f2724b4f7c9f037a0f4b376f3
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Mon Mar 18 18:34:39 2019 +0100
Commit:     Aron Budea <aron.budea at collabora.com>
CommitDate: Fri Jan 10 18:04:16 2020 +0100

    Remove broken MergeDataHashMap optimization
    
    ...which apparently didn't take into account that inserting into a
    std::unordered_map may invalidate iterators, which now started to cause build
    failures like
    
    > [PRP] dictionaries/hu_HU/dialog/hu_HU_de
    > warn:sal.osl:25175:25175:sal/osl/unx/thread.cxx:1026: RTL_TEXTENCODING_DONTKNOW -> _ASCII_US
    > .../gcc/trunk/inst/include/c++/9.0.1/debug/safe_iterator.h:307:
    > In function:
    >     __gnu_debug::_Safe_iterator<_Iterator, _Sequence, _Category>::pointer
    >     __gnu_debug::_Safe_iterator<_Iterator, _Sequence,
    >     _Category>::operator->() const [with _Iterator =
    >     std::__detail::_Node_iterator<std::pair<const rtl::OString,
    >     std::unique_ptr<MergeData> >, false, true>; _Sequence =
    >     std::__debug::unordered_map<rtl::OString, std::unique_ptr<MergeData> >;
    >     _Category = std::forward_iterator_tag;
    >     __gnu_debug::_Safe_iterator<_Iterator, _Sequence, _Category>::pointer =
    >     std::pair<const rtl::OString, std::unique_ptr<MergeData> >*]
    >
    > Error: attempt to dereference a singular iterator.
    >
    > Objects involved in the operation:
    >     iterator "this" @ 0x0x5a8228 {
    >       type = std::__detail::_Node_iterator<std::pair<rtl::OString const, std::unique_ptr<MergeData, std::default_delete<MergeData> > >, false, true> (mutable iterator);
    >       state = singular;
    >       references sequence with type 'std::__debug::unordered_map<rtl::OString, std::unique_ptr<MergeData, std::default_delete<MergeData> >, std::hash<rtl::OString>, std::equal_to<rtl::OString>, std::allocator<std::pair<rtl::OString const, std::unique_ptr<MergeData, std::default_delete<MergeData> > > > >' @ 0x0x5a81c8
    >     }
    > /bin/sh: line 1: 25175 Aborted                 (core dumped) LD_LIBRARY_PATH=${LD_LIBRARY_PATH:+$LD_LIBRARY_PATH:}"$I/program:$I/program" $W/LinkTarget/Executable/propex -i $S/dictionaries/hu_HU/dialog/hu_HU_en_US.properties -l de -m ${MERGEINPUT} -o $W/PropertiesTranslateTarget/dictionaries/hu_HU/dialog/hu_HU_de.properties
    > make[1]: *** [.../solenv/gbuild/Dictionary.mk:88: .../workdir/PropertiesTranslateTarget/dictionaries/hu_HU/dialog/hu_HU_de.properties] Error 134
    
    with trunk libstdc++ in debug mode.
    
    Both classes MergeData and MergeDataHashMap became redundant.
    
    Reviewed-on: https://gerrit.libreoffice.org/69395
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>
    (cherry picked from commit dc2329fb66dc38f62a21b1afaca38529d7e40fa9)
    
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/85866
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Aron Budea <aron.budea at collabora.com>
    Tested-by: Aron Budea <aron.budea at collabora.com>
    (cherry picked from commit 0df20a65441cacb58cddfa6e5451691088bfcf8f)
    
    Change-Id: I771548a6155e14037d84acfd74cd13566a26a8d7
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/85962
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Andras Timar <andras.timar at collabora.com>
    (cherry picked from commit d45cb911dd3853bc5f590d12265706aeaa9f0080)

diff --git a/l10ntools/inc/export.hxx b/l10ntools/inc/export.hxx
index 0ed0f45c5175..ced715071f2e 100644
--- a/l10ntools/inc/export.hxx
+++ b/l10ntools/inc/export.hxx
@@ -130,76 +130,6 @@ public:
 
 };
 
-
-// class MergeDataHashMap
-
-
-class MergeData;
-
-/** Container for MergeData
-
-  This class is an HashMap with a hidden insertion
-  order. The class can used just like a simple
-  HashMap, but good to know that it's use is
-  more effective if the accessing(find) order
-  match with the insertion order.
-
-  In the most case, this match is good.
-  (e.g. reading PO files of different languages,
-  executables merging)
-*/
-class MergeDataHashMap
-{
-    private:
-        typedef std::unordered_map<OString, MergeData*> HashMap_t;
-
-    public:
-        MergeDataHashMap()
-            : bFirstSearch(true)
-            , aLastInsertion(m_aHashMap.end())
-            , aLastFound(m_aHashMap.end())
-            , aFirstInOrder(m_aHashMap.end())
-        {
-        }
-
-        typedef HashMap_t::iterator iterator;
-        typedef HashMap_t::const_iterator const_iterator;
-
-        std::pair<iterator,bool> insert(const OString& rKey, MergeData* pMergeData);
-        iterator const & find(const OString& rKey);
-
-        iterator begin() {return m_aHashMap.begin();}
-        iterator end() {return m_aHashMap.end();}
-
-    private:
-        bool bFirstSearch;
-        HashMap_t m_aHashMap;
-        iterator aLastInsertion;
-        iterator aLastFound;
-        iterator aFirstInOrder;
-};
-
-
-// class MergeData
-
-
-/// Purpose: holds information of data to merge (one resource)
-class MergeData
-{
-    friend class MergeDataHashMap;
-
-public:
-    std::unique_ptr<MergeEntrys> pMergeEntrys;
-private:
-    MergeDataHashMap::iterator m_aNextData;
-public:
-    MergeData();
-    ~MergeData();
-    MergeEntrys* GetMergeEntries() { return pMergeEntrys.get();}
-
-};
-
-
 // class MergeDataFile
 
 
@@ -207,10 +137,10 @@ public:
 class MergeDataFile
 {
     private:
-        MergeDataHashMap aMap;
+        std::unordered_map<OString, std::unique_ptr<MergeEntrys>> aMap;
         std::set<OString> aLanguageSet;
 
-        MergeData *GetMergeData( ResData *pResData , bool bCaseSensitive = false );
+        MergeEntrys *GetMergeData( ResData *pResData , bool bCaseSensitive = false );
         void InsertEntry(const OString &rTYP, const OString &rGID,
             const OString &rLID, const OString &nLang,
             const OString &rTEXT, const OString &rQHTEXT,
diff --git a/l10ntools/source/merge.cxx b/l10ntools/source/merge.cxx
index a86ddf96124e..ea236948b8b8 100644
--- a/l10ntools/source/merge.cxx
+++ b/l10ntools/source/merge.cxx
@@ -107,74 +107,6 @@ OString MergeEntrys::GetQTZText(const ResData& rResData, const OString& rOrigTex
     return sKey + GetDoubleBars() + rOrigText;
 }
 
-
-// class MergeDataHashMap
-
-
-std::pair<MergeDataHashMap::iterator,bool> MergeDataHashMap::insert(const OString& rKey, MergeData* pMergeData)
-{
-    std::pair<iterator,bool> aTemp = m_aHashMap.emplace( rKey, pMergeData );
-    if( m_aHashMap.size() == 1 )
-    {
-        // When first insert, set an iterator to the first element
-        aFirstInOrder = aTemp.first;
-    }
-    else
-    {
-        // Define insertion order by setting an iterator to the next element.
-        aLastInsertion->second->m_aNextData = aTemp.first;
-    }
-    aLastInsertion = aTemp.first;
-    return aTemp;
-}
-
-MergeDataHashMap::iterator const & MergeDataHashMap::find(const OString& rKey)
-{
-    iterator aHint = m_aHashMap.end();
-
-    // Add a hint
-    if( bFirstSearch && !m_aHashMap.empty() )
-    {
-        aHint = aFirstInOrder;
-    }
-    else if( aLastFound == aLastInsertion )
-    {
-        // Next to the last element is the first element
-        aHint = aFirstInOrder;
-    }
-    else if( aLastFound != m_aHashMap.end() && aLastFound != aLastInsertion )
-    {
-        aHint = aLastFound->second->m_aNextData;
-    }
-
-    // If hint works than no need for search
-    if( aHint != m_aHashMap.end() && aHint->first == rKey )
-    {
-        aLastFound = aHint;
-    }
-    else
-    {
-        aLastFound = m_aHashMap.find(rKey);
-    }
-
-    bFirstSearch = false;
-    return aLastFound;
-}
-
-
-// class MergeData
-
-
-MergeData::MergeData()
-    : pMergeEntrys( new MergeEntrys() )
-{
-}
-
-MergeData::~MergeData()
-{
-}
-
-
 // class MergeDataFile
 
 
@@ -296,8 +228,6 @@ MergeDataFile::MergeDataFile(
 
 MergeDataFile::~MergeDataFile()
 {
-    for (MergeDataHashMap::iterator aI = aMap.begin(), aEnd = aMap.end(); aI != aEnd; ++aI)
-        delete aI->second;
 }
 
 std::vector<OString> MergeDataFile::GetLanguages() const
@@ -305,7 +235,7 @@ std::vector<OString> MergeDataFile::GetLanguages() const
     return std::vector<OString>(aLanguageSet.begin(),aLanguageSet.end());
 }
 
-MergeData *MergeDataFile::GetMergeData( ResData *pResData , bool bCaseSensitive )
+MergeEntrys *MergeDataFile::GetMergeData( ResData *pResData , bool bCaseSensitive )
 {
     OString sOldG = pResData->sGId;
     OString sOldL = pResData->sId;
@@ -320,12 +250,12 @@ MergeData *MergeDataFile::GetMergeData( ResData *pResData , bool bCaseSensitive
 
     OString sKey = CreateKey( pResData->sResTyp , pResData->sGId , pResData->sId , pResData->sFilename , bCaseSensitive );
 
-    MergeDataHashMap::const_iterator mit = aMap.find( sKey );
+    auto mit = aMap.find( sKey );
     if(mit != aMap.end())
     {
         pResData->sGId = sOldG;
         pResData->sId = sOldL;
-        return mit->second;
+        return mit->second.get();
     }
     pResData->sGId = sOldG;
     pResData->sId = sOldL;
@@ -335,19 +265,13 @@ MergeData *MergeDataFile::GetMergeData( ResData *pResData , bool bCaseSensitive
 MergeEntrys *MergeDataFile::GetMergeEntrys( ResData *pResData )
 {
     // search for requested MergeEntrys
-    MergeData *pData = GetMergeData( pResData );
-    if ( pData )
-        return pData->GetMergeEntries();
-    return nullptr;
+    return GetMergeData( pResData );
 }
 
 MergeEntrys *MergeDataFile::GetMergeEntrysCaseSensitive( ResData *pResData )
 {
     // search for requested MergeEntrys
-    MergeData *pData = GetMergeData( pResData , true );
-    if ( pData )
-        return pData->GetMergeEntries();
-    return nullptr;
+    return GetMergeData( pResData , true );
 }
 
 void MergeDataFile::InsertEntry(
@@ -357,28 +281,27 @@ void MergeDataFile::InsertEntry(
     const OString &rTITLE, const OString &rInFilename,
     bool bFirstLang, bool bCaseSensitive )
 {
-    MergeData *pData = nullptr;
+    MergeEntrys *pMergeEntrys = nullptr;
 
     // search for MergeData
     OString sKey = CreateKey(rTYP , rGID , rLID , rInFilename , bCaseSensitive);
 
     if( !bFirstLang )
     {
-        MergeDataHashMap::const_iterator mit = aMap.find( sKey );
+        auto mit = aMap.find( sKey );
         if(mit != aMap.end())
-            pData = mit->second;
+            pMergeEntrys = mit->second.get();
 
     }
 
-    if( !pData )
+    if( !pMergeEntrys )
     {
-        pData = new MergeData;
-        aMap.insert( sKey, pData );
+        pMergeEntrys = new MergeEntrys;
+        aMap.emplace( sKey, std::unique_ptr<MergeEntrys>(pMergeEntrys) );
     }
 
 
     // insert the cur string
-    MergeEntrys *pMergeEntrys = pData->GetMergeEntries();
     if( nLANG =="qtz" )
     {
         const OString sTemp = rInFilename + rGID + rLID + rTYP;


More information about the Libreoffice-commits mailing list