From 64c76888666448deb00e31ce04d6b5f96b26ec0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89milie=20Feral?= Date: Tue, 31 Mar 2020 11:22:47 +0200 Subject: [PATCH 1/6] [.github] workflow: update to actions/checkout@v2 --- .github/workflows/ci-workflow.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci-workflow.yml b/.github/workflows/ci-workflow.yml index 408a899d7..ce71e4b32 100644 --- a/.github/workflows/ci-workflow.yml +++ b/.github/workflows/ci-workflow.yml @@ -5,7 +5,7 @@ jobs: build-simulator-android: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - run: make -j2 PLATFORM=simulator TARGET=android - uses: actions/upload-artifact@master with: @@ -16,7 +16,7 @@ jobs: steps: - run: sudo apt-get install build-essential imagemagick libfreetype6-dev libjpeg-dev libpng-dev pkg-config - uses: numworks/setup-arm-toolchain@v1 - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - run: make -j2 MODEL=n0100 epsilon.dfu - run: make -j2 MODEL=n0100 epsilon.onboarding.dfu - run: make -j2 MODEL=n0100 epsilon.official.onboarding.dfu @@ -34,7 +34,7 @@ jobs: steps: - run: sudo apt-get install build-essential imagemagick libfreetype6-dev libjpeg-dev libpng-dev pkg-config - uses: numworks/setup-arm-toolchain@v1 - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - run: make -j2 epsilon.dfu - run: make -j2 epsilon.onboarding.dfu - run: make -j2 epsilon.official.onboarding.dfu @@ -53,7 +53,7 @@ jobs: runs-on: windows-latest steps: - uses: numworks/setup-msys2@v1 - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - run: msys2do pacman -S --noconfirm mingw-w64-x86_64-gcc mingw-w64-x86_64-freetype mingw-w64-x86_64-pkg-config make mingw-w64-x86_64-python3 mingw-w64-x86_64-libjpeg-turbo mingw-w64-x86_64-libpng - run: msys2do make -j2 PLATFORM=simulator - uses: actions/upload-artifact@master @@ -68,7 +68,7 @@ jobs: - uses: numworks/setup-emscripten@v1 with: sdk: latest-fastcomp - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - run: make -j2 PLATFORM=simulator TARGET=web - run: make -j2 PLATFORM=simulator TARGET=web epsilon.official.js - uses: actions/upload-artifact@master @@ -81,7 +81,7 @@ jobs: runs-on: ubuntu-latest steps: - run: sudo apt-get install build-essential imagemagick libfreetype6-dev libjpeg-dev libpng-dev pkg-config - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - run: make -j2 PLATFORM=simulator - uses: actions/upload-artifact@master with: @@ -93,7 +93,7 @@ jobs: runs-on: macOS-latest steps: - run: brew install numworks/tap/epsilon-sdk - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - run: make -j2 PLATFORM=simulator - uses: actions/upload-artifact@master with: @@ -105,7 +105,7 @@ jobs: runs-on: macOS-latest steps: - run: brew install numworks/tap/epsilon-sdk - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - run: make -j2 PLATFORM=simulator TARGET=ios EPSILON_TELEMETRY=0 - uses: actions/upload-artifact@master with: From 764cf1087ac255deb7fab43a829ae7f3bb2a6bee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Tue, 31 Mar 2020 10:34:27 +0200 Subject: [PATCH 2/6] [apps/solver] Fix EquationStore::resolveLinearSystem Scenario: solve a system with the maximum number of variables -> our routine to compute the number of variables did not use k_maxNumberOfVariables, so we ended up adding the same children to the matrix Ab, which created ghosts, which made the deep reduction crash. --- apps/solver/equation_store.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/apps/solver/equation_store.cpp b/apps/solver/equation_store.cpp index d9a227574..a490f4ad7 100644 --- a/apps/solver/equation_store.cpp +++ b/apps/solver/equation_store.cpp @@ -211,7 +211,16 @@ EquationStore::Error EquationStore::privateExactSolve(Poincare::Context * contex // Step 3. Polynomial & Monovariable? assert(numberOfVariables == 1 && numberOfDefinedModels() == 1); Expression polynomialCoefficients[Expression::k_maxNumberOfPolynomialCoefficients]; - int degree = modelForRecord(definedRecordAtIndex(0))->standardForm(context, replaceFunctionsButNotSymbols).getPolynomialReducedCoefficients(m_variables[0], polynomialCoefficients, context, updatedComplexFormat(context), preferences->angleUnit(), replaceFunctionsButNotSymbols ? ExpressionNode::SymbolicComputation::ReplaceDefinedFunctionsWithDefinitions : ExpressionNode::SymbolicComputation::ReplaceAllDefinedSymbolsWithDefinition); + int degree = modelForRecord(definedRecordAtIndex(0))->standardForm(context, replaceFunctionsButNotSymbols) + .getPolynomialReducedCoefficients( + m_variables[0], + polynomialCoefficients, + context, + updatedComplexFormat(context), + preferences->angleUnit(), + replaceFunctionsButNotSymbols ? + ExpressionNode::SymbolicComputation::ReplaceDefinedFunctionsWithDefinitions : + ExpressionNode::SymbolicComputation::ReplaceAllDefinedSymbolsWithDefinition); if (degree == 2) { // Polynomial degree <= 2 m_type = Type::PolynomialMonovariable; @@ -262,7 +271,9 @@ EquationStore::Error EquationStore::resolveLinearSystem(Expression exactSolution Preferences::AngleUnit angleUnit = Preferences::sharedPreferences()->angleUnit(); // n unknown variables int n = 0; - while (m_variables[n][0] != 0) { n++; } + while (n < Expression::k_maxNumberOfVariables && m_variables[n][0] != 0) { + n++; + } int m = numberOfDefinedModels(); // m equations /* Create the matrix (A | b) for the equation Ax=b */ Matrix Ab = Matrix::Builder(); From 94fb5815f9e0d3ba15ee2e694559305649e9fd63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Tue, 31 Mar 2020 11:00:43 +0200 Subject: [PATCH 3/6] [poincare] Fix replaceReplaceableSymbols for parametered expressions Scenario: ans*int(0,x,0,0)->x then cos(x) crashed because replaceReplaceableSymbols would not care if it was replacing parameters in a parametered expression --- poincare/include/poincare/expression.h | 5 +-- poincare/include/poincare/expression_node.h | 2 +- poincare/include/poincare/function.h | 4 +-- .../include/poincare/parametered_expression.h | 4 +++ poincare/include/poincare/symbol.h | 5 +-- poincare/src/expression.cpp | 6 ++-- poincare/src/expression_node.cpp | 4 +-- poincare/src/function.cpp | 18 ++++++----- poincare/src/parametered_expression.cpp | 31 +++++++++++++++++++ poincare/src/symbol.cpp | 25 +++++++++++++-- 10 files changed, 81 insertions(+), 23 deletions(-) diff --git a/poincare/include/poincare/expression.h b/poincare/include/poincare/expression.h index 25ec2da9c..86e8e0b9b 100644 --- a/poincare/include/poincare/expression.h +++ b/poincare/include/poincare/expression.h @@ -76,6 +76,7 @@ class Expression : public TreeHandle { friend class NthRoot; friend class Number; friend class Opposite; + friend class ParameteredExpression; friend class Parenthesis; friend class PermuteCoefficient; friend class Power; @@ -347,8 +348,8 @@ protected: Expression defaultReplaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression expression); /* 'deepReplaceReplaceableSymbols' returns an uninitialized expression if it * is circularly defined. Same convention as for 'ExpressionWithoutSymbols'.*/ - Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly) { return node()->deepReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly); } - Expression defaultReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly); + Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount) { return node()->deepReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly, parameteredAncestorsCount); } + Expression defaultReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount); /* Simplification */ void beautifyAndApproximateScalar(Expression * simplifiedExpression, Expression * approximateExpression, ExpressionNode::ReductionContext userReductionContext, Context * context, Preferences::ComplexFormat complexFormat, Preferences::AngleUnit angleUnit); diff --git a/poincare/include/poincare/expression_node.h b/poincare/include/poincare/expression_node.h index 4041add6c..26acc69e1 100644 --- a/poincare/include/poincare/expression_node.h +++ b/poincare/include/poincare/expression_node.h @@ -172,7 +172,7 @@ public: /*!*/ virtual Expression setSign(Sign s, ReductionContext reductionContext); virtual int polynomialDegree(Context * context, const char * symbolName) const; /*!*/ virtual int getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[], ExpressionNode::SymbolicComputation symbolicComputation) const; - /*!*/ virtual Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly); + /*!*/ virtual Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount); typedef bool (*isVariableTest)(const char * c, Poincare::Context * context); virtual int getVariables(Context * context, isVariableTest isVariable, char * variables, int maxSizeVariable, int nextVariableIndex) const; virtual float characteristicXRange(Context * context, Preferences::AngleUnit angleUnit) const; diff --git a/poincare/include/poincare/function.h b/poincare/include/poincare/function.h index c627aa8e3..999321f8d 100644 --- a/poincare/include/poincare/function.h +++ b/poincare/include/poincare/function.h @@ -41,7 +41,7 @@ private: int serialize(char * buffer, int bufferSize, Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override; // Simplification Expression shallowReduce(ReductionContext reductionContext) override; - Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly) override; + Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount) override; LayoutShape leftLayoutShape() const override { return strlen(m_name) > 1 ? LayoutShape::MoreLetters : LayoutShape::OneLetter; }; LayoutShape rightLayoutShape() const override { return LayoutShape::BoundaryPunctuation; } @@ -60,7 +60,7 @@ public: // Simplification Expression replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression); Expression shallowReduce(ExpressionNode::ReductionContext reductionContext); - Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly); + Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount); }; } diff --git a/poincare/include/poincare/parametered_expression.h b/poincare/include/poincare/parametered_expression.h index 456a63be0..e5f23ec17 100644 --- a/poincare/include/poincare/parametered_expression.h +++ b/poincare/include/poincare/parametered_expression.h @@ -14,6 +14,8 @@ public: // Expression bool isParameteredExpression() const override { return true; } Expression replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression) override; + Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount) override; + // Expression properties int getVariables(Context * context, isVariableTest isVariable, char * variables, int maxSizeVariable, int nextVariableIndex) const override; }; @@ -36,6 +38,8 @@ public: * f(X)=diff(cos(x),x,X), X being an unknown. ReplaceUnknownInExpression does * that. */ Expression replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression); + Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount); + Symbol parameter(); protected: ParameteredExpression(const ParameteredExpressionNode * node) : Expression(node) {} }; diff --git a/poincare/include/poincare/symbol.h b/poincare/include/poincare/symbol.h index 2fb4fb6a6..4421d81de 100644 --- a/poincare/include/poincare/symbol.h +++ b/poincare/include/poincare/symbol.h @@ -34,7 +34,7 @@ public: /* Simplification */ Expression shallowReduce(ReductionContext reductionContext) override; - Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly) override; + Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount) override; LayoutShape leftLayoutShape() const override; /* Approximation */ @@ -62,6 +62,7 @@ public: // Symbol properties bool isSystemSymbol() const { return node()->isUnknown(); } + bool hasSameNameAs(Symbol & other) const; const char * name() const { return node()->name(); } // IsVariable tests static bool isSeriesSymbol(const char * c, Poincare::Context * context); @@ -71,7 +72,7 @@ public: Expression shallowReduce(ExpressionNode::ReductionContext reductionContext); Expression replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression); int getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[], ExpressionNode::SymbolicComputation symbolicComputation) const; - Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly); + Expression deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount); private: SymbolNode * node() const { return static_cast(Expression::node()); } }; diff --git a/poincare/src/expression.cpp b/poincare/src/expression.cpp index 7f9c761d8..bf6b35f67 100644 --- a/poincare/src/expression.cpp +++ b/poincare/src/expression.cpp @@ -394,10 +394,10 @@ void Expression::defaultSetChildrenInPlace(Expression other) { } } -Expression Expression::defaultReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly) { +Expression Expression::defaultReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount) { int nbChildren = numberOfChildren(); for (int i = 0; i < nbChildren; i++) { - Expression c = childAtIndex(i).deepReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly); + Expression c = childAtIndex(i).deepReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly, parameteredAncestorsCount); if (c.isUninitialized()) { // the expression is circularly defined, escape return Expression(); } @@ -746,7 +746,7 @@ Expression Expression::ExpressionWithoutSymbols(Expression e, Context * context, break; } didReplace = false; - e = e.deepReplaceReplaceableSymbols(context, &didReplace, replaceFunctionsOnly); + e = e.deepReplaceReplaceableSymbols(context, &didReplace, replaceFunctionsOnly, 0); if (e.isUninitialized()) { // the expression is circularly defined, escape replacementCount = k_maxSymbolReplacementsCount; } diff --git a/poincare/src/expression_node.cpp b/poincare/src/expression_node.cpp index 9889b69d1..d1d6d1455 100644 --- a/poincare/src/expression_node.cpp +++ b/poincare/src/expression_node.cpp @@ -36,8 +36,8 @@ int ExpressionNode::getPolynomialCoefficients(Context * context, const char * sy return Expression(this).defaultGetPolynomialCoefficients(context, symbolName, coefficients); } -Expression ExpressionNode::deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly) { - return Expression(this).defaultReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly); +Expression ExpressionNode::deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount) { + return Expression(this).defaultReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly, parameteredAncestorsCount); } int ExpressionNode::getVariables(Context * context, isVariableTest isVariable, char * variables, int maxSizeVariable, int nextVariableIndex) const { diff --git a/poincare/src/function.cpp b/poincare/src/function.cpp index 90ec61731..cc2055a6b 100644 --- a/poincare/src/function.cpp +++ b/poincare/src/function.cpp @@ -65,8 +65,8 @@ Expression FunctionNode::shallowReduce(ReductionContext reductionContext) { return Function(this).shallowReduce(reductionContext); // This uses Symbol::shallowReduce } -Expression FunctionNode::deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly) { - return Function(this).deepReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly); +Expression FunctionNode::deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount) { + return Function(this).deepReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly, parameteredAncestorsCount); } Evaluation FunctionNode::approximate(SinglePrecision p, Context * context, Preferences::ComplexFormat complexFormat, Preferences::AngleUnit angleUnit) const { @@ -133,13 +133,15 @@ Expression Function::shallowReduce(ExpressionNode::ReductionContext reductionCon return result.deepReduce(reductionContext); } -Expression Function::deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly) { - // Replace replaceable symbols in child - Expression self = defaultReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly); - if (self.isUninitialized()) { // if the child is circularly defined, escape - return self; +Expression Function::deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount) { + { + // Replace replaceable symbols in child + Expression self = defaultReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly ,parameteredAncestorsCount); + if (self.isUninitialized()) { // if the child is circularly defined, escape + return self; + } + assert(*this == self); } - assert(*this == self); Expression e = context->expressionForSymbolAbstract(*this, false); if (e.isUninitialized()) { return *this; diff --git a/poincare/src/parametered_expression.cpp b/poincare/src/parametered_expression.cpp index 98002fd1e..a8d06abcb 100644 --- a/poincare/src/parametered_expression.cpp +++ b/poincare/src/parametered_expression.cpp @@ -9,6 +9,10 @@ Expression ParameteredExpressionNode::replaceSymbolWithExpression(const SymbolAb return ParameteredExpression(this).replaceSymbolWithExpression(symbol, expression); } +Expression ParameteredExpressionNode::deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount) { + return ParameteredExpression(this).deepReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly, parameteredAncestorsCount); +} + int ParameteredExpressionNode::getVariables(Context * context, isVariableTest isVariable, char * variables, int maxSizeVariable, int nextVariableIndex) const { int numberOfVariables = childAtIndex(ParameteredExpression::ParameteredChildIndex())->getVariables(context, isVariable, variables, maxSizeVariable, nextVariableIndex); // Handle exception @@ -62,4 +66,31 @@ Expression ParameteredExpression::replaceSymbolWithExpression(const SymbolAbstra return *this; } +Expression ParameteredExpression::deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount) { + /* All children replaceable symbols should be replaced apart from symbols that + * are parameters in parametered expressions.*/ + int childrenCount = numberOfChildren(); + for (int i = 0; i < childrenCount; i++) { + if (i == ParameterChildIndex()) { + // Do not replace symbols in the parameter child + continue; + } + /* In the parametered child, increase the parametered ancestors count so + * that when replacing symbols, the expressions check that the symbols are + * not the parametered symbols. */ + bool shouldIncreaseParameteredAncestorsCount = i == ParameteredChildIndex(); + Expression c = childAtIndex(i).deepReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly, parameteredAncestorsCount + (shouldIncreaseParameteredAncestorsCount ? 1 : 0)); + if (c.isUninitialized()) { // the expression is circularly defined, escape + return Expression(); + } + } + return *this; +} + +Symbol ParameteredExpression::parameter() { + Expression e = childAtIndex(ParameteredExpression::ParameterChildIndex()); + assert(e.type() == ExpressionNode::Type::Symbol); + return static_cast(e); +} + } diff --git a/poincare/src/symbol.cpp b/poincare/src/symbol.cpp index 8e48844b9..58df963d9 100644 --- a/poincare/src/symbol.cpp +++ b/poincare/src/symbol.cpp @@ -94,8 +94,8 @@ Expression SymbolNode::shallowReduce(ReductionContext reductionContext) { return Symbol(this).shallowReduce(reductionContext); } -Expression SymbolNode::deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly) { - return Symbol(this).deepReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly); +Expression SymbolNode::deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount) { + return Symbol(this).deepReplaceReplaceableSymbols(context, didReplace, replaceFunctionsOnly, parameteredAncestorsCount); } ExpressionNode::LayoutShape SymbolNode::leftLayoutShape() const { @@ -134,6 +134,10 @@ Symbol Symbol::Builder(CodePoint name) { return Symbol::Builder(buffer, codePointLength); } +bool Symbol::hasSameNameAs(Symbol & other) const { + return (strcmp(other.name(), name()) == 0); +} + bool Symbol::isSeriesSymbol(const char * c, Poincare::Context * context) { // [NV][1-3] if (c[2] == 0 && (c[0] == 'N' || c[0] == 'V') && c[1] >= '1' && c[1] <= '3') { @@ -218,10 +222,25 @@ int Symbol::getPolynomialCoefficients(Context * context, const char * symbolName return 0; } -Expression Symbol::deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly) { +Expression Symbol::deepReplaceReplaceableSymbols(Context * context, bool * didReplace, bool replaceFunctionsOnly, int parameteredAncestorsCount) { if (replaceFunctionsOnly || isSystemSymbol()) { return *this; } + + // Check that this is not a parameter in a parametered expression + Expression ancestor = *this; + while (parameteredAncestorsCount > 0) { + ancestor = ancestor.parent(); + assert(!ancestor.isUninitialized()); + if (ancestor.isParameteredExpression()) { + parameteredAncestorsCount--; + Symbol ancestorParameter = static_cast(ancestor).parameter(); + if (hasSameNameAs(ancestorParameter)) { + return *this; + } + } + } + Expression e = context->expressionForSymbolAbstract(*this, true); if (e.isUninitialized()) { return *this; From 169d6bf94d516849637f8da4c8b58c991e31fae2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Tue, 31 Mar 2020 12:17:39 +0200 Subject: [PATCH 4/6] [poincare/parametered_expression] Use parameter() --- poincare/src/parametered_expression.cpp | 14 +++++--------- poincare/src/symbol.cpp | 11 ++++------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/poincare/src/parametered_expression.cpp b/poincare/src/parametered_expression.cpp index a8d06abcb..b6b9258db 100644 --- a/poincare/src/parametered_expression.cpp +++ b/poincare/src/parametered_expression.cpp @@ -21,11 +21,9 @@ int ParameteredExpressionNode::getVariables(Context * context, isVariableTest is } /* Remove the parameter symbol from the list of variable if it was added at * the previous line */ - // Get the parameter symbol - assert(childAtIndex(ParameteredExpression::ParameterChildIndex())->type() == ExpressionNode::Type::Symbol); - SymbolNode * parameterChild = static_cast(childAtIndex(ParameteredExpression::ParameterChildIndex())); + const char * parameterName = ParameteredExpression(this).parameter().name(); for (int i = nextVariableIndex; i < numberOfVariables; i++) { - if (strcmp(parameterChild->name(), &variables[i]) == 0) { + if (strcmp(parameterName, &variables[i]) == 0) { variables[i] = 0; numberOfVariables--; break; @@ -46,11 +44,9 @@ int ParameteredExpressionNode::getVariables(Context * context, isVariableTest is } Expression ParameteredExpression::replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression) { - Expression c = childAtIndex(ParameterChildIndex()); - assert(c.type() == ExpressionNode::Type::Symbol); - Symbol& parameterChild = static_cast(c); - if (symbol.type() != ExpressionNode::Type::Symbol || - strcmp(symbol.name(), parameterChild.name()) != 0) { + if (symbol.type() != ExpressionNode::Type::Symbol + || !parameter().hasSameNameAs(static_cast(symbol))) + { // If the symbol is not the parameter, replace normally return defaultReplaceSymbolWithExpression(symbol, expression); } diff --git a/poincare/src/symbol.cpp b/poincare/src/symbol.cpp index 58df963d9..50637fd65 100644 --- a/poincare/src/symbol.cpp +++ b/poincare/src/symbol.cpp @@ -173,13 +173,10 @@ Expression Symbol::shallowReduce(ExpressionNode::ReductionContext reductionConte // The symbol is a paremetered expression's parameter return *this; } - if (index == ParameteredExpression::ParameteredChildIndex()) { - assert(p.childAtIndex(ParameteredExpression::ParameterChildIndex()).type() == ExpressionNode::Type::Symbol); - Expression untypedParameter = p.childAtIndex(ParameteredExpression::ParameterChildIndex()); - Symbol parameter = static_cast(untypedParameter); - if (strcmp(parameter.name(), name()) == 0) { - return *this; - } + if (index == ParameteredExpression::ParameteredChildIndex() + && hasSameNameAs(static_cast(p).parameter())) + { + return *this; } } current = p; From 620ef3f5708413f0f6567b14e1bb349b9cc94995 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Tue, 31 Mar 2020 12:27:50 +0200 Subject: [PATCH 5/6] [poincare] Use SymbolAbstract::hasSameNameAs --- poincare/include/poincare/symbol.h | 1 - poincare/include/poincare/symbol_abstract.h | 1 + poincare/src/function.cpp | 2 +- poincare/src/parametered_expression.cpp | 4 +--- poincare/src/symbol.cpp | 6 +----- poincare/src/symbol_abstract.cpp | 4 ++++ 6 files changed, 8 insertions(+), 10 deletions(-) diff --git a/poincare/include/poincare/symbol.h b/poincare/include/poincare/symbol.h index 4421d81de..e441c8e27 100644 --- a/poincare/include/poincare/symbol.h +++ b/poincare/include/poincare/symbol.h @@ -62,7 +62,6 @@ public: // Symbol properties bool isSystemSymbol() const { return node()->isUnknown(); } - bool hasSameNameAs(Symbol & other) const; const char * name() const { return node()->name(); } // IsVariable tests static bool isSeriesSymbol(const char * c, Poincare::Context * context); diff --git a/poincare/include/poincare/symbol_abstract.h b/poincare/include/poincare/symbol_abstract.h index 521b20217..fab4c79a9 100644 --- a/poincare/include/poincare/symbol_abstract.h +++ b/poincare/include/poincare/symbol_abstract.h @@ -66,6 +66,7 @@ class SymbolAbstract : public Expression { friend class SymbolAbstractNode; public: const char * name() const { return node()->name(); } + bool hasSameNameAs(const SymbolAbstract & other) const; static size_t TruncateExtension(char * dst, const char * src, size_t len); static bool matches(const SymbolAbstract & symbol, ExpressionTest test, Context * context); constexpr static size_t k_maxNameSize = 8; diff --git a/poincare/src/function.cpp b/poincare/src/function.cpp index cc2055a6b..9ed5e7fae 100644 --- a/poincare/src/function.cpp +++ b/poincare/src/function.cpp @@ -102,7 +102,7 @@ Function Function::Builder(const char * name, size_t length, Expression child) { Expression Function::replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression) { // Replace the symbol in the child childAtIndex(0).replaceSymbolWithExpression(symbol, expression); - if (symbol.type() == ExpressionNode::Type::Function && strcmp(name(), symbol.name()) == 0) { + if (symbol.type() == ExpressionNode::Type::Function && hasSameNameAs(symbol)) { Expression value = expression.clone(); Expression p = parent(); if (!p.isUninitialized() && p.node()->childAtIndexNeedsUserParentheses(value, p.indexOfChild(*this))) { diff --git a/poincare/src/parametered_expression.cpp b/poincare/src/parametered_expression.cpp index b6b9258db..57bc73dcf 100644 --- a/poincare/src/parametered_expression.cpp +++ b/poincare/src/parametered_expression.cpp @@ -44,9 +44,7 @@ int ParameteredExpressionNode::getVariables(Context * context, isVariableTest is } Expression ParameteredExpression::replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression) { - if (symbol.type() != ExpressionNode::Type::Symbol - || !parameter().hasSameNameAs(static_cast(symbol))) - { + if (symbol.type() != ExpressionNode::Type::Symbol || !parameter().hasSameNameAs(symbol)) { // If the symbol is not the parameter, replace normally return defaultReplaceSymbolWithExpression(symbol, expression); } diff --git a/poincare/src/symbol.cpp b/poincare/src/symbol.cpp index 50637fd65..7f58b44b9 100644 --- a/poincare/src/symbol.cpp +++ b/poincare/src/symbol.cpp @@ -134,10 +134,6 @@ Symbol Symbol::Builder(CodePoint name) { return Symbol::Builder(buffer, codePointLength); } -bool Symbol::hasSameNameAs(Symbol & other) const { - return (strcmp(other.name(), name()) == 0); -} - bool Symbol::isSeriesSymbol(const char * c, Poincare::Context * context) { // [NV][1-3] if (c[2] == 0 && (c[0] == 'N' || c[0] == 'V') && c[1] >= '1' && c[1] <= '3') { @@ -197,7 +193,7 @@ Expression Symbol::shallowReduce(ExpressionNode::ReductionContext reductionConte } Expression Symbol::replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression) { - if (symbol.type() == ExpressionNode::Type::Symbol && strcmp(name(), symbol.name()) == 0) { + if (symbol.type() == ExpressionNode::Type::Symbol && hasSameNameAs(symbol)) { Expression value = expression.clone(); Expression p = parent(); if (!p.isUninitialized() && p.node()->childAtIndexNeedsUserParentheses(value, p.indexOfChild(*this))) { diff --git a/poincare/src/symbol_abstract.cpp b/poincare/src/symbol_abstract.cpp index 6b2901181..a6708fdee 100644 --- a/poincare/src/symbol_abstract.cpp +++ b/poincare/src/symbol_abstract.cpp @@ -53,6 +53,10 @@ T SymbolAbstract::Builder(const char * name, int length) { return static_cast(h); } +bool SymbolAbstract::hasSameNameAs(const SymbolAbstract & other) const { + return strcmp(other.name(), name()) == 0; +} + size_t SymbolAbstract::TruncateExtension(char * dst, const char * src, size_t len) { return UTF8Helper::CopyUntilCodePoint(dst, len, src, '.'); } From bd0b9f8a4a067eb10d023c554b4f8ac0dab6da6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Tue, 31 Mar 2020 15:02:20 +0200 Subject: [PATCH 6/6] [apps/curve_view] Fix typo The labels would not float on the right of the display --- apps/shared/curve_view.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/shared/curve_view.cpp b/apps/shared/curve_view.cpp index da15f9daa..5082cf26f 100644 --- a/apps/shared/curve_view.cpp +++ b/apps/shared/curve_view.cpp @@ -374,7 +374,7 @@ void CurveView::drawLabelsAndGraduations(KDContext * ctx, KDRect rect, Axis axis position = positionLabel(horizontalCoordinate, labelPosition, textSize, RelativePosition::Before, RelativePosition::None); if (floatingLabels == FloatingPosition::Min) { position = KDPoint(k_labelMargin, position.y()); - } else if (floatingLabels == FloatingPosition::Min) { + } else if (floatingLabels == FloatingPosition::Max) { position = KDPoint(Ion::Display::Width - textSize.width() - k_labelMargin, position.y()); } }