[Libreoffice-commits] core.git: Branch 'feature/unitver' - 14 commits - sc/Library_sc.mk sc/qa sc/source
Andrzej Hunt
andrzej.hunt at collabora.com
Sat May 9 00:28:57 PDT 2015
sc/Library_sc.mk | 1
sc/qa/unit/units.cxx | 141 ++++++++++++++++
sc/source/core/units/raustack.cxx | 54 ++++++
sc/source/core/units/raustack.hxx | 70 ++++++++
sc/source/core/units/unitsimpl.cxx | 308 ++++++++++++++++++++++++++++---------
sc/source/core/units/unitsimpl.hxx | 57 ++++++
sc/source/core/units/utunit.hxx | 18 ++
7 files changed, 575 insertions(+), 74 deletions(-)
New commits:
commit eba23893a8671d0d951a9b87d355b124024cee24
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Fri Apr 10 11:03:33 2015 +0100
Move and rename Range/Unit Stack.
This in preparation for implementing a combined Unit and Range iterator.
Change-Id: I08d28e175453f65c3696e9d1c6c20c7076d9b164
diff --git a/sc/Library_sc.mk b/sc/Library_sc.mk
index 778415c..e22bda9 100644
--- a/sc/Library_sc.mk
+++ b/sc/Library_sc.mk
@@ -688,6 +688,7 @@ $(call gb_Library_add_exception_objects,sc,\
ifeq ($(ENABLE_CALC_UNITVERIFICATION),TRUE)
$(eval $(call gb_Library_add_exception_objects,sc,\
+ sc/source/core/units/raustack \
sc/source/core/units/units \
sc/source/core/units/unitsimpl \
sc/source/core/units/util \
diff --git a/sc/source/core/units/raustack.cxx b/sc/source/core/units/raustack.cxx
new file mode 100644
index 0000000..f838acf
--- /dev/null
+++ b/sc/source/core/units/raustack.cxx
@@ -0,0 +1,54 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ */
+#include "raustack.hxx"
+
+using namespace sc::units;
+
+RangeListIterator::RangeListIterator(ScDocument* pDoc, const ScRangeList& rRangeList)
+ :
+ mRangeList(rRangeList),
+ mpDoc(pDoc),
+ mIt(pDoc, ScRange()),
+ nCurrentIndex(0)
+{
+}
+
+bool RangeListIterator::first() {
+ if (mRangeList.size() > 0) {
+ mIt = ScCellIterator(mpDoc, *mRangeList[0]);
+ return mIt.first();
+ } else {
+ return false;
+ }
+}
+
+const ScAddress& RangeListIterator::GetPos() const {
+ return mIt.GetPos();
+}
+
+bool RangeListIterator::next() {
+ if (!(mRangeList.size() > 0) || nCurrentIndex >= mRangeList.size()) {
+ return false;
+ }
+
+ if (mIt.next()) {
+ return true;
+ } else if (++nCurrentIndex < mRangeList.size()) {
+ mIt = ScCellIterator(mpDoc, *mRangeList[nCurrentIndex]);
+ mIt.first();
+ // TODO: if emtpy - skip to next...?
+ return true;
+ } else {
+ return false;
+ }
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
+
diff --git a/sc/source/core/units/raustack.hxx b/sc/source/core/units/raustack.hxx
new file mode 100644
index 0000000..b5cf48b
--- /dev/null
+++ b/sc/source/core/units/raustack.hxx
@@ -0,0 +1,70 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ */
+#ifndef INCLUDED_SC_SOURCE_CORE_UNITS_RAUSTACK_HXX
+#define INCLUDED_SC_SOURCE_CORE_UNITS_RAUSTACK_HXX
+
+#include <boost/variant.hpp>
+
+#include <address.hxx>
+#include <dociter.hxx>
+#include <rangelst.hxx>
+
+
+#include "utunit.hxx"
+
+namespace sc {
+namespace units {
+
+enum class RAUSItemType {
+ UNITS,
+ RANGE
+};
+
+struct RAUSItem {
+ RAUSItemType type;
+ boost::variant< ScRange, UtUnit > item;
+};
+
+class RangeListIterator {
+private:
+ const ScRangeList mRangeList;
+ ScDocument* mpDoc;
+
+ ScCellIterator mIt;
+ size_t nCurrentIndex;
+
+public:
+ RangeListIterator(ScDocument* pDoc, const ScRangeList& rRangeList);
+
+ bool first();
+
+ const ScAddress& GetPos() const;
+
+ bool next();
+};
+
+}} // sc::units
+
+// class RangeAndUnitStack {
+
+
+
+// };
+
+// class RATSIterator {
+// public:
+// // TODO: need to be able to return non-initialisation
+// static RATSIterator getIterator(RangeAndTokenStack& rStack, ScDoc* pDoc, int nItems);
+
+// }
+
+#endif // INCLUDED_SC_SOURCE_CORE_UNITS_RAUSTACK_HXX
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx
index 99e03af..e8fb261 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -11,7 +11,6 @@
#include "util.hxx"
-#include "dociter.hxx"
#include "document.hxx"
#include "refdata.hxx"
#include "stringutil.hxx"
@@ -85,55 +84,7 @@ UnitsImpl::~UnitsImpl() {
// (i.e. if udunits can't handle being used across threads)?
}
-class RangeListIterator {
-private:
- const ScRangeList mRangeList;
- ScDocument* mpDoc;
-
- ScCellIterator mIt;
- size_t nCurrentIndex;
-
-public:
- RangeListIterator(ScDocument* pDoc, const ScRangeList& rRangeList)
- :
- mRangeList(rRangeList),
- mpDoc(pDoc),
- mIt(pDoc, ScRange()),
- nCurrentIndex(0)
- {
- }
-
- bool first() {
- if (mRangeList.size() > 0) {
- mIt = ScCellIterator(mpDoc, *mRangeList[0]);
- return mIt.first();
- } else {
- return false;
- }
- }
-
- const ScAddress& GetPos() const {
- return mIt.GetPos();
- }
-
- bool next() {
- if (!(mRangeList.size() > 0) || nCurrentIndex >= mRangeList.size()) {
- return false;
- }
-
- if (mIt.next()) {
- return true;
- } else if (++nCurrentIndex < mRangeList.size()) {
- mIt = ScCellIterator(mpDoc, *mRangeList[nCurrentIndex]);
- mIt.first();
- return true;
- } else {
- return false;
- }
- }
-};
-
-UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< StackItem >& rStack, const formula::FormulaToken* pToken, ScDocument* pDoc) {
+UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< RAUSItem >& rStack, const formula::FormulaToken* pToken, ScDocument* pDoc) {
const OpCode aOpCode = pToken->GetOpCode();
auto nOpCode = static_cast<std::underlying_type<const OpCode>::type>(aOpCode);
@@ -147,7 +98,7 @@ UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< StackItem >& rStack, const
if (rStack.size() == 0) {
SAL_WARN("sc.units", "Single item opcode failed (no stack items, or range used)");
return { UnitsStatus::UNITS_INVALID, boost::none };
- } else if (rStack.top().type != StackItemType::UNITS) {
+ } else if (rStack.top().type != RAUSItemType::UNITS) {
return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
@@ -183,13 +134,13 @@ UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< StackItem >& rStack, const
return { UnitsStatus::UNITS_INVALID, boost::none };
}
- if (rStack.top().type != StackItemType::UNITS) {
+ if (rStack.top().type != RAUSItemType::UNITS) {
return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
UtUnit pSecondUnit = boost::get<UtUnit>(rStack.top().item);
rStack.pop();
- if (rStack.top().type != StackItemType::UNITS) {
+ if (rStack.top().type != RAUSItemType::UNITS) {
return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
UtUnit pFirstUnit = boost::get<UtUnit>(rStack.top().item);
@@ -240,12 +191,12 @@ UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< StackItem >& rStack, const
for ( ; nParams > 0; nParams--) {
switch (rStack.top().type) {
- case StackItemType::UNITS:
+ case RAUSItemType::UNITS:
{
aUnitsStack.push(boost::get< UtUnit >(rStack.top().item));
break;
}
- case StackItemType::RANGE:
+ case RAUSItemType::RANGE:
{
aRangeList.Append(boost::get< ScRange >(rStack.top().item));
break;
@@ -545,7 +496,7 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
pArray->Dump();
#endif
- stack< StackItem > aStack;
+ stack< RAUSItem > aStack;
for (FormulaToken* pToken = pArray->FirstRPN(); pToken != 0; pToken = pArray->NextRPN()) {
switch (pToken->GetType()) {
@@ -564,14 +515,14 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
return false;
}
- aStack.push( { StackItemType::UNITS, aUnit } );
+ aStack.push( { RAUSItemType::UNITS, aUnit } );
break;
}
case formula::svDoubleRef:
{
ScComplexRefData* pDoubleRef = pToken->GetDoubleRef();
ScRange aRange = pDoubleRef->toAbs(rFormulaAddress);
- aStack.push( { StackItemType::RANGE, aRange } );
+ aStack.push( { RAUSItemType::RANGE, aRange } );
break;
}
@@ -588,7 +539,7 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
case UnitsStatus::UNITS_VALID:
assert(aResult.units); // ensure that we have the optional unit
assert(aResult.units->isValid());
- aStack.push( { StackItemType::UNITS, aResult.units.get() } );
+ aStack.push( { RAUSItemType::UNITS, aResult.units.get() } );
break;
}
@@ -603,7 +554,7 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
UtUnit::createUnit("", aScale, mpUnitSystem); // Get the dimensionless unit 1
aScale = aScale*(pToken->GetDouble());
- aStack.push( { StackItemType::UNITS, aScale } );
+ aStack.push( { RAUSItemType::UNITS, aScale } );
break;
}
@@ -619,7 +570,7 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
if (aStack.size() != 1) {
SAL_WARN("sc.units", "Wrong number of units on stack, should be 1, actual number: " << aStack.size());
return false;
- } else if (aStack.top().type != StackItemType::UNITS) {
+ } else if (aStack.top().type != RAUSItemType::UNITS) {
SAL_WARN("sc.units", "End of verification: item on stack does not contain units");
return false;
}
diff --git a/sc/source/core/units/unitsimpl.hxx b/sc/source/core/units/unitsimpl.hxx
index 6abffdd..379a6a7 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -13,7 +13,6 @@
#include <boost/optional.hpp>
#include <boost/scoped_ptr.hpp>
#include <boost/shared_ptr.hpp>
-#include <boost/variant.hpp>
#include <boost/weak_ptr.hpp>
#include <formula/opcode.hxx>
@@ -25,6 +24,7 @@
#include "rangelst.hxx"
#include "units.hxx"
+#include "raustack.hxx"
#include "utunit.hxx"
#include <stack>
@@ -42,16 +42,6 @@ namespace test {
class UnitsTest;
}
-enum class StackItemType {
- UNITS,
- RANGE
-};
-
-struct StackItem {
- StackItemType type;
- boost::variant< ScRange, UtUnit > item;
-};
-
enum class UnitsStatus {
UNITS_VALID,
UNITS_UNKNOWN,
@@ -103,7 +93,7 @@ public:
const OUString& rsOldUnit) SAL_OVERRIDE;
private:
- UnitsResult getOutputUnitsForOpCode(std::stack< StackItem >& rStack, const formula::FormulaToken* pToken, ScDocument* pDoc);
+ UnitsResult getOutputUnitsForOpCode(std::stack< RAUSItem >& rStack, const formula::FormulaToken* pToken, ScDocument* pDoc);
/**
* Find and extract a Unit in the standard header notation,
commit d9bdadb13fa78593c63d561fa00dbbb4c9a72245
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Wed Apr 1 14:54:20 2015 +0200
Check output unit against header definition during verification.
Change-Id: I98a706d80eb442d274fc111fb6c22e43d79fb9ff
diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx
index fbd0c2d..99e03af 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -616,7 +616,23 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
}
}
- // TODO: only fail if actual parsing fails?
+ if (aStack.size() != 1) {
+ SAL_WARN("sc.units", "Wrong number of units on stack, should be 1, actual number: " << aStack.size());
+ return false;
+ } else if (aStack.top().type != StackItemType::UNITS) {
+ SAL_WARN("sc.units", "End of verification: item on stack does not contain units");
+ return false;
+ }
+
+ OUString sUnitString;
+ ScAddress aAddress;
+
+ UtUnit aHeaderUnit = findHeaderUnitForCell(rFormulaAddress, pDoc, sUnitString, aAddress);
+ UtUnit aResultUnit = boost::get< UtUnit>(aStack.top().item);
+
+ if (aHeaderUnit.isValid() && aHeaderUnit != aResultUnit) {
+ return false;
+ }
return true;
}
commit f1328fab55923b99b2c08dc5fbba0b4dfff2eda4
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Wed Apr 1 13:48:01 2015 +0200
Use correct stack for multiparam opcodes.
Also add an appropriate unit test.
Change-Id: Ie2e9cce74563dea80b33f5e8238ba6ad1aae1b49
diff --git a/sc/qa/unit/units.cxx b/sc/qa/unit/units.cxx
index ac1bfa5..7b5fd25 100644
--- a/sc/qa/unit/units.cxx
+++ b/sc/qa/unit/units.cxx
@@ -291,7 +291,6 @@ void UnitsTest::testUnitVerification() {
// SUM("cm"&"kg") - multiple ranges
address.IncRow();
- // TODO: by default / when testing: separator is...?
mpDoc->SetFormula(address, "=SUM(A1:A3,B1:B3)");
pCell = mpDoc->GetFormulaCell(address);
pTokens = pCell->GetCode();
@@ -325,6 +324,23 @@ void UnitsTest::testUnitVerification() {
pTokens = pCell->GetCode();
CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+ // Test that multiple arguments of mixed types work too
+ // Adding multiple cells from one column is ok
+ address.IncRow();
+ mpDoc->SetFormula(address, "=SUM(A1,A2:A3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // But mixing the columns fails because of mixed units
+ // (This test is primarily to ensure that we can handle arbitrary numbers
+ // of arguments.)
+ address.IncRow();
+ mpDoc->SetFormula(address, "=SUM(A1,A2:A3,B1:B3,C1,C2,C3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
// Do a quick sanity check for all the other supported functions
// The following all expect identically united inputs, and should
// preserve that unit:
diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx
index d49799f..fbd0c2d 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -274,7 +274,7 @@ UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< StackItem >& rStack, const
return { UnitsStatus::UNITS_INVALID, boost::none };
}
}
- rStack.pop();
+ aUnitsStack.pop();
}
if (aIt.first()) {
commit d25ffe2e1d65a06e85b5f7f278a977bbd679742c
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Wed Apr 1 13:21:35 2015 +0200
Refactor and rewrite range and multiparam opcode handling.
Change-Id: Ic52bfb7daae44ea8f8af710fa70f1f30150fc274
diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx
index 6057a0e..d49799f 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -85,26 +85,76 @@ UnitsImpl::~UnitsImpl() {
// (i.e. if udunits can't handle being used across threads)?
}
-UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack, const OpCode& rOpCode) {
- UtUnit pOut;
+class RangeListIterator {
+private:
+ const ScRangeList mRangeList;
+ ScDocument* mpDoc;
- auto nOpCode = static_cast<std::underlying_type<const OpCode>::type>(rOpCode);
+ ScCellIterator mIt;
+ size_t nCurrentIndex;
+
+public:
+ RangeListIterator(ScDocument* pDoc, const ScRangeList& rRangeList)
+ :
+ mRangeList(rRangeList),
+ mpDoc(pDoc),
+ mIt(pDoc, ScRange()),
+ nCurrentIndex(0)
+ {
+ }
+ bool first() {
+ if (mRangeList.size() > 0) {
+ mIt = ScCellIterator(mpDoc, *mRangeList[0]);
+ return mIt.first();
+ } else {
+ return false;
+ }
+ }
+
+ const ScAddress& GetPos() const {
+ return mIt.GetPos();
+ }
+
+ bool next() {
+ if (!(mRangeList.size() > 0) || nCurrentIndex >= mRangeList.size()) {
+ return false;
+ }
+
+ if (mIt.next()) {
+ return true;
+ } else if (++nCurrentIndex < mRangeList.size()) {
+ mIt = ScCellIterator(mpDoc, *mRangeList[nCurrentIndex]);
+ mIt.first();
+ return true;
+ } else {
+ return false;
+ }
+ }
+};
+
+UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< StackItem >& rStack, const formula::FormulaToken* pToken, ScDocument* pDoc) {
+ const OpCode aOpCode = pToken->GetOpCode();
+ auto nOpCode = static_cast<std::underlying_type<const OpCode>::type>(aOpCode);
+
+ UtUnit pOut;
// TODO: sc/source/core/tool/parclass.cxx has a mapping of opcodes to possible operands, which we
// should probably be using in practice.
if (nOpCode >= SC_OPCODE_START_UN_OP &&
nOpCode < SC_OPCODE_STOP_UN_OP) {
- if (!(rUnitStack.size() >= 1)) {
- SAL_WARN("sc.units", "no units on stack for unary operation");
+ if (rStack.size() == 0) {
+ SAL_WARN("sc.units", "Single item opcode failed (no stack items, or range used)");
return { UnitsStatus::UNITS_INVALID, boost::none };
+ } else if (rStack.top().type != StackItemType::UNITS) {
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
- UtUnit pUnit = rUnitStack.top();
- rUnitStack.pop();
+ UtUnit pUnit = boost::get<UtUnit>(rStack.top().item);//rStack.top().item.get< UtUnit >();
+ rStack.pop();
- switch (rOpCode) {
+ switch (aOpCode) {
case ocNot:
if (!pUnit.isDimensionless()) {
return { UnitsStatus::UNITS_INVALID, boost::none };
@@ -128,19 +178,24 @@ UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack, cons
} else if (nOpCode >= SC_OPCODE_START_BIN_OP &&
nOpCode < SC_OPCODE_STOP_BIN_OP) {
- if (!(rUnitStack.size() >= 2)) {
- SAL_WARN("sc.units", "less than two units on stack when attempting binary operation");
- // TODO: what should we be telling the user in this case? Can this even happen (i.e.
- // should we just be asserting here?)
+ if (rStack.size() < 2) {
+ SAL_WARN("sc.units", "less than two items on stack when attempting binary operation");
return { UnitsStatus::UNITS_INVALID, boost::none };
}
- UtUnit pSecondUnit = rUnitStack.top();
- rUnitStack.pop();
- UtUnit pFirstUnit = rUnitStack.top();
- rUnitStack.pop();
+ if (rStack.top().type != StackItemType::UNITS) {
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
+ }
+ UtUnit pSecondUnit = boost::get<UtUnit>(rStack.top().item);
+ rStack.pop();
+
+ if (rStack.top().type != StackItemType::UNITS) {
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
+ }
+ UtUnit pFirstUnit = boost::get<UtUnit>(rStack.top().item);
+ rStack.pop();
- switch (rOpCode) {
+ switch (aOpCode) {
case ocAdd:
// Adding and subtracting both require the same units on both sides
// hence we can just fall through / use the same logic.
@@ -164,90 +219,111 @@ UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack, cons
SAL_INFO("sc.units", "unit verification not supported for opcode: " << nOpCode);
assert(false);
}
+ } else if (nOpCode >= SC_OPCODE_START_2_PAR &&
+ nOpCode < SC_OPCODE_STOP_2_PAR) {
+ sal_uInt8 nParams = pToken->GetByte();
- } else {
- SAL_INFO("sc.units", "unit verification not supported for opcode: " << nOpCode);
- return { UnitsStatus::UNITS_UNKNOWN, boost::none };
- }
- // TODO: else if unary, or no params, or ...
- // TODO: implement further sensible opcode handling
- return { UnitsStatus::UNITS_VALID, pOut };
-}
-
-class RangeListIterator {
-private:
- const ScRangeList mRangeList;
- ScDocument* mpDoc;
-
- ScCellIterator mIt;
- size_t nCurrentIndex;
+ assert(nParams <= rStack.size());
-public:
- RangeListIterator(ScDocument* pDoc, const ScRangeList& rRangeList)
- :
- mRangeList(rRangeList),
- mpDoc(pDoc),
- mIt(pDoc, ScRange()),
- nCurrentIndex(0)
- {
- if (rRangeList.size() > 0) {
- mIt = ScCellIterator(pDoc, *rRangeList[0]);
- mIt.first();
+ // If there are no input parameters then the output unit is undefined.
+ // (In practice this would probably be nonsensical, but isn't a unit
+ // error per-se.)
+ if (nParams == 0) {
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
- }
- const ScAddress& GetPos() const {
- return mIt.GetPos();
- }
+ // This is still quite an ugly solution, even better would maybe be to have
+ // a StackUnitIterator which iterates all the appropriate units given a number of stack items
+ // to iterate over?
+ ScRangeList aRangeList;
+ stack< UtUnit > aUnitsStack;
+
+ for ( ; nParams > 0; nParams--) {
+ switch (rStack.top().type) {
+ case StackItemType::UNITS:
+ {
+ aUnitsStack.push(boost::get< UtUnit >(rStack.top().item));
+ break;
+ }
+ case StackItemType::RANGE:
+ {
+ aRangeList.Append(boost::get< ScRange >(rStack.top().item));
+ break;
+ }
+ }
- bool next() {
- if (!(mRangeList.size() > 0) || nCurrentIndex >= mRangeList.size()) {
- return false;
+ rStack.pop();
}
- if (mIt.next()) {
- return true;
- } else if (++nCurrentIndex < mRangeList.size()) {
- mIt = ScCellIterator(mpDoc, *mRangeList[nCurrentIndex]);
- mIt.first();
- return true;
- } else {
- return false;
- }
- }
-};
+ RangeListIterator aIt(pDoc, aRangeList);
-UnitsResult UnitsImpl::getOutputUnitForDoubleRefOpcode(const OpCode& rOpCode, const ScRangeList& rRangeList, ScDocument* pDoc) {
- RangeListIterator aIt(pDoc, rRangeList);
+ switch (aOpCode) {
+ case ocSum:
+ case ocMin:
+ case ocMax:
+ case ocAverage:
+ {
+ boost::optional< UtUnit > aFirstUnit;
+
+ while (aUnitsStack.size() > 0) {
+ if (!aFirstUnit) {
+ aFirstUnit = aUnitsStack.top();
+ } else {
+ UtUnit aCurrentUnit(aUnitsStack.top());
+ if (aFirstUnit.get() != aCurrentUnit) {
+ return { UnitsStatus::UNITS_INVALID, boost::none };
+ }
+ }
+ rStack.pop();
+ }
- switch (rOpCode) {
- case ocSum:
- case ocMin:
- case ocMax:
- case ocAverage:
- {
- UtUnit aFirstUnit = getUnitForCell(aIt.GetPos(), pDoc);
+ if (aIt.first()) {
+ do {
+ if (!aFirstUnit) {
+ aFirstUnit = getUnitForCell(aIt.GetPos(), pDoc);
+ } else {
+ UtUnit aCurrentUnit = getUnitForCell(aIt.GetPos(), pDoc);
+ if (aFirstUnit.get() != aCurrentUnit) {
+ return { UnitsStatus::UNITS_INVALID, boost::none };
+ }
+ }
+ } while (aIt.next());
- while (aIt.next()) {
- UtUnit aCurrentUnit = getUnitForCell(aIt.GetPos(), pDoc);
- if (aFirstUnit != aCurrentUnit) {
- return { UnitsStatus::UNITS_INVALID, boost::none };
}
+
+ return { UnitsStatus::UNITS_VALID, aFirstUnit };
}
+ case ocProduct:
+ {
+ // To avoid having to detect when we get the first unit (i.e. to avoid
+ // the play with the optinal< UtUnit > as above), we just start with
+ // the dimensionless Unit 1 and multiply from there.
+ // This can also be avoided if we implement a combined Unit and Range
+ // Iterator (see above for more info).
+ UtUnit aUnit;
+ UtUnit::createUnit("", aUnit, mpUnitSystem);
+
+ while (aUnitsStack.size() > 0) {
+ aUnit *= aUnitsStack.top();
+ aUnitsStack.pop();
+ }
- return { UnitsStatus::UNITS_VALID, aFirstUnit };
- }
- case ocProduct:
- {
- UtUnit aUnit = getUnitForCell(aIt.GetPos(), pDoc);
- while (aIt.next()) {
- aUnit *= getUnitForCell(aIt.GetPos(), pDoc);
+ if (aIt.first()) {
+ do {
+ aUnit *= getUnitForCell(aIt.GetPos(), pDoc);
+ } while (aIt.next());
+ }
+
+ return { UnitsStatus::UNITS_VALID, aUnit };
}
- return { UnitsStatus::UNITS_VALID, aUnit };
- }
- default:
+ default:
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
+ }
+ } else {
+ SAL_INFO("sc.units", "unit verification not supported for opcode: " << nOpCode);
return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
+ return { UnitsStatus::UNITS_VALID, pOut };
}
OUString UnitsImpl::extractUnitStringFromFormat(const OUString& rFormatString) {
@@ -469,62 +545,42 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
pArray->Dump();
#endif
- stack< UtUnit > aUnitStack;
+ stack< StackItem > aStack;
for (FormulaToken* pToken = pArray->FirstRPN(); pToken != 0; pToken = pArray->NextRPN()) {
switch (pToken->GetType()) {
case formula::svSingleRef:
{
- UtUnit pUnit(getUnitForRef(pToken, rFormulaAddress, pDoc));
+ UtUnit aUnit(getUnitForRef(pToken, rFormulaAddress, pDoc));
- if (!pUnit.isValid()) {
+ if (!aUnit.isValid()) {
SAL_INFO("sc.units", "no unit returned for scSingleRef, ut_status: " << getUTStatus());
// This only happens in case of parsing (or system) errors.
// However maybe we should be returning "unverified" for
// unparseable formulas?
// (or even have a "can't be verified" state too?)
- // see below for more.
+ // see below for more.x
return false;
}
- aUnitStack.push(pUnit);
+ aStack.push( { StackItemType::UNITS, aUnit } );
break;
}
case formula::svDoubleRef:
{
ScComplexRefData* pDoubleRef = pToken->GetDoubleRef();
- ScRangeList aRangeList(pDoubleRef->toAbs(rFormulaAddress));
-
- // Functions operating on DoubleRefs seem to have a variable number of input
- // arguments (which will all preceed the opcode in the TokenArray), hence
- // we read all the ranges in first before operating on them.
- // The appropriate handling of the opcode depends on the specific opcode
- // (i.e. identical units needed for sum, but not for multiplication), hence
- // we need to handle that opcode here as opposed to as part of the usual flow
- // (where we would simply push the units onto the stack).
- pToken = pArray->NextRPN();
- while (pToken->GetType() == formula::svDoubleRef) {
- pDoubleRef = pToken->GetDoubleRef();
- aRangeList.Append(pDoubleRef->toAbs(rFormulaAddress));
- pToken = pArray->NextRPN();
- }
-
- if (!pToken) {
- // It's possible to have DoubleRef as the last token, e.g the simplest
- // example being "=A1:B1" - in this case the formula can't be evaluated
- // anyways, so giving up on unit verification is the most sensible option.
- // (I.e. the formula ends up spewing out #VALUE to the end-user - there's
- // no point in doing any verification here.)
- return true;
- }
-
- UnitsResult aResult(getOutputUnitForDoubleRefOpcode(pToken->GetOpCode(), aRangeList, pDoc));
+ ScRange aRange = pDoubleRef->toAbs(rFormulaAddress);
+ aStack.push( { StackItemType::RANGE, aRange } );
+ break;
+ }
+ case formula::svByte:
+ {
+ UnitsResult aResult = getOutputUnitsForOpCode(aStack, pToken, pDoc);
switch (aResult.status) {
case UnitsStatus::UNITS_INVALID:
- SAL_INFO("sc.units", "svDoubleRef opcode unit error detected");
return false;
case UnitsStatus::UNITS_UNKNOWN:
// Unsupported hence we stop processing.
@@ -532,26 +588,12 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
case UnitsStatus::UNITS_VALID:
assert(aResult.units); // ensure that we have the optional unit
assert(aResult.units->isValid());
- aUnitStack.push(aResult.units.get());
+ aStack.push( { StackItemType::UNITS, aResult.units.get() } );
break;
}
break;
}
- case formula::svByte:
- {
- UtUnit pOut = getOutputUnitsForOpCode(aUnitStack, pToken->GetOpCode());
-
- // A null unit indicates either invalid units and/or other erronous input
- // i.e. is an indication that getOutputUnitsForOpCode failed.
- if (pOut.isValid()) {
- aUnitStack.push(pOut);
- } else {
- return false;
- }
-
- break;
- }
// As far as I can tell this is only used for an actual numerical value
// in which case we just want to use it as scaling factor
// (for example [m] + [cm]/100 would be a valid operation)
@@ -561,7 +603,8 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
UtUnit::createUnit("", aScale, mpUnitSystem); // Get the dimensionless unit 1
aScale = aScale*(pToken->GetDouble());
- aUnitStack.push(aScale);
+ aStack.push( { StackItemType::UNITS, aScale } );
+
break;
}
default:
diff --git a/sc/source/core/units/unitsimpl.hxx b/sc/source/core/units/unitsimpl.hxx
index aacf9bb..6abffdd 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -13,6 +13,7 @@
#include <boost/optional.hpp>
#include <boost/scoped_ptr.hpp>
#include <boost/shared_ptr.hpp>
+#include <boost/variant.hpp>
#include <boost/weak_ptr.hpp>
#include <formula/opcode.hxx>
@@ -41,6 +42,16 @@ namespace test {
class UnitsTest;
}
+enum class StackItemType {
+ UNITS,
+ RANGE
+};
+
+struct StackItem {
+ StackItemType type;
+ boost::variant< ScRange, UtUnit > item;
+};
+
enum class UnitsStatus {
UNITS_VALID,
UNITS_UNKNOWN,
@@ -92,8 +103,7 @@ public:
const OUString& rsOldUnit) SAL_OVERRIDE;
private:
- UnitsResult getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack, const OpCode& rOpCode);
- UnitsResult getOutputUnitForDoubleRefOpcode(const OpCode& rOpCode, const ScRangeList& rRange, ScDocument* pDoc);
+ UnitsResult getOutputUnitsForOpCode(std::stack< StackItem >& rStack, const formula::FormulaToken* pToken, ScDocument* pDoc);
/**
* Find and extract a Unit in the standard header notation,
commit ab49005fecee1a28e376b10883b82d88d80eab9d
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Mon Mar 30 19:57:04 2015 +0200
Use UnitsResult for processing of opcodes too.
Change-Id: I7bbbf4ff0bcbd01d70fd929cf62537ceec364c83
diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx
index 3a181b5..6057a0e 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -85,7 +85,7 @@ UnitsImpl::~UnitsImpl() {
// (i.e. if udunits can't handle being used across threads)?
}
-UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack, const OpCode& rOpCode) {
+UnitsResult UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack, const OpCode& rOpCode) {
UtUnit pOut;
auto nOpCode = static_cast<std::underlying_type<const OpCode>::type>(rOpCode);
@@ -98,7 +98,7 @@ UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack, const OpC
if (!(rUnitStack.size() >= 1)) {
SAL_WARN("sc.units", "no units on stack for unary operation");
- return UtUnit();
+ return { UnitsStatus::UNITS_INVALID, boost::none };
}
UtUnit pUnit = rUnitStack.top();
@@ -107,7 +107,7 @@ UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack, const OpC
switch (rOpCode) {
case ocNot:
if (!pUnit.isDimensionless()) {
- return UtUnit();
+ return { UnitsStatus::UNITS_INVALID, boost::none };
}
// We just keep the same unit (in this case no unit) so can
// fall through.
@@ -132,7 +132,7 @@ UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack, const OpC
SAL_WARN("sc.units", "less than two units on stack when attempting binary operation");
// TODO: what should we be telling the user in this case? Can this even happen (i.e.
// should we just be asserting here?)
- return UtUnit();
+ return { UnitsStatus::UNITS_INVALID, boost::none };
}
UtUnit pSecondUnit = rUnitStack.top();
@@ -150,6 +150,7 @@ UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack, const OpC
pOut = pFirstUnit;
SAL_INFO("sc.units", "verified equality for unit " << pFirstUnit);
} else {
+ return { UnitsStatus::UNITS_INVALID, boost::none };
// TODO: notify/link UI.
}
break;
@@ -166,11 +167,11 @@ UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack, const OpC
} else {
SAL_INFO("sc.units", "unit verification not supported for opcode: " << nOpCode);
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
}
// TODO: else if unary, or no params, or ...
// TODO: implement further sensible opcode handling
-
- return pOut;
+ return { UnitsStatus::UNITS_VALID, pOut };
}
class RangeListIterator {
diff --git a/sc/source/core/units/unitsimpl.hxx b/sc/source/core/units/unitsimpl.hxx
index d6d001c..aacf9bb 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -92,7 +92,7 @@ public:
const OUString& rsOldUnit) SAL_OVERRIDE;
private:
- UtUnit getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack, const OpCode& rOpCode);
+ UnitsResult getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack, const OpCode& rOpCode);
UnitsResult getOutputUnitForDoubleRefOpcode(const OpCode& rOpCode, const ScRangeList& rRange, ScDocument* pDoc);
/**
commit f300d3e5fb406c9cb8df09679144fa8d31751405
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Mon Mar 30 11:12:42 2015 +0200
Implement verification of sum/product and other opcodes for ranges.
Change-Id: I8d0d4cf46bddd585e633b5ee297b6bd7e0a2cf3b
diff --git a/sc/qa/unit/units.cxx b/sc/qa/unit/units.cxx
index af5fff5..ac1bfa5 100644
--- a/sc/qa/unit/units.cxx
+++ b/sc/qa/unit/units.cxx
@@ -79,6 +79,11 @@ void UnitsTest::setUp() {
mpDoc = &m_xDocShRef->GetDocument();
mpUnitsImpl = UnitsImpl::GetUnits();
+
+ // Set separators for formulae - for now just argument separators (no matrix separators)
+ ScFormulaOptions aOptions;
+ aOptions.SetFormulaSepArg(",");
+ m_xDocShRef->SetFormulaOptions(aOptions);
}
void UnitsTest::tearDown() {
@@ -183,11 +188,19 @@ void UnitsTest::testUnitVerification() {
mpDoc->SetNumberFormat(address, nKeyS);
mpDoc->SetValue(address, 3);
- // 4th column: 5cm/s
+ // 4th column: 5cm/s, 10cm/s, 15cm/s
address = ScAddress(3, 0, 0);
mpDoc->SetNumberFormat(address, nKeyCM_S);
mpDoc->SetValue(address, 5);
+ address.IncRow();
+ mpDoc->SetNumberFormat(address, nKeyCM_S);
+ mpDoc->SetValue(address, 10);
+
+ address.IncRow();
+ mpDoc->SetNumberFormat(address, nKeyCM_S);
+ mpDoc->SetValue(address, 15);
+
// 5th column: 1m
address = ScAddress(4, 0, 0);
mpDoc->SetNumberFormat(address, nKeyM);
@@ -256,11 +269,81 @@ void UnitsTest::testUnitVerification() {
// And scaling doesn't help when adding incompatible units (kg + 100*m)
address = ScAddress(0, 13, 0);
- mpDoc->SetFormula(address, "=B1+100E1");
+ mpDoc->SetFormula(address, "=B1+100*E1");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // Test Ranges:
+ // SUM("kg")
+ address.IncRow();
+ mpDoc->SetFormula(address, "=SUM(B1:B3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // SUM("cm"&"kg")
+ address.IncRow();
+ mpDoc->SetFormula(address, "=SUM(A1:B3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // SUM("cm"&"kg") - multiple ranges
+ address.IncRow();
+ // TODO: by default / when testing: separator is...?
+ mpDoc->SetFormula(address, "=SUM(A1:A3,B1:B3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // SUM("cm")+SUM("kg")
+ address.IncRow();
+ mpDoc->SetFormula(address, "=SUM(A1:A3)+SUM(B1:B3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // SUM("cm")/SUM("s")+SUM("cm/s")
+ address.IncRow();
+ mpDoc->SetFormula(address, "=SUM(A1:A3)/SUM(C1:C3)+SUM(D1:D3)");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // PRODUCT("cm/","s")+"cm"
+ address.IncRow();
+ mpDoc->SetFormula(address, "=PRODUCT(C1:D1)+A1");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // PRODUCT("cm/","s")+"kg"
+ address.IncRow();
+ mpDoc->SetFormula(address, "=PRODUCT(C1:D1)+B1");
pCell = mpDoc->GetFormulaCell(address);
pTokens = pCell->GetCode();
CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+ // Do a quick sanity check for all the other supported functions
+ // The following all expect identically united inputs, and should
+ // preserve that unit:
+ std::vector<OUString> aPreservingFuncs{ "SUM", "AVERAGE", "MIN", "MAX" };
+ for (const OUString& aFunc: aPreservingFuncs) {
+ // FOO(cm) + cm
+ address.IncRow();
+ mpDoc->SetFormula(address, "=" + aFunc + "(A1:A2)+A3");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // FOO(cm) + kg
+ address.IncRow();
+ mpDoc->SetFormula(address, "=" + aFunc + "(A1:A2)+B3");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+ }
}
void UnitsTest::testUnitFromFormatStringExtraction() {
diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx
index 6d280b4..3a181b5 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -11,6 +11,7 @@
#include "util.hxx"
+#include "dociter.hxx"
#include "document.hxx"
#include "refdata.hxx"
#include "stringutil.hxx"
@@ -172,6 +173,82 @@ UtUnit UnitsImpl::getOutputUnitsForOpCode(stack< UtUnit >& rUnitStack, const OpC
return pOut;
}
+class RangeListIterator {
+private:
+ const ScRangeList mRangeList;
+ ScDocument* mpDoc;
+
+ ScCellIterator mIt;
+ size_t nCurrentIndex;
+
+public:
+ RangeListIterator(ScDocument* pDoc, const ScRangeList& rRangeList)
+ :
+ mRangeList(rRangeList),
+ mpDoc(pDoc),
+ mIt(pDoc, ScRange()),
+ nCurrentIndex(0)
+ {
+ if (rRangeList.size() > 0) {
+ mIt = ScCellIterator(pDoc, *rRangeList[0]);
+ mIt.first();
+ }
+ }
+
+ const ScAddress& GetPos() const {
+ return mIt.GetPos();
+ }
+
+ bool next() {
+ if (!(mRangeList.size() > 0) || nCurrentIndex >= mRangeList.size()) {
+ return false;
+ }
+
+ if (mIt.next()) {
+ return true;
+ } else if (++nCurrentIndex < mRangeList.size()) {
+ mIt = ScCellIterator(mpDoc, *mRangeList[nCurrentIndex]);
+ mIt.first();
+ return true;
+ } else {
+ return false;
+ }
+ }
+};
+
+UnitsResult UnitsImpl::getOutputUnitForDoubleRefOpcode(const OpCode& rOpCode, const ScRangeList& rRangeList, ScDocument* pDoc) {
+ RangeListIterator aIt(pDoc, rRangeList);
+
+ switch (rOpCode) {
+ case ocSum:
+ case ocMin:
+ case ocMax:
+ case ocAverage:
+ {
+ UtUnit aFirstUnit = getUnitForCell(aIt.GetPos(), pDoc);
+
+ while (aIt.next()) {
+ UtUnit aCurrentUnit = getUnitForCell(aIt.GetPos(), pDoc);
+ if (aFirstUnit != aCurrentUnit) {
+ return { UnitsStatus::UNITS_INVALID, boost::none };
+ }
+ }
+
+ return { UnitsStatus::UNITS_VALID, aFirstUnit };
+ }
+ case ocProduct:
+ {
+ UtUnit aUnit = getUnitForCell(aIt.GetPos(), pDoc);
+ while (aIt.next()) {
+ aUnit *= getUnitForCell(aIt.GetPos(), pDoc);
+ }
+ return { UnitsStatus::UNITS_VALID, aUnit };
+ }
+ default:
+ return { UnitsStatus::UNITS_UNKNOWN, boost::none };
+ }
+}
+
OUString UnitsImpl::extractUnitStringFromFormat(const OUString& rFormatString) {
// TODO: decide what we do for different subformats? Simplest solution
// would be to not allow unit storage for multiple subformats.
@@ -413,6 +490,53 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
aUnitStack.push(pUnit);
break;
}
+ case formula::svDoubleRef:
+ {
+ ScComplexRefData* pDoubleRef = pToken->GetDoubleRef();
+ ScRangeList aRangeList(pDoubleRef->toAbs(rFormulaAddress));
+
+ // Functions operating on DoubleRefs seem to have a variable number of input
+ // arguments (which will all preceed the opcode in the TokenArray), hence
+ // we read all the ranges in first before operating on them.
+ // The appropriate handling of the opcode depends on the specific opcode
+ // (i.e. identical units needed for sum, but not for multiplication), hence
+ // we need to handle that opcode here as opposed to as part of the usual flow
+ // (where we would simply push the units onto the stack).
+ pToken = pArray->NextRPN();
+ while (pToken->GetType() == formula::svDoubleRef) {
+ pDoubleRef = pToken->GetDoubleRef();
+ aRangeList.Append(pDoubleRef->toAbs(rFormulaAddress));
+ pToken = pArray->NextRPN();
+ }
+
+ if (!pToken) {
+ // It's possible to have DoubleRef as the last token, e.g the simplest
+ // example being "=A1:B1" - in this case the formula can't be evaluated
+ // anyways, so giving up on unit verification is the most sensible option.
+ // (I.e. the formula ends up spewing out #VALUE to the end-user - there's
+ // no point in doing any verification here.)
+ return true;
+ }
+
+ UnitsResult aResult(getOutputUnitForDoubleRefOpcode(pToken->GetOpCode(), aRangeList, pDoc));
+
+
+ switch (aResult.status) {
+ case UnitsStatus::UNITS_INVALID:
+ SAL_INFO("sc.units", "svDoubleRef opcode unit error detected");
+ return false;
+ case UnitsStatus::UNITS_UNKNOWN:
+ // Unsupported hence we stop processing.
+ return true;
+ case UnitsStatus::UNITS_VALID:
+ assert(aResult.units); // ensure that we have the optional unit
+ assert(aResult.units->isValid());
+ aUnitStack.push(aResult.units.get());
+ break;
+ }
+
+ break;
+ }
case formula::svByte:
{
UtUnit pOut = getOutputUnitsForOpCode(aUnitStack, pToken->GetOpCode());
diff --git a/sc/source/core/units/unitsimpl.hxx b/sc/source/core/units/unitsimpl.hxx
index e1427ed..d6d001c 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -10,6 +10,7 @@
#ifndef INCLUDED_SC_SOURCE_CORE_UNITS_UNITSIMPL_HXX
#define INCLUDED_SC_SOURCE_CORE_UNITS_UNITSIMPL_HXX
+#include <boost/optional.hpp>
#include <boost/scoped_ptr.hpp>
#include <boost/shared_ptr.hpp>
#include <boost/weak_ptr.hpp>
@@ -20,7 +21,9 @@
#include <udunits2.h>
-#include <units.hxx>
+#include "rangelst.hxx"
+#include "units.hxx"
+
#include "utunit.hxx"
#include <stack>
@@ -38,6 +41,22 @@ namespace test {
class UnitsTest;
}
+enum class UnitsStatus {
+ UNITS_VALID,
+ UNITS_UNKNOWN,
+ UNITS_INVALID
+};
+
+/**
+ * The result for a given units operation.
+ * If UNITS_VALID then the resulting unit is also included,
+ * otherwise units is empty.
+ */
+struct UnitsResult {
+ UnitsStatus status;
+ boost::optional<UtUnit> units;
+};
+
class UnitsImpl: public Units {
friend class test::UnitsTest;
@@ -74,6 +93,7 @@ public:
private:
UtUnit getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack, const OpCode& rOpCode);
+ UnitsResult getOutputUnitForDoubleRefOpcode(const OpCode& rOpCode, const ScRangeList& rRange, ScDocument* pDoc);
/**
* Find and extract a Unit in the standard header notation,
commit 9b3ca1ea7ba35f2c1404d24419edde2d7b02ecc7
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Mon Mar 30 09:50:45 2015 +0200
Move declarations to more sensible place.
Change-Id: Ieb60b2a263a4d700f366b0aae7128e5a66ac4205
diff --git a/sc/source/core/units/unitsimpl.hxx b/sc/source/core/units/unitsimpl.hxx
index 91730f7..e1427ed 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -74,8 +74,6 @@ public:
private:
UtUnit getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack, const OpCode& rOpCode);
- OUString extractUnitStringFromFormat(const OUString& rFormatString);
- OUString extractUnitStringForCell(const ScAddress& rAddress, ScDocument* pDoc);
/**
* Find and extract a Unit in the standard header notation,
@@ -101,6 +99,9 @@ private:
bool extractUnitFromHeaderString(const OUString& rHeader, UtUnit& aUnit, OUString& sUnitString);
+ OUString extractUnitStringFromFormat(const OUString& rFormatString);
+ OUString extractUnitStringForCell(const ScAddress& rAddress, ScDocument* pDoc);
+
/**
* Retrieve the units for a given cell. This probes based on the usual rules
* for cell annotation/column header.
commit 640e4e47758baebdef385f135ab6a07b8c9ada36
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Mon Mar 30 09:50:05 2015 +0200
Implement UtUnit::operator*=
Change-Id: I2733154c3d7c42f3af273a0267158e8fa5957c23
diff --git a/sc/source/core/units/utunit.hxx b/sc/source/core/units/utunit.hxx
index 24225dd..c137f67 100644
--- a/sc/source/core/units/utunit.hxx
+++ b/sc/source/core/units/utunit.hxx
@@ -90,6 +90,11 @@ public:
return UtUnit(ut_multiply(this->get(), rUnit.get()));
}
+ UtUnit& operator*=(const UtUnit& rUnit) {
+ reset(ut_multiply(this->get(), rUnit.get()));
+ return *this;
+ }
+
/**
* Multiply a unit by a given constant.
* This scales the unit by the inverse of that constant,
commit c4e7c77021d92c22e9cec7733f5be6a22de6bfaa
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Mon Mar 30 09:49:27 2015 +0200
Split getUnitForRef into mapping and retrieval.
We will need getUnitForCell when verifying formulas operating on ranges
(where we need to iterate over cells in a range, and retrieve those
cells units - as opposed to retrieving the unit for a reference in a
token array).
Change-Id: I6652057bcb3ee42becb4c5425b06ce544cd400a6
diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx
index 9492dd1..6d280b4 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -314,19 +314,8 @@ bool UnitsImpl::extractUnitFromHeaderString(const OUString& rsHeader, UtUnit& aU
return false;
}
-UtUnit UnitsImpl::getUnitForRef(FormulaToken* pToken, const ScAddress& rFormulaAddress,
- ScDocument* pDoc) {
- assert(pToken->GetType() == formula::svSingleRef);
-
- ScSingleRefData* pRef = pToken->GetSingleRef();
- assert(pRef);
-
- // Addresses can/will be relative to the formula, for extracting
- // units however we will need to get the absolute address (i.e.
- // by adding the current address to the relative formula address).
- const ScAddress aInputAddress = pRef->toAbs( rFormulaAddress );
-
- OUString sUnitString = extractUnitStringForCell(aInputAddress, pDoc);
+UtUnit UnitsImpl::getUnitForCell(const ScAddress& rCellAddress, ScDocument* pDoc) {
+ OUString sUnitString = extractUnitStringForCell(rCellAddress, pDoc);
UtUnit aUnit;
if (sUnitString.getLength() > 0 &&
@@ -336,15 +325,31 @@ UtUnit UnitsImpl::getUnitForRef(FormulaToken* pToken, const ScAddress& rFormulaA
OUString aHeaderUnitString; // Unused -- passed by reference below
ScAddress aHeaderAddress; // Unused too
- UtUnit aHeaderUnit = findHeaderUnitForCell(aInputAddress, pDoc, aHeaderUnitString, aHeaderAddress);
+ UtUnit aHeaderUnit = findHeaderUnitForCell(rCellAddress, pDoc, aHeaderUnitString, aHeaderAddress);
if (aHeaderUnit.isValid())
return aHeaderUnit;
- SAL_INFO("sc.units", "no unit obtained for token at cell " << aInputAddress.GetColRowString());
+ SAL_INFO("sc.units", "no unit obtained for token at cell " << rCellAddress.GetColRowString());
// We return the dimensionless unit 1 if we don't find any other data suggesting a unit.
UtUnit::createUnit("", aUnit, mpUnitSystem);
return aUnit;
+
+}
+
+UtUnit UnitsImpl::getUnitForRef(FormulaToken* pToken, const ScAddress& rFormulaAddress,
+ ScDocument* pDoc) {
+ assert(pToken->GetType() == formula::svSingleRef);
+
+ ScSingleRefData* pRef = pToken->GetSingleRef();
+ assert(pRef);
+
+ // Addresses can/will be relative to the formula, for extracting
+ // units however we will need to get the absolute address (i.e.
+ // by adding the current address to the relative formula address).
+ const ScAddress aCellAddress = pRef->toAbs( rFormulaAddress );
+
+ return getUnitForCell(aCellAddress, pDoc);
}
UtUnit UnitsImpl::findHeaderUnitForCell(const ScAddress& rCellAddress,
diff --git a/sc/source/core/units/unitsimpl.hxx b/sc/source/core/units/unitsimpl.hxx
index 1e8ac8f..91730f7 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -101,6 +101,12 @@ private:
bool extractUnitFromHeaderString(const OUString& rHeader, UtUnit& aUnit, OUString& sUnitString);
+ /**
+ * Retrieve the units for a given cell. This probes based on the usual rules
+ * for cell annotation/column header.
+ * Retrieving units for a formula cell is not yet supported.
+ */
+ UtUnit getUnitForCell(const ScAddress& rCellAddress, ScDocument* pDoc);
UtUnit getUnitForRef(formula::FormulaToken* pToken,
const ScAddress& rFormulaAddress,
ScDocument* pDoc);
commit eccddddee5b9a03c0f32ca8d273d3cf6e5a4709a
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Mon Mar 30 09:46:36 2015 +0200
Typo in comment
Change-Id: Ic6755b950f6b26546072afa82a9ad68caf2becc8
diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx
index 06cd4f3..9492dd1 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -257,7 +257,7 @@ bool UnitsImpl::findUnitInStandardHeader(const OUString& rsHeader, UtUnit& aUnit
bool UnitsImpl::findFreestandingUnitInHeader(const OUString& rsHeader, UtUnit& aUnit, OUString& sUnitString) {
- // We just split the string and test whether each token is either a valid unit in it's own right,
+ // We just split the string and test whether each token is either a valid unit in its own right,
// or is an operator that could glue together multiple units (i.e. multiplication/division).
// This is sufficient for when there are spaces between elements composing the unit, and none
// of the individual elements starts or begins with an operator.
commit 9cdb8f9dc8cdde7b8f26a143a3ad105147d82545
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Sat Mar 21 06:47:24 2015 +0100
Handle constants in formulas for verification too.
Change-Id: Ib1b07e6dabf961726b3411b5ff00c67862d95d03
diff --git a/sc/qa/unit/units.cxx b/sc/qa/unit/units.cxx
index a0d5fa1..af5fff5 100644
--- a/sc/qa/unit/units.cxx
+++ b/sc/qa/unit/units.cxx
@@ -125,7 +125,7 @@ void UnitsTest::testUnitVerification() {
mpDoc->EnsureTable(0);
SvNumberFormatter* pFormatter = mpDoc->GetFormatTable();
- sal_uInt32 nKeyCM, nKeyKG, nKeyS, nKeyCM_S;
+ sal_uInt32 nKeyCM, nKeyM, nKeyKG, nKeyS, nKeyCM_S;
// Used to return position of error in input string for PutEntry
// -- not needed here.
@@ -133,6 +133,8 @@ void UnitsTest::testUnitVerification() {
short nType = css::util::NumberFormat::DEFINED;
+ OUString sM = "#\"m\"";
+ pFormatter->PutEntry(sM, nCheckPos, nType, nKeyM);
OUString sCM = "#\"cm\"";
pFormatter->PutEntry(sCM, nCheckPos, nType, nKeyCM);
OUString sKG = "#\"kg\"";
@@ -186,6 +188,11 @@ void UnitsTest::testUnitVerification() {
mpDoc->SetNumberFormat(address, nKeyCM_S);
mpDoc->SetValue(address, 5);
+ // 5th column: 1m
+ address = ScAddress(4, 0, 0);
+ mpDoc->SetNumberFormat(address, nKeyM);
+ mpDoc->SetValue(address, 1);
+
ScFormulaCell* pCell;
ScTokenArray* pTokens;
@@ -223,6 +230,37 @@ void UnitsTest::testUnitVerification() {
pCell = mpDoc->GetFormulaCell(address);
pTokens = pCell->GetCode();
CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // Test that addition of scaled units works (cm + 100*m)
+ address = ScAddress(0, 10, 0);
+ mpDoc->SetFormula(address, "=A1+100*E1");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+ // 10cm + 100*1m = 110cm
+ CPPUNIT_ASSERT(mpDoc->GetValue(address) == 110);
+
+ // But addition of them unscaled fails (cm + m)
+ address = ScAddress(0, 11, 0);
+ mpDoc->SetFormula(address, "=A1+E1");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // As does wrong scaling (cm+m/50)
+ address = ScAddress(0, 12, 0);
+ mpDoc->SetFormula(address, "=A1+E1/50");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
+ // And scaling doesn't help when adding incompatible units (kg + 100*m)
+ address = ScAddress(0, 13, 0);
+ mpDoc->SetFormula(address, "=B1+100E1");
+ pCell = mpDoc->GetFormulaCell(address);
+ pTokens = pCell->GetCode();
+ CPPUNIT_ASSERT(!mpUnitsImpl->verifyFormula(pTokens, address, mpDoc));
+
}
void UnitsTest::testUnitFromFormatStringExtraction() {
diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx
index 58f4d14..06cd4f3 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -422,6 +422,18 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
break;
}
+ // As far as I can tell this is only used for an actual numerical value
+ // in which case we just want to use it as scaling factor
+ // (for example [m] + [cm]/100 would be a valid operation)
+ case formula::svDouble:
+ {
+ UtUnit aScale;
+ UtUnit::createUnit("", aScale, mpUnitSystem); // Get the dimensionless unit 1
+ aScale = aScale*(pToken->GetDouble());
+
+ aUnitStack.push(aScale);
+ break;
+ }
default:
// We can't parse any other types of tokens yet, so assume that the formula
// was correct.
commit 052c19c26fdc152646a5c476a957aa2b0be5e82e
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Sat Mar 21 06:46:15 2015 +0100
Refactor unit extraction to separate local/header unit extraction
Change-Id: I46eb09fcbb4d83fc58140cda7fada2bf287b0a65
diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx
index 5c9b28c..58f4d14 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -208,7 +208,10 @@ OUString UnitsImpl::extractUnitStringForCell(const ScAddress& rAddress, ScDocume
return extractUnitStringFromFormat(rFormatString);
}
-bool UnitsImpl::extractUnitFromHeaderString(const OUString& rString, UtUnit& aUnit, OUString& sUnitString) {
+bool UnitsImpl::findUnitInStandardHeader(const OUString& rsHeader, UtUnit& aUnit, OUString& sUnitString) {
+ // TODO: we should do a sanity check that there's only one such unit though (and fail if there are multiple).
+ // Since otherwise there's no way for us to know which unit is the intended one, hence we need to get
+ // the user to deconfuse us by correcting their header to only contain the intended unit.
com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> xContext = comphelper::getProcessComponentContext();
uno::Reference<lang::XMultiServiceFactory> xFactory(xContext->getServiceManager(), uno::UNO_QUERY_THROW);
@@ -222,18 +225,14 @@ bool UnitsImpl::extractUnitFromHeaderString(const OUString& rString, UtUnit& aUn
aOptions.algorithmType = util::SearchAlgorithms_REGEXP ;
aOptions.searchFlag = util::SearchFlags::ALL_IGNORE_CASE;
- // 1. We try to check for units contained within square brackets
- // TODO: we should do a sanity check that there's only one such unit though (and fail if there are multiple).
- // Since otherwise there's no way for us to know which unit is the intended one, hence we need to get
- // the user to deconfuse us by correcting their header to only contain the intended unit.
aOptions.searchString = "\\[([^\\]]+)\\]"; // Grab the contents between [ and ].
xSearch->setOptions( aOptions );
util::SearchResult aResult;
- sal_Int32 nStartPosition = rString.getLength();
+ sal_Int32 nStartPosition = rsHeader.getLength();
while (nStartPosition) {
// Search from the back since units are more likely to be at the end of the header.
- aResult = xSearch->searchBackward(rString, nStartPosition, 0);
+ aResult = xSearch->searchBackward(rsHeader, nStartPosition, 0);
// We have either 0 items (no match), or 2 (matched string + the group within)
if (aResult.subRegExpressions != 2) {
@@ -243,7 +242,7 @@ bool UnitsImpl::extractUnitFromHeaderString(const OUString& rString, UtUnit& aUn
// i.e. startOffset is the last character of the intended substring, endOffset the first character.
// We specifically grab the offsets for the first actual regex group, which are stored in [1], the indexes
// at [0] represent the whole matched string (i.e. including square brackets).
- sUnitString = rString.copy(aResult.endOffset[1], aResult.startOffset[1] - aResult.endOffset[1]);
+ sUnitString = rsHeader.copy(aResult.endOffset[1], aResult.startOffset[1] - aResult.endOffset[1]);
if (UtUnit::createUnit(sUnitString, aUnit, mpUnitSystem)) {
return true;
@@ -252,17 +251,26 @@ bool UnitsImpl::extractUnitFromHeaderString(const OUString& rString, UtUnit& aUn
nStartPosition = aResult.endOffset[0];
}
}
+ sUnitString.clear();
+ return false;
+}
+
- // 2. We try to check for any free-standing units by splitting the string and testing each split
- const sal_Int32 nTokenCount = comphelper::string::getTokenCount(rString, ' ');
+bool UnitsImpl::findFreestandingUnitInHeader(const OUString& rsHeader, UtUnit& aUnit, OUString& sUnitString) {
+ // We just split the string and test whether each token is either a valid unit in it's own right,
+ // or is an operator that could glue together multiple units (i.e. multiplication/division).
+ // This is sufficient for when there are spaces between elements composing the unit, and none
+ // of the individual elements starts or begins with an operator.
// There's an inherent limit to how well we can cope with various spacing issues here without
// a ton of computational complexity.
- // E.g. by parsing in this way we might end up with unmatched parentheses which udunits won't like.
- // for now operators have to be completely freestanding i.e. "cm / kg" or completely within a valid unit string e.g. "cm/kg"
+ // E.g. by parsing in this way we might end up with unmatched parentheses which udunits won't like
+ // (thus rejecting the unit) etc.
+
+ const sal_Int32 nTokenCount = comphelper::string::getTokenCount(rsHeader, ' ');
const OUString sOperators = "/*"; // valid
sUnitString.clear();
for (sal_Int32 nToken = 0; nToken < nTokenCount; nToken++) {
- OUString sToken = rString.getToken(nToken, ' ');
+ OUString sToken = rsHeader.getToken(nToken, ' ');
UtUnit aTestUnit;
if (UtUnit::createUnit(sToken, aTestUnit, mpUnitSystem) ||
// Only test for a separator character if we have already got something in our string, as
@@ -285,6 +293,20 @@ bool UnitsImpl::extractUnitFromHeaderString(const OUString& rString, UtUnit& aUn
if (sUnitString.getLength() && UtUnit::createUnit(sUnitString, aUnit, mpUnitSystem)) {
return true;
}
+ sUnitString.clear();
+ return false;
+}
+
+bool UnitsImpl::extractUnitFromHeaderString(const OUString& rsHeader, UtUnit& aUnit, OUString& sUnitString) {
+ // 1. Ideally we have units in a 'standard' format, i.e. enclose in square brackets:
+ if (findUnitInStandardHeader(rsHeader, aUnit, sUnitString)) {
+ return true;
+ }
+
+ // 2. But if not we check for free-standing units
+ if (findFreestandingUnitInHeader(rsHeader, aUnit, sUnitString)) {
+ return true;
+ }
// 3. Give up
aUnit = UtUnit(); // assign invalid
diff --git a/sc/source/core/units/unitsimpl.hxx b/sc/source/core/units/unitsimpl.hxx
index 5616379..1e8ac8f 100644
--- a/sc/source/core/units/unitsimpl.hxx
+++ b/sc/source/core/units/unitsimpl.hxx
@@ -76,7 +76,31 @@ private:
UtUnit getOutputUnitsForOpCode(std::stack< UtUnit >& rUnitStack, const OpCode& rOpCode);
OUString extractUnitStringFromFormat(const OUString& rFormatString);
OUString extractUnitStringForCell(const ScAddress& rAddress, ScDocument* pDoc);
- bool extractUnitFromHeaderString(const OUString& rString, UtUnit& aUnit, OUString& sUnitString);
+
+ /**
+ * Find and extract a Unit in the standard header notation,
+ * i.e. a unit enclose within square brackets (e.g. "length [cm]".
+ *
+ * @return true if such a unit is found.
+ */
+ bool findUnitInStandardHeader(const OUString& rHeader, UtUnit& aUnit, OUString& sUnitString);
+ /**
+ * Find and extract a freestanding Unit from a header string.
+ * This includes strings such as "speed m/s", "speed m / s",
+ * "speed (m/s)" etc.
+ * This is (for now) only a fallback for the case that the user hasn't defined
+ * units in the standard notation, and can only handle a limited number
+ * of ways in which units could be written in a string.
+ * It may turn out that in practice this method should be extended to support
+ * more permutations of the same unit, but this should at least cover the most
+ * obvious cases.
+ *
+ * @ return true if a unit is found.
+ */
+ bool findFreestandingUnitInHeader(const OUString& rHeader, UtUnit& aUnit, OUString& sUnitString);
+
+ bool extractUnitFromHeaderString(const OUString& rHeader, UtUnit& aUnit, OUString& sUnitString);
+
UtUnit getUnitForRef(formula::FormulaToken* pToken,
const ScAddress& rFormulaAddress,
ScDocument* pDoc);
commit b265b5d2d35aacb617c210c19df64a68e9b42320
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Sat Mar 21 06:42:48 2015 +0100
Implement UtUnit::operator* for constants
Change-Id: I9c44f0a4d0b78c3eb3262bd55d8304b7b07460e2
diff --git a/sc/source/core/units/utunit.hxx b/sc/source/core/units/utunit.hxx
index 058891a..24225dd 100644
--- a/sc/source/core/units/utunit.hxx
+++ b/sc/source/core/units/utunit.hxx
@@ -90,6 +90,19 @@ public:
return UtUnit(ut_multiply(this->get(), rUnit.get()));
}
+ /**
+ * Multiply a unit by a given constant.
+ * This scales the unit by the inverse of that constant,
+ * e.g. 100*m is equivalent to cm, hence
+ * the unit needs to be scaled by 1/100.
+ * (This is implemented in this way so that we can simply
+ * multiply units by any constants present in a formula
+ * in a natural way.)
+ */
+ UtUnit operator*(const double nMultiplier) {
+ return UtUnit(ut_scale(1/nMultiplier, this->get()));
+ }
+
UtUnit operator/(const UtUnit& rUnit) {
// the parameter is the right hand side value in the operation,
// i.e. we are working with this / rUnit.
commit 669f6861c6d9f2e60ea51accb1cc4c1fc99c66b2
Author: Andrzej Hunt <andrzej.hunt at collabora.com>
Date: Wed Mar 18 12:28:33 2015 +0000
Turn while into for for clarity.
Change-Id: I6e55441c900b64af014fe902c9c8bf928aa7e31a
diff --git a/sc/source/core/units/unitsimpl.cxx b/sc/source/core/units/unitsimpl.cxx
index 427aec3..5c9b28c 100644
--- a/sc/source/core/units/unitsimpl.cxx
+++ b/sc/source/core/units/unitsimpl.cxx
@@ -366,9 +366,7 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
stack< UtUnit > aUnitStack;
- FormulaToken* pToken = pArray->FirstRPN();
-
- while (pToken != 0) {
+ for (FormulaToken* pToken = pArray->FirstRPN(); pToken != 0; pToken = pArray->NextRPN()) {
switch (pToken->GetType()) {
case formula::svSingleRef:
{
@@ -409,8 +407,6 @@ bool UnitsImpl::verifyFormula(ScTokenArray* pArray, const ScAddress& rFormulaAdd
SAL_WARN("sc.units", "Unrecognised token type " << pToken->GetType());
return true;
}
-
- pToken = pArray->NextRPN();
}
// TODO: only fail if actual parsing fails?
More information about the Libreoffice-commits
mailing list