[PATCH] Change in core[libreoffice-4-0]: fdo#58539: Correctly set cached matrix formula result.

Kohei Yoshida (via Code Review) gerrit at gerrit.libreoffice.org
Wed Jan 9 10:07:53 PST 2013


Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/1623

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/23/1623/1

fdo#58539: Correctly set cached matrix formula result.

During the import, a cached matrix value only has an empty matrix
of correct geometry, plus the token type of the top-left cell.  The
rest of the elements are imported as hybrid values.  For now, this
seems to do the trick.

In the future we may want to change it to fully populate the matrix
cache value during the import, and skip setting the hybrid values for
the non-top-left elements.

This commit also make several other trivial changes:

* Mark pRawToken mutable so that we can mark those IsFoo() methods
  const.

* Move the ScCompiler instance from static instance to member of
  ScXMLImport. Since we don't need the instance to persist once the
  import is over, this is more appropriate.

Change-Id: I1abde03c0fcd91b02ef4dbf8b5526f7965eaf19c
---
M formula/inc/formula/FormulaCompiler.hxx
M formula/source/core/api/FormulaCompiler.cxx
M sc/inc/cell.hxx
M sc/inc/compiler.hxx
M sc/inc/formularesult.hxx
M sc/inc/token.hxx
M sc/source/core/tool/compiler.cxx
M sc/source/core/tool/formularesult.cxx
M sc/source/core/tool/interpr1.cxx
M sc/source/filter/xml/xmlcelli.cxx
M sc/source/filter/xml/xmlimprt.cxx
M sc/source/filter/xml/xmlimprt.hxx
12 files changed, 70 insertions(+), 14 deletions(-)



diff --git a/formula/inc/formula/FormulaCompiler.hxx b/formula/inc/formula/FormulaCompiler.hxx
index ce4157d..85295b2 100644
--- a/formula/inc/formula/FormulaCompiler.hxx
+++ b/formula/inc/formula/FormulaCompiler.hxx
@@ -212,6 +212,8 @@
      */
     OpCode GetEnglishOpCode( const String& rName ) const;
 
+    sal_uInt16 GetErrorConstant( const String& rName ) const;
+
     void            SetCompileForFAP( bool bVal )
                         { bCompileForFAP = bVal; bIgnoreErrors = bVal; }
 
@@ -265,7 +267,6 @@
     virtual void CreateStringFromIndex(rtl::OUStringBuffer& rBuffer,FormulaToken* pTokenP);
     virtual void LocalizeString( String& rName );   // modify rName - input: exact name
 
-    sal_uInt16 GetErrorConstant( const String& rName );
     void AppendErrorConstant( rtl::OUStringBuffer& rBuffer, sal_uInt16 nError );
 
     bool   GetToken();
diff --git a/formula/source/core/api/FormulaCompiler.cxx b/formula/source/core/api/FormulaCompiler.cxx
index 29b6694..f79201b 100644
--- a/formula/source/core/api/FormulaCompiler.cxx
+++ b/formula/source/core/api/FormulaCompiler.cxx
@@ -816,7 +816,7 @@
 }
 // -----------------------------------------------------------------------------
 
-sal_uInt16 FormulaCompiler::GetErrorConstant( const String& rName )
+sal_uInt16 FormulaCompiler::GetErrorConstant( const String& rName ) const
 {
     sal_uInt16 nError = 0;
     OpCodeHashMap::const_iterator iLook( mxSymbols->getHashMap()->find( rName));
diff --git a/sc/inc/cell.hxx b/sc/inc/cell.hxx
index 521a28f..008b510 100644
--- a/sc/inc/cell.hxx
+++ b/sc/inc/cell.hxx
@@ -492,6 +492,11 @@
                                     const formula::FormulaGrammar::Grammar eGrammar )
                         { aResult.SetHybridFormula( r); eTempGrammar = eGrammar; }
 
