sc/inc/compiler.hxx | 2
sc/inc/rangenam.hxx | 30 +++++++++---
sc/source/core/tool/compiler.cxx | 36 ++++++++++----
sc/source/core/tool/rangenam.cxx | 75 +++++++++++++++----------------
sc/source/ui/docshell/externalrefmgr.cxx | 6 +-
5 files changed, 90 insertions(+), 59 deletions(-)
New commits:
commit 4f2ec0ab9c1055920cb5c08b2d237087493130b8
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Tue Mar 8 17:53:27 2022 +0100
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Tue Mar 8 21:18:00 2022 +0100
optimize checking for conflicting named ranges
This extends 582fc887f1faafe8ff5f16a13a0208483a93353f, first
check if there is any named range that could possibly conflict (which
generally should be rare).
Change-Id: Ia5e9e56cab29b459bcb489e871b4960ba215b665
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/131219
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/sc/inc/compiler.hxx b/sc/inc/compiler.hxx
index 15b3823f649d..5f3b6354c967 100644
--- a/sc/inc/compiler.hxx
+++ b/sc/inc/compiler.hxx
@@ -375,6 +375,8 @@ private:
*/
ScRangeData* GetRangeData( const formula::FormulaToken& pToken ) const;
+ bool HasPossibleNamedRangeConflict(SCTAB nTab) const;
+
static const CharClass* GetCharClassEnglish();
static const CharClass* GetCharClassLocalized();
diff --git a/sc/inc/rangenam.hxx b/sc/inc/rangenam.hxx
index d51aa85a1008..31bdd1f5a23c 100644
--- a/sc/inc/rangenam.hxx
+++ b/sc/inc/rangenam.hxx
@@ -156,6 +156,8 @@ public:
SC_DLLPUBLIC static IsNameValidType IsNameValid( const OUString&
rName, const ScDocument& rDoc );
+ bool HasPossibleAddressConflict() const;
+
void CompileUnresolvedXML( sc::CompileFormulaContext& rCxt );
#if DEBUG_FORMULA_COMPILER
@@ -187,6 +189,12 @@ private:
typedef ::std::map<OUString, std::unique_ptr<ScRangeData>> DataType;
DataType m_Data;
IndexDataType maIndexToData;
+ // Use for optimization, true if any of the contained names resolves
+ // as a valid cell address (e.g. 'day1' with 16k columns).
+ mutable bool mHasPossibleAddressConflict : 1;
+ mutable bool mHasPossibleAddressConflictDirty : 1;
+
+ void checkHasPossibleAddressConflict() const;
public:
/// Map that stores non-managed pointers to ScRangeName instances.
@@ -266,6 +274,13 @@ public:
void clear();
bool operator== (const ScRangeName& r) const;
+
+ bool hasPossibleAddressConflict() const
+ {
+ if( mHasPossibleAddressConflictDirty )
+ checkHasPossibleAddressConflict();
+ return mHasPossibleAddressConflict;
+ }
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/tool/compiler.cxx b/sc/source/core/tool/compiler.cxx
index fca54da17f9c..151ccefd6134 100644
--- a/sc/source/core/tool/compiler.cxx
+++ b/sc/source/core/tool/compiler.cxx
@@ -3344,17 +3344,20 @@ bool ScCompiler::ParseSingleReference( const OUString&
rName, const OUString* pE
return false;
}
- // A named range named e.g. 'num1' is valid with 1k columns, but would
become a reference
- // when the document is opened later with 16k columns. Resolve the
conflict by not
- // considering it a reference.
- OUString aUpper;
- bool bAsciiUpper = ToUpperAsciiOrI18nIsAscii( aUpper, rName );
- if (bAsciiUpper || mbCharClassesDiffer)
- aUpper = ScGlobal::getCharClass().uppercase( rName );
- mnCurrentSheetTab = aAddr.Tab(); // temporarily set for
ParseNamedRange()
- if(ParseNamedRange( aUpper, true )) // only check
- return false;
- mnCurrentSheetTab = -1;
+ if( HasPossibleNamedRangeConflict( aAddr.Tab()))
+ {
+ // A named range named e.g. 'num1' is valid with 1k columns, but
would become a reference
+ // when the document is opened later with 16k columns. Resolve the
conflict by not
+ // considering it a reference.
+ OUString aUpper;
+ bool bAsciiUpper = ToUpperAsciiOrI18nIsAscii( aUpper, rName );
+ if (bAsciiUpper || mbCharClassesDiffer)
+ aUpper = ScGlobal::getCharClass().uppercase( rName );
+ mnCurrentSheetTab = aAddr.Tab(); // temporarily set for
ParseNamedRange()
+ if(ParseNamedRange( aUpper, true )) // only check
+ return false;
+ mnCurrentSheetTab = -1;
+ }
ScSingleRefData aRef;
aRef.InitAddress( aAddr );
@@ -3586,6 +3589,17 @@ const ScRangeData* ScCompiler::GetRangeData( SCTAB&
rSheet, const OUString& rUpp
return pData;
}
+bool ScCompiler::HasPossibleNamedRangeConflict( SCTAB nTab ) const
+{
+ const ScRangeName* pRangeName = rDoc.GetRangeName();
+ if (pRangeName && pRangeName->hasPossibleAddressConflict())
+ return true;
+ pRangeName = rDoc.GetRangeName(nTab);
+ if (pRangeName && pRangeName->hasPossibleAddressConflict())
+ return true;
+ return false;
+}
+
bool ScCompiler::ParseNamedRange( const OUString& rUpperName, bool onlyCheck )
{
// ParseNamedRange is called only from NextNewToken, with an upper-case
string
diff --git a/sc/source/core/tool/rangenam.cxx b/sc/source/core/tool/rangenam.cxx
index 09bef58a1bd4..89d3f9e75055 100644
--- a/sc/source/core/tool/rangenam.cxx
+++ b/sc/source/core/tool/rangenam.cxx
@@ -497,6 +497,21 @@ ScRangeData::IsNameValidType ScRangeData::IsNameValid(
const OUString& rName, co
return IsNameValidType::NAME_VALID;
}
+bool ScRangeData::HasPossibleAddressConflict() const
+{
+ // Similar to part of IsNameValid(), but only check if the name is a valid
address.
+ ScAddress aAddr;
+ for (int nConv = FormulaGrammar::CONV_UNSPECIFIED; ++nConv <
FormulaGrammar::CONV_LAST; )
+ {
+ ScAddress::Details details(
static_cast<FormulaGrammar::AddressConvention>( nConv ) );
+ // Don't check Parse on VALID, any partial only VALID may result in
+ // #REF! during compile later!
+ if(aAddr.Parse(aUpperName, rDoc, details) != ScRefFlags::ZERO)
+ return true;
+ }
+ return false;
+}
+
FormulaError ScRangeData::GetErrCode() const
{
return pCode ? pCode->GetCodeError() : FormulaError::NONE;
@@ -654,6 +669,8 @@ public:
ScRangeName::ScRangeName() {}
ScRangeName::ScRangeName(const ScRangeName& r)
+ : mHasPossibleAddressConflict( r.mHasPossibleAddressConflict )
+ , mHasPossibleAddressConflictDirty( r.mHasPossibleAddressConflictDirty )
{
for (auto const& it : r.m_Data)
{
@@ -817,6 +834,7 @@ bool ScRangeName::insert( ScRangeData* p, bool
bReuseFreeIndex )
if (nPos >= maIndexToData.size())
maIndexToData.resize(nPos+1, nullptr);
maIndexToData[nPos] = p;
+ mHasPossibleAddressConflictDirty = true;
}
return r.second;
}
@@ -840,12 +858,30 @@ void ScRangeName::erase(const_iterator itr)
OSL_ENSURE( 0 < nIndex && nIndex <= maIndexToData.size(),
"ScRangeName::erase: bad index");
if (0 < nIndex && nIndex <= maIndexToData.size())
maIndexToData[nIndex-1] = nullptr;
+ if(mHasPossibleAddressConflict)
+ mHasPossibleAddressConflictDirty = true;
}
void ScRangeName::clear()
{
m_Data.clear();
maIndexToData.clear();
+ mHasPossibleAddressConflict = false;
+ mHasPossibleAddressConflictDirty = false;
+}
+
+void ScRangeName::checkHasPossibleAddressConflict() const
+{
+ mHasPossibleAddressConflict = false;
+ mHasPossibleAddressConflictDirty = false;
+ for (auto const& itr : m_Data)
+ {
+ if( itr.second->HasPossibleAddressConflict())
+ {
+ mHasPossibleAddressConflict = true;
+ return;
+ }
+ }
}
bool ScRangeName::operator== (const ScRangeName& r) const
commit 371e82c2d90c21bb9df6e4c110a229d29e24c4d3
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Tue Mar 8 17:08:55 2022 +0100
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Tue Mar 8 21:17:46 2022 +0100
don't provide non-const iterators to ScRangeName internal data
Also make few trivial functions inline.
Change-Id: I89b11b2aa4558696d624c9bde9135b1324b8eb36
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/131218
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/sc/inc/rangenam.hxx b/sc/inc/rangenam.hxx
index f7217d567e87..d51aa85a1008 100644
--- a/sc/inc/rangenam.hxx
+++ b/sc/inc/rangenam.hxx
@@ -234,13 +234,11 @@ public:
void CopyUsedNames( const SCTAB nLocalTab, const SCTAB nOldTab, const
SCTAB nNewTab,
const ScDocument& rOldDoc, ScDocument& rNewDoc, const bool
bGlobalNamesToLocal ) const;
- SC_DLLPUBLIC const_iterator begin() const;
- SC_DLLPUBLIC const_iterator end() const;
- SC_DLLPUBLIC iterator begin();
- SC_DLLPUBLIC iterator end();
- SC_DLLPUBLIC size_t size() const;
- SC_DLLPUBLIC size_t index_size() const;
- bool empty() const;
+ SC_DLLPUBLIC const_iterator begin() const { return m_Data.begin(); }
+ SC_DLLPUBLIC const_iterator end() const { return m_Data.end(); }
+ SC_DLLPUBLIC size_t size() const { return m_Data.size(); }
+ SC_DLLPUBLIC size_t index_size() const { return maIndexToData.size(); }
+ bool empty() const { return m_Data.empty(); }
/** Insert object into set.
@ATTENTION: The underlying ::std::map<std::unique_ptr>::insert(p) takes
@@ -264,7 +262,8 @@ public:
* iterator's validity. The caller must make sure that the iterator is
* valid.
*/
- void erase(const iterator& itr);
+ void erase(const_iterator itr);
+
void clear();
bool operator== (const ScRangeName& r) const;
};
diff --git a/sc/source/core/tool/rangenam.cxx b/sc/source/core/tool/rangenam.cxx
index 11e177158148..09bef58a1bd4 100644
--- a/sc/source/core/tool/rangenam.cxx
+++ b/sc/source/core/tool/rangenam.cxx
@@ -778,41 +778,6 @@ void ScRangeName::CopyUsedNames( const SCTAB nLocalTab,
const SCTAB nOldTab, con
}
}
-ScRangeName::const_iterator ScRangeName::begin() const
-{
- return m_Data.begin();
-}
-
-ScRangeName::const_iterator ScRangeName::end() const
-{
- return m_Data.end();
-}
-
-ScRangeName::iterator ScRangeName::begin()
-{
- return m_Data.begin();
-}
-
-ScRangeName::iterator ScRangeName::end()
-{
- return m_Data.end();
-}
-
-size_t ScRangeName::size() const
-{
- return m_Data.size();
-}
-
-size_t ScRangeName::index_size() const
-{
- return maIndexToData.size();
-}
-
-bool ScRangeName::empty() const
-{
- return m_Data.empty();
-}
-
bool ScRangeName::insert( ScRangeData* p, bool bReuseFreeIndex )
{
if (!p)
@@ -863,12 +828,12 @@ void ScRangeName::erase(const ScRangeData& r)
void ScRangeName::erase(const OUString& rName)
{
- DataType::iterator itr = m_Data.find(rName);
+ DataType::const_iterator itr = m_Data.find(rName);
if (itr != m_Data.end())
erase(itr);
}
-void ScRangeName::erase(const iterator& itr)
+void ScRangeName::erase(const_iterator itr)
{
sal_uInt16 nIndex = itr->second->GetIndex();
m_Data.erase(itr);
diff --git a/sc/source/ui/docshell/externalrefmgr.cxx
b/sc/source/ui/docshell/externalrefmgr.cxx
index 1fac22e41861..c25a79f1f919 100644
--- a/sc/source/ui/docshell/externalrefmgr.cxx
+++ b/sc/source/ui/docshell/externalrefmgr.cxx
@@ -222,7 +222,7 @@ class EraseRangeByIterator
ScRangeName& mrRanges;
public:
explicit EraseRangeByIterator(ScRangeName& rRanges) : mrRanges(rRanges) {}
- void operator() (const ScRangeName::iterator& itr)
+ void operator() (const ScRangeName::const_iterator& itr)
{
mrRanges.erase(itr);
}
@@ -234,8 +234,8 @@ public:
*/
void removeRangeNamesBySrcDoc(ScRangeName& rRanges, sal_uInt16 nFileId)
{
- ScRangeName::iterator itr = rRanges.begin(), itrEnd = rRanges.end();
- vector<ScRangeName::iterator> v;
+ ScRangeName::const_iterator itr = rRanges.begin(), itrEnd = rRanges.end();
+ vector<ScRangeName::const_iterator> v;
for (; itr != itrEnd; ++itr)
{
if (hasRefsToSrcDoc(*itr->second, nFileId))
