[Libreoffice-commits] core.git: sc/inc sc/source

[Stephan Bergmann (via logerrit)]

 sc/inc/scopetools.hxx            |    6 ++++++
 sc/source/core/data/document.cxx |   25 ++++++++++++++++++++-----
 sc/source/core/inc/bcaslot.hxx   |    6 ++++++
 3 files changed, 32 insertions(+), 5 deletions(-)

New commits:
commit 3e59716375a240576fd6d8759b32b4319506ed70
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Thu Dec 12 16:42:42 2019 +0100
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Thu Dec 12 18:17:26 2019 +0100

    Prevent BroadcastRecalcOnRefMoveHandler copies
    
    ...which would contain copies of AutoCalcSwitch and ScBulkBroadcast that both do
    things in their dtors.  std::sort (into which BroadcastRecalcOnRefMoveHandler is
    passed as function object argument) returns its function object argument, which
    caused the BroadcastRecalcOnRefMoveHandler copies.
    
    Better split the RAII part that holds AutoCalcSwitch and ScBulkBroadcast and
    does things in their dtors into a BroadcastRecalcOnRefMoveGuard class of its
    own.  (And while at it, explicitly delete the copy/move functions of
    AutoCalcSwitch and ScBulkBroadcast to avoid such issues in the future.)
    
    (The RAII part of BroadcastRecalcOnRefMoveHandler had been added with
    60d0b992ea3a910be79ae4a8e8b0bb32a358b18a "sc-perf: tdf#87101 add bulk scope for
    BroadcastRecalcOnRefMove() calls".  The issue was found with Clang 10 trunk
    -Wdeprecated-copy-dtor.)
    
    Change-Id: I45d6b81e7c8b34aed57b6f66c5e1e966a43a565d
    Reviewed-on: https://gerrit.libreoffice.org/85063
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/sc/inc/scopetools.hxx b/sc/inc/scopetools.hxx
index 571b1fce52f5..b71154330e96 100644
--- a/sc/inc/scopetools.hxx
+++ b/sc/inc/scopetools.hxx
@@ -24,6 +24,12 @@ class SC_DLLPUBLIC AutoCalcSwitch
 {
     ScDocument& mrDoc;
     bool const mbOldValue;
+
+    AutoCalcSwitch(AutoCalcSwitch const &) = delete;
+    AutoCalcSwitch(AutoCalcSwitch &&) = delete;
+    AutoCalcSwitch & operator =(AutoCalcSwitch const &) = delete;
+    AutoCalcSwitch & operator =(AutoCalcSwitch &&) = delete;
+
 public:
     AutoCalcSwitch(ScDocument& rDoc, bool bAutoCalc);
     ~AutoCalcSwitch();
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index 3c34473359f2..fa1913a0c11c 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -1226,8 +1226,11 @@ struct BroadcastRecalcOnRefMoveHandler
         if (p)
             p->BroadcastRecalcOnRefMove();
     }
+};
 
-    explicit BroadcastRecalcOnRefMoveHandler( ScDocument* pDoc ) :
+struct BroadcastRecalcOnRefMoveGuard
+{
+    explicit BroadcastRecalcOnRefMoveGuard( ScDocument* pDoc ) :
         aSwitch( *pDoc, false),
         aBulk( pDoc->GetBASM(), SfxHintId::ScDataChanged)
     {
@@ -1339,7 +1342,10 @@ bool ScDocument::InsertRow( SCCOL nStartCol, SCTAB nStartTab,
                     a->SetDirtyIfPostponed();
             }
 
-            std::for_each(maTabs.begin(), maTabs.end(), BroadcastRecalcOnRefMoveHandler( this));
+            {
+                BroadcastRecalcOnRefMoveGuard g(this);
+                std::for_each(maTabs.begin(), maTabs.end(), BroadcastRecalcOnRefMoveHandler());
+            }
         }
         bRet = true;
     }
@@ -1450,7 +1456,10 @@ void ScDocument::DeleteRow( SCCOL nStartCol, SCTAB nStartTab,
                 a->SetDirtyIfPostponed();
         }
 
-        std::for_each(maTabs.begin(), maTabs.end(), BroadcastRecalcOnRefMoveHandler( this));
+        {
+            BroadcastRecalcOnRefMoveGuard g(this);
+            std::for_each(maTabs.begin(), maTabs.end(), BroadcastRecalcOnRefMoveHandler());
+        }
     }
 
     if (pChartListenerCollection)
@@ -1552,7 +1561,10 @@ bool ScDocument::InsertCol( SCROW nStartRow, SCTAB nStartTab,
             std::for_each(maTabs.begin(), maTabs.end(), SetDirtyIfPostponedHandler());
             // Cells containing functions such as CELL, COLUMN or ROW may have
             // changed their values on relocation. Broadcast them.
-            std::for_each(maTabs.begin(), maTabs.end(), BroadcastRecalcOnRefMoveHandler( this));
+            {
+                BroadcastRecalcOnRefMoveGuard g(this);
+                std::for_each(maTabs.begin(), maTabs.end(), BroadcastRecalcOnRefMoveHandler());
+            }
         }
         bRet = true;
     }
@@ -1653,7 +1665,10 @@ void ScDocument::DeleteCol(SCROW nStartRow, SCTAB nStartTab, SCROW nEndRow, SCTA
                 a->SetDirtyIfPostponed();
         }
 
-        std::for_each(maTabs.begin(), maTabs.end(), BroadcastRecalcOnRefMoveHandler( this));
+        {
+            BroadcastRecalcOnRefMoveGuard g(this);
+            std::for_each(maTabs.begin(), maTabs.end(), BroadcastRecalcOnRefMoveHandler());
+        }
     }
 
     if (pChartListenerCollection)
diff --git a/sc/source/core/inc/bcaslot.hxx b/sc/source/core/inc/bcaslot.hxx
index 20322d5f4fc2..11c0942df9a2 100644
--- a/sc/source/core/inc/bcaslot.hxx
+++ b/sc/source/core/inc/bcaslot.hxx
@@ -351,6 +351,12 @@ class ScBulkBroadcast
 {
     ScBroadcastAreaSlotMachine* pBASM;
     SfxHintId const             mnHintId;
+
+    ScBulkBroadcast(ScBulkBroadcast const &) = delete;
+    ScBulkBroadcast(ScBulkBroadcast &&) = delete;
+    ScBulkBroadcast & operator =(ScBulkBroadcast const &) = delete;
+    ScBulkBroadcast & operator =(ScBulkBroadcast &&) = delete;
+
 public:
     explicit ScBulkBroadcast( ScBroadcastAreaSlotMachine* p, SfxHintId nHintId ) :
         pBASM(p),