+    void SetResultMatrix( SCCOL nCols, SCROW nRows, const ScConstMatrixRef& pMat, formula::FormulaToken* pUL )
+    {
+        aResult.SetMatrix(nCols, nRows, pMat, pUL);
+    }
+
     /** For import only: set a double result.
         Use this instead of SetHybridDouble() if there is no (temporary)
         formula string because the formula is present as a token array, as it
diff --git a/sc/inc/compiler.hxx b/sc/inc/compiler.hxx
index 4aa393d..d40d261 100644
--- a/sc/inc/compiler.hxx
+++ b/sc/inc/compiler.hxx
@@ -320,7 +320,7 @@
     sal_Unicode cSymbol[MAXSTRLEN];                 // current Symbol
     String      aFormula;                           // formula source code
     xub_StrLen  nSrcPos;                            // tokenizer position (source code)
-    ScRawTokenRef   pRawToken;
+    mutable ScRawTokenRef pRawToken;
 
     const CharClass*    pCharClass;         // which character classification is used for parseAnyToken
     sal_uInt16      mnPredetectedReference;     // reference when reading ODF, 0 (none), 1 (single) or 2 (double)
@@ -386,7 +386,7 @@
 
     // Check if it is a valid english function name
     bool IsEnglishSymbol( const String& rName );
-    bool IsErrorConstant( const String& );
+    bool IsErrorConstant( const String& ) const;
 
     //! _either_ CompileForFAP _or_ AutoCorrection, _not_ both
     // #i101512# SetCompileForFAP is in formula::FormulaCompiler
diff --git a/sc/inc/formularesult.hxx b/sc/inc/formularesult.hxx
index c4f281f..2128636 100644
--- a/sc/inc/formularesult.hxx
+++ b/sc/inc/formularesult.hxx
@@ -179,6 +179,8 @@
         SetHybridFormula() for formula string to be compiled later. */
     SC_DLLPUBLIC void SetHybridFormula( const String & rFormula );
 
+    SC_DLLPUBLIC void SetMatrix( SCCOL nCols, SCROW nRows, const ScConstMatrixRef& pMat, formula::FormulaToken* pUL );
+
     /** Get the const ScMatrixFormulaCellToken* if token is of that type, else
         NULL. */
     const ScMatrixFormulaCellToken* GetMatrixFormulaCellToken() const;
diff --git a/sc/inc/token.hxx b/sc/inc/token.hxx
index c8c8782..e08db6c 100644
--- a/sc/inc/token.hxx
+++ b/sc/inc/token.hxx
@@ -350,6 +350,10 @@
             SCROW               nRows;
             SCCOL               nCols;
 public:
+    ScMatrixFormulaCellToken( SCCOL nC, SCROW nR, const ScConstMatrixRef& pMat, formula::FormulaToken* pUL ) :
+        ScMatrixCellResultToken(pMat, pUL),
+        nRows(nR), nCols(nC) {}
+
                                 ScMatrixFormulaCellToken( SCCOL nC, SCROW nR ) :
                                     ScMatrixCellResultToken( NULL, NULL ),
                                     nRows( nR ), nCols( nC ) {}
diff --git a/sc/source/core/tool/compiler.cxx b/sc/source/core/tool/compiler.cxx
index 6be204b..b827019 100644
--- a/sc/source/core/tool/compiler.cxx
+++ b/sc/source/core/tool/compiler.cxx
@@ -3306,7 +3306,7 @@
 }
 
 
-bool ScCompiler::IsErrorConstant( const String& rName )
+bool ScCompiler::IsErrorConstant( const String& rName ) const
 {
     sal_uInt16 nError = GetErrorConstant( rName);
     if (nError)
diff --git a/sc/source/core/tool/formularesult.cxx b/sc/source/core/tool/formularesult.cxx
index 3b94481..9e1032e 100644
--- a/sc/source/core/tool/formularesult.cxx
+++ b/sc/source/core/tool/formularesult.cxx
@@ -443,6 +443,16 @@
     mbToken = true;
 }
 
