sc/source/filter/ftools/sharedformulagroups.cxx | 15 +++++++++++++-- sc/source/filter/inc/sharedformulagroups.hxx | 22 +++++++++++++++++++++- sc/source/filter/oox/formulabuffer.cxx | 23 +++++++++++++++++++---- 3 files changed, 53 insertions(+), 7 deletions(-)
New commits: commit b30251ca0d102ced36799ee18d4bbcd9e8530fa0 Author: Dennis Francis <dennis.fran...@collabora.com> AuthorDate: Wed Nov 20 13:24:48 2019 +0530 Commit: Eike Rathke <er...@redhat.com> CommitDate: Fri Nov 22 12:58:57 2019 +0100 tdf#128894: xlsx-import : Do not share tokens between cells... which are part of a xlsx-shared-formula along a *row*. If we do, any reference-updates on these cells while editing will mess things up. For example a shared formula "=A30+1" used for a few cells in the first row (say, A1, B1, C1 and D1) and on deleting a row, say row#5, the reference update operation will decrement the row index of all tokens in A1, B1, C1 and D1. But if they share tokens, they all end up pointing to row#26 instead of row#29 as each cell is updated which amounts to decrementing 4 times instead of once. However shared formulas along columns are not affected by this bug, when tokens are shared since we use formula-groups which only keeps one copy of token array for the entire group and reference-update code is designed to correctly work with formula-groups. Change-Id: Ic0fe84d12fef18fbf21658664e2b2b86409bca27 Reviewed-on: https://gerrit.libreoffice.org/83361 Tested-by: Jenkins Reviewed-by: Luboš Luňák <l.lu...@collabora.com> Reviewed-by: Eike Rathke <er...@redhat.com> diff --git a/sc/source/filter/ftools/sharedformulagroups.cxx b/sc/source/filter/ftools/sharedformulagroups.cxx index 6844e9bf3181..9b028d9eb307 100644 --- a/sc/source/filter/ftools/sharedformulagroups.cxx +++ b/sc/source/filter/ftools/sharedformulagroups.cxx @@ -15,13 +15,24 @@ namespace sc { void SharedFormulaGroups::set( size_t nSharedId, std::unique_ptr<ScTokenArray> pArray ) { - m_Store.insert(std::make_pair(nSharedId, std::move(pArray))); + m_Store.try_emplace(nSharedId, std::move(pArray), ScAddress(ScAddress::INITIALIZE_INVALID)); +} + +void SharedFormulaGroups::set( size_t nSharedId, std::unique_ptr<ScTokenArray> pArray, const ScAddress& rOrigin ) +{ + m_Store.try_emplace(nSharedId, std::move(pArray), rOrigin); } const ScTokenArray* SharedFormulaGroups::get( size_t nSharedId ) const { StoreType::const_iterator const it = m_Store.find(nSharedId); - return it == m_Store.end() ? nullptr : it->second.get(); + return it == m_Store.end() ? nullptr : it->second.getTokenArray(); +} + +const SharedFormulaGroupEntry* SharedFormulaGroups::getEntry( size_t nSharedId ) const +{ + StoreType::const_iterator const it = m_Store.find(nSharedId); + return it == m_Store.end() ? nullptr : &(it->second); } } diff --git a/sc/source/filter/inc/sharedformulagroups.hxx b/sc/source/filter/inc/sharedformulagroups.hxx index 745b5b14bdfa..540d1e901ef6 100644 --- a/sc/source/filter/inc/sharedformulagroups.hxx +++ b/sc/source/filter/inc/sharedformulagroups.hxx @@ -10,21 +10,41 @@ #ifndef INCLUDED_SC_SOURCE_FILTER_INC_SHAREDFORMULAGROUPS_HXX #define INCLUDED_SC_SOURCE_FILTER_INC_SHAREDFORMULAGROUPS_HXX +#include <address.hxx> #include <memory> #include <map> class ScTokenArray; namespace sc { +class SharedFormulaGroupEntry +{ +private: + std::unique_ptr<ScTokenArray> mpArray; + ScAddress maOrigin; + +public: + SharedFormulaGroupEntry(std::unique_ptr<ScTokenArray> pArray, const ScAddress& rOrigin) + : mpArray(std::move(pArray)) + , maOrigin(rOrigin) + { + } + + const ScTokenArray* getTokenArray() const { return mpArray.get(); } + const ScAddress& getOrigin() const { return maOrigin; } +}; + class SharedFormulaGroups { private: - typedef std::map<size_t, std::unique_ptr<ScTokenArray>> StoreType; + typedef std::map<size_t, SharedFormulaGroupEntry> StoreType; StoreType m_Store; public: void set( size_t nSharedId, std::unique_ptr<ScTokenArray> pArray ); + void set( size_t nSharedId, std::unique_ptr<ScTokenArray> pArray, const ScAddress& rOrigin ); const ScTokenArray* get( size_t nSharedId ) const; + const SharedFormulaGroupEntry* getEntry( size_t nSharedId ) const; }; } diff --git a/sc/source/filter/oox/formulabuffer.cxx b/sc/source/filter/oox/formulabuffer.cxx index ffc60099eba8..e8f9f5c6a2a6 100644 --- a/sc/source/filter/oox/formulabuffer.cxx +++ b/sc/source/filter/oox/formulabuffer.cxx @@ -121,7 +121,7 @@ void applySharedFormulas( if (pArray) { aComp.CompileTokenArray(); // Generate RPN tokens. - aGroups.set(nId, std::move(pArray)); + aGroups.set(nId, std::move(pArray), aPos); } } } @@ -132,11 +132,26 @@ void applySharedFormulas( for (const FormulaBuffer::SharedFormulaDesc& rDesc : rCells) { const ScAddress& aPos = rDesc.maAddress; - const ScTokenArray* pArray = aGroups.get(rDesc.mnSharedId); - if (!pArray) + const sc::SharedFormulaGroupEntry* pEntry = aGroups.getEntry(rDesc.mnSharedId); + if (!pEntry) continue; - ScFormulaCell* pCell = new ScFormulaCell(&rDoc.getDoc(), aPos, *pArray); + const ScTokenArray* pArray = pEntry->getTokenArray(); + assert(pArray); + const ScAddress& rOrigin = pEntry->getOrigin(); + assert(rOrigin.IsValid()); + + ScFormulaCell* pCell; + // In case of shared-formula along a row, do not let + // these cells share the same token objects. + // If we do, any reference-updates on these cells + // (while editing) will mess things up. Pass the cloned array as a + // pointer and not as reference to avoid any further allocation. + if (rOrigin.Col() != aPos.Col()) + pCell = new ScFormulaCell(&rDoc.getDoc(), aPos, pArray->Clone()); + else + pCell = new ScFormulaCell(&rDoc.getDoc(), aPos, *pArray); + rDoc.setFormulaCell(aPos, pCell); if (rDesc.maCellValue.isEmpty()) { _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits