From cf4eaa3d1fc692d03697b87ce53f536171a3967d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Mon, 15 Jun 2020 15:39:04 +0200 Subject: [PATCH] [apps/poincare] Use symbolicComputation in recursivelyMatches This fixes a failed assertion for the scenario: [3]->x then, in the Equation app, solve x+1->0 --- apps/calculation/calculation.cpp | 2 +- apps/solver/equation.cpp | 5 ++-- apps/solver/equation_store.cpp | 2 +- poincare/include/poincare/expression.h | 2 +- poincare/include/poincare/expression_node.h | 3 ++- poincare/src/addition.cpp | 4 +-- poincare/src/expression.cpp | 29 ++++++++++++++++----- poincare/src/function.cpp | 3 +++ poincare/src/multiplication.cpp | 4 +-- poincare/src/symbol.cpp | 4 ++- poincare/src/symbol_abstract.cpp | 6 +++-- poincare/test/context.cpp | 4 +-- poincare/test/expression_properties.cpp | 5 ++-- poincare/test/helper.h | 1 + 14 files changed, 49 insertions(+), 25 deletions(-) diff --git a/apps/calculation/calculation.cpp b/apps/calculation/calculation.cpp index ba3d46532..7a7a9e425 100644 --- a/apps/calculation/calculation.cpp +++ b/apps/calculation/calculation.cpp @@ -176,7 +176,7 @@ Calculation::DisplayOutput Calculation::displayOutput(Context * context) { ExpressionNode::Type::PredictionInterval }; return e.isOfType(approximateOnlyTypes, sizeof(approximateOnlyTypes)/sizeof(ExpressionNode::Type)); - }, context, true) + }, context) ) { m_displayOutput = DisplayOutput::ApproximateOnly; diff --git a/apps/solver/equation.cpp b/apps/solver/equation.cpp index 9c72f454e..80d2a4cab 100644 --- a/apps/solver/equation.cpp +++ b/apps/solver/equation.cpp @@ -15,7 +15,7 @@ using namespace Shared; namespace Solver { bool Equation::containsIComplex(Context * context) const { - return expressionClone().recursivelyMatches([](const Expression e, Context * context) { return e.type() == ExpressionNode::Type::Constant && static_cast(e).isIComplex(); }, context, true); + return expressionClone().recursivelyMatches([](const Expression e, Context * context) { return e.type() == ExpressionNode::Type::Constant && static_cast(e).isIComplex(); }, context); } Expression Equation::Model::standardForm(const Storage::Record * record, Context * context, bool replaceFunctionsButNotSymbols) const { @@ -44,8 +44,7 @@ Expression Equation::Model::standardForm(const Storage::Record * record, Context [](const Expression e, Context * context) { return e.type() == ExpressionNode::Type::Undefined || e.type() == ExpressionNode::Type::Infinity || Expression::IsMatrix(e, context); }, - contextToUse, - true)) + contextToUse)) { *returnedExpression = Undefined::Builder(); } else if (expressionRed.type() == ExpressionNode::Type::Equal) { diff --git a/apps/solver/equation_store.cpp b/apps/solver/equation_store.cpp index bdd4b59b6..867a19d7c 100644 --- a/apps/solver/equation_store.cpp +++ b/apps/solver/equation_store.cpp @@ -156,7 +156,7 @@ EquationStore::Error EquationStore::privateExactSolve(Poincare::Context * contex } const Expression e = eq->standardForm(context, replaceFunctionsButNotSymbols); // The standard form is memoized so there is no double computation even if replaceFunctionsButNotSymbols is true. - if (e.isUninitialized() || e.type() == ExpressionNode::Type::Undefined) { + if (e.isUninitialized() || e.type() == ExpressionNode::Type::Undefined || e.recursivelyMatches(Expression::IsMatrix, context, replaceFunctionsButNotSymbols ? ExpressionNode::SymbolicComputation::ReplaceDefinedFunctionsWithDefinitions : ExpressionNode::SymbolicComputation::ReplaceAllDefinedSymbolsWithDefinition)) { return Error::EquationUndefined; } if (e.type() == ExpressionNode::Type::Unreal) { diff --git a/poincare/include/poincare/expression.h b/poincare/include/poincare/expression.h index 14def7d80..6bdb4008e 100644 --- a/poincare/include/poincare/expression.h +++ b/poincare/include/poincare/expression.h @@ -151,7 +151,7 @@ public: bool isDivisionOfIntegers() const; bool hasDefinedComplexApproximation(Context * context, Preferences::ComplexFormat complexFormat, Preferences::AngleUnit angleUnit) const; typedef bool (*ExpressionTest)(const Expression e, Context * context); - bool recursivelyMatches(ExpressionTest test, Context * context, bool replaceSymbols = true) const; + bool recursivelyMatches(ExpressionTest test, Context * context, ExpressionNode::SymbolicComputation replaceSymbols = ExpressionNode::SymbolicComputation::ReplaceAllDefinedSymbolsWithDefinition) const; typedef bool (*ExpressionTypeTest)(const Expression e, const void * context); bool hasExpression(ExpressionTypeTest test, const void * context) const; // WARNING: this method must be called on reduced (sorted) expressions diff --git a/poincare/include/poincare/expression_node.h b/poincare/include/poincare/expression_node.h index ef9f6a81a..2d3b33a33 100644 --- a/poincare/include/poincare/expression_node.h +++ b/poincare/include/poincare/expression_node.h @@ -128,7 +128,8 @@ public: ReplaceAllSymbolsWithDefinitionsOrUndefined = 0, ReplaceAllDefinedSymbolsWithDefinition = 1, ReplaceDefinedFunctionsWithDefinitions = 2, - ReplaceAllSymbolsWithUndefined = 3 // Used in UnitConvert::shallowReduce + ReplaceAllSymbolsWithUndefined = 3, // Used in UnitConvert::shallowReduce + DoNotReplaceAnySymbol = 4 }; enum class UnitConversion { None = 0, diff --git a/poincare/src/addition.cpp b/poincare/src/addition.cpp index 076c054b7..141cf5a5f 100644 --- a/poincare/src/addition.cpp +++ b/poincare/src/addition.cpp @@ -355,7 +355,7 @@ bool Addition::TermsHaveIdenticalNonNumeralFactors(const Expression & e1, const int numberOfNonNumeralFactors = numberOfNonNumeralFactorsInE1; if (numberOfNonNumeralFactors == 1) { Expression nonNumeralFactor = FirstNonNumeralFactor(e1); - if (nonNumeralFactor.recursivelyMatches(Expression::IsRandom, context, true)) { + if (nonNumeralFactor.recursivelyMatches(Expression::IsRandom, context)) { return false; } return FirstNonNumeralFactor(e1).isIdenticalTo(FirstNonNumeralFactor(e2)); @@ -380,7 +380,7 @@ Expression Addition::factorizeOnCommonDenominator(ExpressionNode::ReductionConte Expression childI = childAtIndex(i); Expression currentDenominator = childI.denominator(reductionContext); if (!currentDenominator.isUninitialized()) { - if (currentDenominator.recursivelyMatches(Expression::IsRandom, reductionContext.context(), true)) { + if (currentDenominator.recursivelyMatches(Expression::IsRandom, reductionContext.context())) { // Remove "random" factors removeChildInPlace(childI, childI.numberOfChildren()); a.addChildAtIndexInPlace(childI, a.numberOfChildren(), a.numberOfChildren()); diff --git a/poincare/src/expression.cpp b/poincare/src/expression.cpp index 1fb8778c4..41b9b04bd 100644 --- a/poincare/src/expression.cpp +++ b/poincare/src/expression.cpp @@ -87,14 +87,29 @@ bool Expression::isRationalOne() const { return type() == ExpressionNode::Type::Rational && convert().isOne(); } -bool Expression::recursivelyMatches(ExpressionTest test, Context * context, bool replaceSymbols) const { +bool Expression::recursivelyMatches(ExpressionTest test, Context * context, ExpressionNode::SymbolicComputation replaceSymbols) const { if (test(*this, context)) { return true; } + + // Handle symbols and functions ExpressionNode::Type t = type(); - if (replaceSymbols && (t == ExpressionNode::Type::Symbol || t == ExpressionNode::Type::Function)) { - return SymbolAbstract::matches(convert(), test, context); + if (t == ExpressionNode::Type::Symbol || t == ExpressionNode::Type::Function) { + assert(replaceSymbols == ExpressionNode::SymbolicComputation::ReplaceAllDefinedSymbolsWithDefinition + || replaceSymbols == ExpressionNode::SymbolicComputation::ReplaceDefinedFunctionsWithDefinitions + || replaceSymbols == ExpressionNode::SymbolicComputation::DoNotReplaceAnySymbol); // We need only those cases for now + + if (replaceSymbols == ExpressionNode::SymbolicComputation::DoNotReplaceAnySymbol + || (replaceSymbols == ExpressionNode::SymbolicComputation::ReplaceDefinedFunctionsWithDefinitions + && t == ExpressionNode::Type::Symbol)) + { + return false; + } + assert(replaceSymbols == ExpressionNode::SymbolicComputation::ReplaceAllDefinedSymbolsWithDefinition + || t == ExpressionNode::Type::Function); + return SymbolAbstract::matches(convert(), test, context); } + const int childrenCount = this->numberOfChildren(); for (int i = 0; i < childrenCount; i++) { if (childAtIndex(i).recursivelyMatches(test, context, replaceSymbols)) { @@ -197,7 +212,7 @@ bool containsVariables(const Expression e, char * variables, int maxVariableSize } bool Expression::getLinearCoefficients(char * variables, int maxVariableSize, Expression coefficients[], Expression constant[], Context * context, Preferences::ComplexFormat complexFormat, Preferences::AngleUnit angleUnit, ExpressionNode::SymbolicComputation symbolicComputation) const { - assert(!recursivelyMatches(IsMatrix, context, true)); + assert(!recursivelyMatches(IsMatrix, context, symbolicComputation)); // variables is in fact of type char[k_maxNumberOfVariables][maxVariableSize] int index = 0; while (variables[index*maxVariableSize] != 0) { @@ -223,7 +238,7 @@ bool Expression::getLinearCoefficients(char * variables, int maxVariableSize, Ex /* degree is supposed to be 0 or 1. Otherwise, it means that equation * is 'undefined' due to the reduction of 0*inf for example. * (ie, x*y*inf = 0) */ - assert(!recursivelyMatches([](const Expression e, Context * context) { return e.isUndefined(); }, context, true)); + assert(!recursivelyMatches([](const Expression e, Context * context) { return e.isUndefined(); }, context)); return false; } /* The equation is can be written: a_1*x+a_0 with a_1 and a_0 x-independent. @@ -473,7 +488,7 @@ int Expression::getPolynomialReducedCoefficients(const char * symbolName, Expres /* Units */ bool Expression::hasUnit() const { - return recursivelyMatches([](const Expression e, Context * context) { return e.type() == ExpressionNode::Type::Unit; }, nullptr, false); + return recursivelyMatches([](const Expression e, Context * context) { return e.type() == ExpressionNode::Type::Unit; }, nullptr, ExpressionNode::SymbolicComputation::DoNotReplaceAnySymbol); } /* Complex */ @@ -494,7 +509,7 @@ Preferences::ComplexFormat Expression::UpdatedComplexFormatWithTextInput(Prefere } Preferences::ComplexFormat Expression::UpdatedComplexFormatWithExpressionInput(Preferences::ComplexFormat complexFormat, const Expression & exp, Context * context) { - if (complexFormat == Preferences::ComplexFormat::Real && exp.recursivelyMatches([](const Expression e, Context * context) { return e.type() == ExpressionNode::Type::Constant && static_cast(e).isIComplex(); }, context, true)) { + if (complexFormat == Preferences::ComplexFormat::Real && exp.recursivelyMatches([](const Expression e, Context * context) { return e.type() == ExpressionNode::Type::Constant && static_cast(e).isIComplex(); }, context)) { return Preferences::ComplexFormat::Cartesian; } return complexFormat; diff --git a/poincare/src/function.cpp b/poincare/src/function.cpp index 6aa0695d6..ae5764af2 100644 --- a/poincare/src/function.cpp +++ b/poincare/src/function.cpp @@ -120,6 +120,9 @@ Expression Function::shallowReduce(ExpressionNode::ReductionContext reductionCon { return replaceWithUndefinedInPlace(); } + if (reductionContext.symbolicComputation() == ExpressionNode::SymbolicComputation::DoNotReplaceAnySymbol) { + return *this; + } Expression result = SymbolAbstract::Expand(*this, reductionContext.context(), true, reductionContext.symbolicComputation()); if (result.isUninitialized()) { if (reductionContext.symbolicComputation() != ExpressionNode::SymbolicComputation::ReplaceAllSymbolsWithDefinitionsOrUndefined) { diff --git a/poincare/src/multiplication.cpp b/poincare/src/multiplication.cpp index 0f6e806d8..509e2696f 100644 --- a/poincare/src/multiplication.cpp +++ b/poincare/src/multiplication.cpp @@ -598,7 +598,7 @@ Expression Multiplication::privateShallowReduce(ExpressionNode::ReductionContext while (i < numberOfChildren()-1) { Expression oi = childAtIndex(i); Expression oi1 = childAtIndex(i+1); - if (oi.recursivelyMatches(Expression::IsRandom, context, true)) { + if (oi.recursivelyMatches(Expression::IsRandom, context)) { // Do not factorize random or randint } else if (TermsHaveIdenticalBase(oi, oi1)) { bool shouldFactorizeBase = true; @@ -715,7 +715,7 @@ Expression Multiplication::privateShallowReduce(ExpressionNode::ReductionContext * reduce expressions such as (x+y)^(-1)*(x+y)(a+b). * If there is a random somewhere, do not expand. */ Expression p = parent(); - bool hasRandom = recursivelyMatches(Expression::IsRandom, context, true); + bool hasRandom = recursivelyMatches(Expression::IsRandom, context); if (shouldExpand && (p.isUninitialized() || p.type() != ExpressionNode::Type::Multiplication) && !hasRandom) diff --git a/poincare/src/symbol.cpp b/poincare/src/symbol.cpp index 3a2a47c50..b1f1f1687 100644 --- a/poincare/src/symbol.cpp +++ b/poincare/src/symbol.cpp @@ -151,7 +151,9 @@ bool Symbol::isRegressionSymbol(const char * c, Poincare::Context * context) { } Expression Symbol::shallowReduce(ExpressionNode::ReductionContext reductionContext) { - if (reductionContext.symbolicComputation() == ExpressionNode::SymbolicComputation::ReplaceDefinedFunctionsWithDefinitions) { + if (reductionContext.symbolicComputation() == ExpressionNode::SymbolicComputation::ReplaceDefinedFunctionsWithDefinitions + || reductionContext.symbolicComputation() == ExpressionNode::SymbolicComputation::DoNotReplaceAnySymbol) + { return *this; } if (reductionContext.symbolicComputation() == ExpressionNode::SymbolicComputation::ReplaceAllSymbolsWithUndefined) { diff --git a/poincare/src/symbol_abstract.cpp b/poincare/src/symbol_abstract.cpp index 2d8b31f8b..61c679767 100644 --- a/poincare/src/symbol_abstract.cpp +++ b/poincare/src/symbol_abstract.cpp @@ -62,12 +62,14 @@ size_t SymbolAbstract::TruncateExtension(char * dst, const char * src, size_t le bool SymbolAbstract::matches(const SymbolAbstract & symbol, ExpressionTest test, Context * context) { Expression e = SymbolAbstract::Expand(symbol, context, false); - return !e.isUninitialized() && e.recursivelyMatches(test, context, false); + return !e.isUninitialized() && e.recursivelyMatches(test, context, ExpressionNode::SymbolicComputation::DoNotReplaceAnySymbol); } Expression SymbolAbstract::Expand(const SymbolAbstract & symbol, Context * context, bool clone, ExpressionNode::SymbolicComputation symbolicComputation) { bool shouldNotReplaceSymbols = symbolicComputation == ExpressionNode::SymbolicComputation::ReplaceDefinedFunctionsWithDefinitions; - if (symbol.type() == ExpressionNode::Type::Symbol && shouldNotReplaceSymbols) { + if (symbolicComputation == ExpressionNode::SymbolicComputation::DoNotReplaceAnySymbol + || (symbol.type() == ExpressionNode::Type::Symbol && shouldNotReplaceSymbols)) + { return clone ? symbol.clone() : *const_cast(&symbol); } Expression e = context->expressionForSymbolAbstract(symbol, clone); diff --git a/poincare/test/context.cpp b/poincare/test/context.cpp index 9137d02df..b8b8d64dd 100644 --- a/poincare/test/context.cpp +++ b/poincare/test/context.cpp @@ -205,7 +205,7 @@ QUIZ_CASE(poincare_context_user_variable_properties) { quiz_assert(Symbol::Builder('a').recursivelyMatches(Expression::IsMatrix, &context)); assert_expression_approximates_to("1.2→b", "1.2"); - quiz_assert(Symbol::Builder('b').recursivelyMatches(Expression::IsApproximate, &context, true)); + quiz_assert(Symbol::Builder('b').recursivelyMatches(Expression::IsApproximate, &context)); /* [[x]]→f(x) expression contains a matrix, so its simplification is going * to be interrupted. We thus rather approximate it instead of simplifying it. @@ -214,7 +214,7 @@ QUIZ_CASE(poincare_context_user_variable_properties) { assert_expression_approximates_to("[[x]]→f(x)", "[[undef]]"); quiz_assert(Function::Builder("f", 1, Symbol::Builder('x')).recursivelyMatches(Poincare::Expression::IsMatrix, &context)); assert_expression_approximates_to("0.2*x→g(x)", "undef"); - quiz_assert(Function::Builder("g", 1, Rational::Builder(2)).recursivelyMatches(Expression::IsApproximate, &context, true)); + quiz_assert(Function::Builder("g", 1, Rational::Builder(2)).recursivelyMatches(Expression::IsApproximate, &context)); // Clean the storage for other tests Ion::Storage::sharedStorage()->recordNamed("a.exp").destroy(); diff --git a/poincare/test/expression_properties.cpp b/poincare/test/expression_properties.cpp index c595aa0d5..90b986427 100644 --- a/poincare/test/expression_properties.cpp +++ b/poincare/test/expression_properties.cpp @@ -35,12 +35,12 @@ QUIZ_CASE(poincare_properties_is_parametered_expression) { void assert_expression_has_property(const char * expression, Context * context, Expression::ExpressionTest test) { Expression e = parse_expression(expression, context, false); - quiz_assert_print_if_failure(e.recursivelyMatches(test, context, true), expression); + quiz_assert_print_if_failure(e.recursivelyMatches(test, context), expression); } void assert_expression_has_not_property(const char * expression, Context * context, Expression::ExpressionTest test) { Expression e = parse_expression(expression, context, false); - quiz_assert_print_if_failure(!e.recursivelyMatches(test, context, true), expression); + quiz_assert_print_if_failure(!e.recursivelyMatches(test, context), expression); } QUIZ_CASE(poincare_properties_is_approximate) { @@ -360,6 +360,7 @@ QUIZ_CASE(poincare_properties_get_polynomial_coefficients) { const char * coefficient7[] = {"4", 0}; assert_reduced_expression_has_polynomial_coefficient("x+1", "x", coefficient7 ); const char * coefficient8[] = {"2", "1", 0}; + assert_reduced_expression_has_polynomial_coefficient("x+2", "x", coefficient8, Real, Radian, DoNotReplaceAnySymbol); assert_reduced_expression_has_polynomial_coefficient("x+2", "x", coefficient8, Real, Radian, ReplaceDefinedFunctionsWithDefinitions); assert_reduced_expression_has_polynomial_coefficient("f(x)", "x", coefficient4, Cartesian, Radian, ReplaceDefinedFunctionsWithDefinitions); diff --git a/poincare/test/helper.h b/poincare/test/helper.h index 229ef4274..6ef23c361 100644 --- a/poincare/test/helper.h +++ b/poincare/test/helper.h @@ -12,6 +12,7 @@ constexpr Poincare::ExpressionNode::SymbolicComputation ReplaceAllDefinedSymbols constexpr Poincare::ExpressionNode::SymbolicComputation ReplaceAllSymbolsWithDefinitionsOrUndefined = Poincare::ExpressionNode::SymbolicComputation::ReplaceAllSymbolsWithDefinitionsOrUndefined; constexpr Poincare::ExpressionNode::SymbolicComputation ReplaceDefinedFunctionsWithDefinitions = Poincare::ExpressionNode::SymbolicComputation::ReplaceDefinedFunctionsWithDefinitions; constexpr Poincare::ExpressionNode::SymbolicComputation ReplaceAllSymbolsWithUndefined = Poincare::ExpressionNode::SymbolicComputation::ReplaceAllSymbolsWithUndefined; +constexpr Poincare::ExpressionNode::SymbolicComputation DoNotReplaceAnySymbol = Poincare::ExpressionNode::SymbolicComputation::DoNotReplaceAnySymbol; constexpr Poincare::ExpressionNode::UnitConversion NoUnitConversion = Poincare::ExpressionNode::UnitConversion::None; constexpr Poincare::ExpressionNode::UnitConversion DefaultUnitConversion = Poincare::ExpressionNode::UnitConversion::Default; constexpr Poincare::ExpressionNode::UnitConversion InternationalSystemUnitConversion = Poincare::ExpressionNode::UnitConversion::InternationalSystem;