From 58c995d63c379d3be152ee1432b651ca810bebfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Fri, 9 Nov 2018 13:31:58 +0100 Subject: [PATCH] [poincare] Symbol replacements limit for functions --- poincare/include/poincare/expression.h | 3 ++ poincare/include/poincare/symbol_abstract.h | 1 - poincare/src/expression.cpp | 18 ++++++++ poincare/src/function.cpp | 10 +++- poincare/src/symbol.cpp | 51 +++++---------------- 5 files changed, 40 insertions(+), 43 deletions(-) diff --git a/poincare/include/poincare/expression.h b/poincare/include/poincare/expression.h index 5b0fbae67..ab69edf76 100644 --- a/poincare/include/poincare/expression.h +++ b/poincare/include/poincare/expression.h @@ -172,6 +172,7 @@ public: void reduceChildren(Context & context, Preferences::AngleUnit angleUnit, bool replaceSymbols) { return node()->reduceChildren(context, angleUnit, replaceSymbols); } + static Expression ExpressionWithoutSymbols(Expression expressionWithSymbols, Context & context); /* Approximation Helper */ template static U epsilon(); @@ -256,6 +257,8 @@ protected: Expression setSign(ExpressionNode::Sign s, Context & context, Preferences::AngleUnit angleUnit); private: + static constexpr int k_maxSymbolReplacementsCount = 30; + /* Simplification */ Expression deepReduce(Context & context, Preferences::AngleUnit angleUnit, bool replaceSymbols = true); void deepReduceChildren(Context & context, Preferences::AngleUnit angleUnit, bool replaceSymbols) { diff --git a/poincare/include/poincare/symbol_abstract.h b/poincare/include/poincare/symbol_abstract.h index f947c2fe5..a8ead1f62 100644 --- a/poincare/include/poincare/symbol_abstract.h +++ b/poincare/include/poincare/symbol_abstract.h @@ -63,7 +63,6 @@ public: } constexpr static size_t k_maxNameSize = 8; protected: - static constexpr int k_maxReplacementsCount = 100; SymbolAbstract(const SymbolAbstractNode * node) : Expression(node) {} SymbolAbstractNode * node() const { return static_cast(Expression::node()); } private: diff --git a/poincare/src/expression.cpp b/poincare/src/expression.cpp index 1117c2bbd..7f85b0e41 100644 --- a/poincare/src/expression.cpp +++ b/poincare/src/expression.cpp @@ -322,6 +322,24 @@ Expression Expression::simplify(Context & context, Preferences::AngleUnit angleU return e; } +Expression Expression::ExpressionWithoutSymbols(Expression e, Context & context) { + if (e.isUninitialized()) { + return e; + } + /* Replace all the symbols iteratively. If we make too many replacements, the + * symbols are likely to be defined circularly, in which case we return an + * uninitialized expression.*/ + int replacementCount = 0; + while (e.hasSymbols(context)) { + replacementCount++; + if (replacementCount > k_maxSymbolReplacementsCount) { + return Expression(); + } + e = e.replaceSymbols(context); + } + return e; +} + Expression Expression::reduce(Context & context, Preferences::AngleUnit angleUnit, bool replaceSymbols) { return deepReduce(context, angleUnit, replaceSymbols); } diff --git a/poincare/src/function.cpp b/poincare/src/function.cpp index 2f61b5496..8655e91a3 100644 --- a/poincare/src/function.cpp +++ b/poincare/src/function.cpp @@ -15,16 +15,18 @@ Expression FunctionNode::replaceSymbolWithExpression(const SymbolAbstract & symb int FunctionNode::polynomialDegree(Context & context, const char * symbolName) const { Expression e = context.expressionForSymbol(Function(this)); + e = Expression::ExpressionWithoutSymbols(e, context); if (e.isUninitialized()) { return -1; } -/*TODO Context should be float or double ?*/ + // TODO Context should be float or double ? VariableContext newContext = xContext(context); return e.polynomialDegree(newContext, symbolName); } int FunctionNode::getPolynomialCoefficients(Context & context, const char * symbolName, Expression coefficients[]) const { Expression e = context.expressionForSymbol(Function(this)); + e = Expression::ExpressionWithoutSymbols(e, context); if (e.isUninitialized()) { return -1; } @@ -34,6 +36,7 @@ int FunctionNode::getPolynomialCoefficients(Context & context, const char * symb int FunctionNode::getVariables(Context & context, isVariableTest isVariable, char * variables, int maxSizeVariable) const { Expression e = context.expressionForSymbol(Function(this)); + e = Expression::ExpressionWithoutSymbols(e, context); if (e.isUninitialized()) { return 0; } @@ -43,6 +46,7 @@ int FunctionNode::getVariables(Context & context, isVariableTest isVariable, cha float FunctionNode::characteristicXRange(Context & context, Preferences::AngleUnit angleUnit) const { Expression e = context.expressionForSymbol(Function(this)); + e = Expression::ExpressionWithoutSymbols(e, context); if (e.isUninitialized()) { return 0.0f; } @@ -80,6 +84,7 @@ Evaluation FunctionNode::approximate(DoublePrecision p, Context& context template Evaluation FunctionNode::templatedApproximate(Context& context, Preferences::AngleUnit angleUnit) const { Expression e = context.expressionForSymbol(Function(this)); + e = Expression::ExpressionWithoutSymbols(e, context); if (e.isUninitialized()) { return Complex::Undefined(); } @@ -116,7 +121,8 @@ Expression Function::shallowReduce(Context & context, Preferences::AngleUnit ang if (!replaceSymbols) { return *this; } - const Expression e = context.expressionForSymbol(*this); + Expression e = context.expressionForSymbol(*this); + e = ExpressionWithoutSymbols(e, context); if (!e.isUninitialized()) { // We need to replace the unknown with the child Expression result = e.clone(); diff --git a/poincare/src/symbol.cpp b/poincare/src/symbol.cpp index c4289077d..5985e4634 100644 --- a/poincare/src/symbol.cpp +++ b/poincare/src/symbol.cpp @@ -127,20 +127,10 @@ Expression SymbolNode::replaceSymbols(Context & context) { template Evaluation SymbolNode::templatedApproximate(Context& context, Preferences::AngleUnit angleUnit) const { Expression e = context.expressionForSymbol(Symbol(this)); + e = Expression::ExpressionWithoutSymbols(e, context); if (e.isUninitialized()) { return Complex::Undefined(); } - - int replacementCount = 0; - /* First, replace all the symbols iteratively. This prevents a memory - * failure symbols are defined circularly. */ - while (e.hasSymbols(context)) { - replacementCount++; - if (replacementCount > Symbol::k_maxReplacementsCount) { - return Complex::Undefined(); - } - e = e.replaceSymbols(context); - } return e.approximateToEvaluation(context, angleUnit); } @@ -168,18 +158,7 @@ bool Symbol::isRegressionSymbol(const char * c) { bool Symbol::matches(ExpressionTest test, Context & context) const { Expression e = context.expressionForSymbol(*this); - if (!e.isUninitialized()) { - /* First, replace all the symbols iteratively. This prevents a memory - * failure symbols are defined circularly. */ - int replacementCount = 0; - while (e.hasSymbols(context)) { - replacementCount++; - if (replacementCount > k_maxReplacementsCount) { - return false; //TODO - } - e = e.replaceSymbols(context); - } - } + e = ExpressionWithoutSymbols(e, context); return !e.isUninitialized() && test(e, context); } @@ -188,24 +167,16 @@ Expression Symbol::shallowReduce(Context & context, Preferences::AngleUnit angle if (!replaceSymbols) { return *this; } - const Expression e = context.expressionForSymbol(*this); - if (!e.isUninitialized()) { - // The stored expression is beautified, so we need to call reduce - Expression result = e; - /* First, replace all the symbols iteratively. This prevents a memory - * failure symbols are defined circularly. */ - int replacementCount = 0; - while (result.hasSymbols(context)) { - replacementCount++; - if (replacementCount > k_maxReplacementsCount) { - return *this; - } - result = result.replaceSymbols(context); - } - replaceWithInPlace(result); - return result.deepReduce(context, angleUnit); + Expression result = context.expressionForSymbol(*this); + // The stored expression is beautified, so we need to call reduce + /* First, replace all the symbols iteratively. This prevents a memory + * failure symbols are defined circularly. */ + result = ExpressionWithoutSymbols(result, context); + if (result.isUninitialized()) { + return *this; } - return *this; + replaceWithInPlace(result); + return result.deepReduce(context, angleUnit); } Expression Symbol::replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression) {