+void ScFormulaResult::SetMatrix( SCCOL nCols, SCROW nRows, const ScConstMatrixRef& pMat, formula::FormulaToken* pUL )
+{
+    ResetToDefaults();
+    if (mbToken && mpToken)
+        mpToken->DecRef();
+    mpToken = new ScMatrixFormulaCellToken(nCols, nRows, pMat, pUL);
+    mpToken->IncRef();
+    mbToken = true;
+}
+
 const ScMatrixFormulaCellToken* ScFormulaResult::GetMatrixFormulaCellToken() const
 {
     return (GetType() == formula::svMatrixCell ?
diff --git a/sc/source/core/tool/interpr1.cxx b/sc/source/core/tool/interpr1.cxx
index 0b86108..ec85a01 100644
--- a/sc/source/core/tool/interpr1.cxx
+++ b/sc/source/core/tool/interpr1.cxx
@@ -135,7 +135,10 @@
                 SCSIZE nCols, nRows;
                 pMat->GetDimensions( nCols, nRows );
                 if ( nCols == 0 || nRows == 0 )
+                {
                     PushIllegalArgument();
+                    return;
+                }
                 else if (pTokenMatrixMap && ((aMapIter = pTokenMatrixMap->find(
                                     pCur)) != pTokenMatrixMap->end()))
                     xNew = (*aMapIter).second;
diff --git a/sc/source/filter/xml/xmlcelli.cxx b/sc/source/filter/xml/xmlcelli.cxx
index b64ab03..bdf5748 100644
--- a/sc/source/filter/xml/xmlcelli.cxx
+++ b/sc/source/filter/xml/xmlcelli.cxx
@@ -47,8 +47,6 @@
 #include "scerrors.hxx"
 #include "editutil.hxx"
 #include "cell.hxx"
-#include "compiler.hxx"
-
 
 #include <xmloff/xmltkmap.hxx>
 #include <xmloff/xmltoken.hxx>
@@ -727,9 +725,7 @@
     {
         if( bFormulaTextResult && pOUTextValue )
         {
-            static ScCompiler aComp(NULL, ScAddress());
-            aComp.SetGrammar(formula::FormulaGrammar::GRAM_ODFF);
-            if(!aComp.IsErrorConstant(*pOUTextValue))
+            if (!GetScImport().IsFormulaErrorConstant(*pOUTextValue))
             {
                 pFCell->SetHybridString( *pOUTextValue );
                 pFCell->ResetDirty();
@@ -1086,11 +1082,29 @@
                         std::min<SCROW>(rCellPos.Row() + nMatrixRows - 1, MAXROW),
                         pOUFormula->first, pOUFormula->second, eGrammar);
 
-                //set the value/text of the first matrix position (top-left).
-                //the value/text of the matrix reference cells will be set later.
+                // Set the value/text of the top-left matrix position in its
+                // cached result.  For import, we only need to set the correct
+                // matrix geometry and the value type of the top-left element.
+
                 ScFormulaCell* pFCell =
                     static_cast<ScFormulaCell*>( rXMLImport.GetDocument()->GetCell(rCellPos) );
-                SetFormulaCell(pFCell);
+
+                ScMatrixRef pMat(new ScMatrix(nMatrixCols, nMatrixRows));
+                if (bFormulaTextResult && pOUTextValue)
+                {
+                    if (!GetScImport().IsFormulaErrorConstant(*pOUTextValue))
+                    {
+                        pFCell->SetResultMatrix(
+                            nMatrixCols, nMatrixRows, pMat, new formula::FormulaStringToken(*pOUTextValue));
+                        pFCell->ResetDirty();
+                    }
+                }
+                else if (!rtl::math::isNan(fValue))
+                {
+                    pFCell->SetResultMatrix(
+                        nMatrixCols, nMatrixRows, pMat, new formula::FormulaDoubleToken(fValue));
+                    pFCell->ResetDirty();
+                }
             }
         }
         else
diff --git a/sc/source/filter/xml/xmlimprt.cxx b/sc/source/filter/xml/xmlimprt.cxx
index 7aae79c..b7f229c 100644
--- a/sc/source/filter/xml/xmlimprt.cxx
+++ b/sc/source/filter/xml/xmlimprt.cxx
@@ -2809,6 +2809,9 @@
     if (!pDoc)
         throw lang::IllegalArgumentException();
 
+    mpComp.reset(new ScCompiler(pDoc, ScAddress()));
+    mpComp->SetGrammar(formula::FormulaGrammar::GRAM_ODFF);
+
     bFromWrapper = pDoc->IsXMLFromWrapper();    // UnlockSolarMutex below still works normally
 
     uno::Reference<document::XActionLockable> xActionLockable(xDoc, uno::UNO_QUERY);
@@ -3294,4 +3297,12 @@
     reGrammar = eDefaultGrammar;
 }
 
+bool ScXMLImport::IsFormulaErrorConstant( const OUString& rStr ) const
+{
+    if (!mpComp)
+        return false;
+
+    return mpComp->GetErrorConstant(rStr) > 0;
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/filter/xml/xmlimprt.hxx b/sc/source/filter/xml/xmlimprt.hxx
index c202160..87f3f3e 100644
--- a/sc/source/filter/xml/xmlimprt.hxx
+++ b/sc/source/filter/xml/xmlimprt.hxx
@@ -31,6 +31,7 @@
 #include "xmlsubti.hxx"
 #include "global.hxx"
 #include "formula/grammar.hxx"
+#include "compiler.hxx"
 
 #include "xmlstyle.hxx"
 #include "XMLDetectiveContext.hxx"
@@ -46,6 +47,8 @@
 #include <boost/unordered_map.hpp>
 #include <boost/ptr_container/ptr_list.hpp>
 #include <boost/ptr_container/ptr_map.hpp>
+#include <boost/noncopyable.hpp>
+#include <boost/scoped_ptr.hpp>
 
 class ScMyStyleNumberFormats;
 class XMLNumberFormatAttributesExportHelper;
@@ -732,7 +735,7 @@
 typedef std::list<SvXMLImportContext*>              ScMyViewContextList;
 class ScMyStylesImportHelper;
 
-class ScXMLImport: public SvXMLImport
+class ScXMLImport: public SvXMLImport, boost::noncopyable
 {
     typedef ::boost::unordered_map< ::rtl::OUString, sal_Int16, ::rtl::OUStringHash >   CellTypeMap;
     typedef ::boost::ptr_map<SCTAB, ScMyNamedExpressions> SheetNamedExpMap;
@@ -740,6 +743,7 @@
     CellTypeMap             aCellTypeMap;
 
     ScDocument*             pDoc;
+    boost::scoped_ptr<ScCompiler> mpComp; // For error-checking of cached string cell values.
     ScXMLChangeTrackingImportHelper*    pChangeTrackingImportHelper;
     ScMyViewContextList                 aViewContextList;
     ScMyStylesImportHelper*             pStylesImportHelper;
@@ -1141,6 +1145,8 @@
             ::formula::FormulaGrammar::Grammar& reGrammar,
             const ::rtl::OUString& rAttrValue,
             bool bRestrictToExternalNmsp = false ) const;
+
+    bool IsFormulaErrorConstant( const OUString& rStr ) const;
 };
 
 #endif

-- 
To view, visit https://gerrit.libreoffice.org/1623
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1abde03c0fcd91b02ef4dbf8b5526f7965eaf19c
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: libreoffice-4-0
Gerrit-Owner: Kohei Yoshida <kohei.yoshida at gmail.com>



More information about the LibreOffice mailing list