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.lunak@collabora.com>
Reviewed-by: Eike Rathke <erack@redhat.com>

3 files changed, 53 insertions, 7 deletions

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())
             {