From 960335c330129edac9eaebc00f6285e725b2acc4 Mon Sep 17 00:00:00 2001 From: Arthur Camouseigt Date: Thu, 17 Sep 2020 11:43:27 +0200 Subject: [PATCH] [Sequences] Fixed a few crashes Change-Id: Ib929bbae0f9ca06409706336ff799075e1288694 --- apps/shared/cache_context.cpp | 18 ++++++++++------ apps/shared/cache_context.h | 3 +-- apps/shared/global_context.cpp | 2 +- apps/shared/sequence.cpp | 39 ++++++++++++++++++++++++++++++++-- apps/shared/sequence.h | 3 ++- apps/solver/equation_store.cpp | 2 +- poincare/src/sequence.cpp | 9 +++++--- 7 files changed, 60 insertions(+), 16 deletions(-) diff --git a/apps/shared/cache_context.cpp b/apps/shared/cache_context.cpp index 3fcf76f43..e2cd46d48 100644 --- a/apps/shared/cache_context.cpp +++ b/apps/shared/cache_context.cpp @@ -12,9 +12,10 @@ using namespace Poincare; namespace Shared { template -CacheContext::CacheContext(Context * parentContext) : - ContextWithParent(parentContext), - m_values{{NAN, NAN},{NAN, NAN},{NAN,NAN}} +CacheContext::CacheContext(SequenceContext * sequenceContext) : + ContextWithParent(sequenceContext), + m_values{{NAN, NAN},{NAN, NAN},{NAN,NAN}}, + m_sequenceContext(sequenceContext) { } @@ -34,9 +35,14 @@ const Expression CacheContext::expressionForSymbolAbstract(const Poincare::Sy Sequence * seq = m_sequenceContext->sequenceStore()->modelForRecord(record); rank.replaceSymbolWithExpression(Symbol::Builder(UCodePointUnknown), Float::Builder(m_nValue)); T n = PoincareHelpers::ApproximateToScalar(rank, this); - // In case the rank is not int or sequence referenced is not defined, return NAN - if (std::floor(n) == n && seq->fullName() != nullptr) { - return Float::Builder(seq->valueAtRank(n, m_sequenceContext)); + // In case the sequence referenced is not defined or if the rank is not an int, return NAN + if (seq->fullName() != nullptr) { + if (std::floor(n) == n) { + Expression sequenceExpression = seq->expressionReduced(this); + if (seq->hasValidExpression(this)) { + return Float::Builder(seq->valueAtRank(n, m_sequenceContext)); + } + } } } else { return Float::Builder(NAN); diff --git a/apps/shared/cache_context.h b/apps/shared/cache_context.h index 3dabf0cfe..56f6f96b6 100644 --- a/apps/shared/cache_context.h +++ b/apps/shared/cache_context.h @@ -11,11 +11,10 @@ namespace Shared { template class CacheContext : public Poincare::ContextWithParent { public: - CacheContext(Poincare::Context * parentContext); + CacheContext(SequenceContext * sequenceContext); const Poincare::Expression expressionForSymbolAbstract(const Poincare::SymbolAbstract & symbol, bool clone, float unknownSymbolValue = NAN) override; void setValueForSymbol(T value, const Poincare::Symbol & symbol); void setNValue(int n) { m_nValue = n; } - void setSequenceContext(SequenceContext * sequenceContext) { m_sequenceContext = sequenceContext;} private: int nameIndexForSymbol(const Poincare::Symbol & symbol); int rankIndexForSymbol(const Poincare::Symbol & symbol); diff --git a/apps/shared/global_context.cpp b/apps/shared/global_context.cpp index 654c7737b..b5b6d6fe2 100644 --- a/apps/shared/global_context.cpp +++ b/apps/shared/global_context.cpp @@ -127,7 +127,7 @@ const Expression GlobalContext::ExpressionForSequence(const SymbolAbstract & sym char unknownN[bufferSize]; Poincare::SerializationHelper::CodePoint(unknownN, bufferSize, UCodePointUnknown); float rank = symbol.childAtIndex(0).approximateWithValueForSymbol(unknownN, unknownSymbolValue, ctx, Preferences::sharedPreferences()->complexFormat(),Preferences::sharedPreferences()->angleUnit()); - if (std::floor(rank) == rank && seq.hasValidExpression()) { + if (std::floor(rank) == rank) { SequenceContext sqctx(ctx, sequenceStore()); return Float::Builder(seq.evaluateXYAtParameter(rank, &sqctx).x2()); } else { diff --git a/apps/shared/sequence.cpp b/apps/shared/sequence.cpp index e6302274f..df64d31a1 100644 --- a/apps/shared/sequence.cpp +++ b/apps/shared/sequence.cpp @@ -7,6 +7,8 @@ #include #include #include +#include +#include #include "../shared/poincare_helpers.h" #include #include @@ -131,6 +133,40 @@ bool Sequence::isEmpty() { (type == Type::SingleRecurrence || data->initialConditionSize(1) == 0))); } +bool Sequence::badlyReferencesItself(Context * context) { + Expression e = expressionReduced(context); + bool value = e.hasExpression([](Expression e, const void * sequencePointer) { + if (e.type() != ExpressionNode::Type::Sequence) { + return false; + } + Sequence * seq = (Sequence *)(sequencePointer); + const char * symbolName = static_cast(e).name(); + /* symbolName is either u, v or w while seq->fullName has the extention .seq + * at the end. Therefore we cannot use strcmp on the two strings. We just + * want to check if the first char are identical*/ + if (strncmp(symbolName, seq->fullName(), strlen(symbolName)) == 0) { + /* The expression of the sequence contains a reference to itself. + * We must check if the sequence can be calculated before continuing + * If the sequence is of explicit type, it cannot reference itself. + * If the sequence is of SingleRecurrent type, it can be defined by: + * u(initialRank and u(n). + * If the sequence is of DoubleRecurrent type, it can be defined by: + * u(initialRank), u(initialRank+1), u(n) and u(n+1). + * In any other case, the value of the sequence cannot be computed. + * We therefore return NAN. */ + Expression rank = e.childAtIndex(0); + if (seq->type() == Sequence::Type::Explicit || + (!(rank.isIdenticalTo(Rational::Builder(seq->initialRank())) || rank.isIdenticalTo(Symbol::Builder(UCodePointUnknown))) && + (seq->type() == Sequence::Type::SingleRecurrence || (seq->type() == Sequence::Type::DoubleRecurrence && !(rank.isIdenticalTo(Rational::Builder(seq->initialRank()+1)) || rank.isIdenticalTo(Addition::Builder(Symbol::Builder(UCodePointUnknown), Rational::Builder(1)))))))) + { + return true; + } + } + return false; + }, reinterpret_cast(this)); + return value; +} + template T Sequence::templatedApproximateAtAbscissa(T x, SequenceContext * sqctx) const { T n = std::round(x); @@ -143,7 +179,7 @@ T Sequence::templatedApproximateAtAbscissa(T x, SequenceContext * sqctx) const { template T Sequence::valueAtRank(int n, SequenceContext *sqctx) { - if (n < 0) { + if (n < 0 || badlyReferencesItself(sqctx)) { return NAN; } int sequenceIndex = SequenceStore::sequenceIndexForName(fullName()[0]); @@ -174,7 +210,6 @@ T Sequence::approximateToNextRank(int n, SequenceContext * sqctx, int sequenceIn Poincare::SerializationHelper::CodePoint(unknownN, bufferSize, UCodePointUnknown); CacheContext ctx = CacheContext(sqctx); - ctx.setSequenceContext(sqctx); // Hold values u(n), u(n-1), u(n-2), v(n), v(n-1), v(n-2)... T values[MaxNumberOfSequences][MaxRecurrenceDepth+1]; diff --git a/apps/shared/sequence.h b/apps/shared/sequence.h index dffbc45b8..d53422c52 100644 --- a/apps/shared/sequence.h +++ b/apps/shared/sequence.h @@ -59,7 +59,8 @@ public: Poincare::Layout nameLayout(); bool isDefined() override; bool isEmpty() override; - bool hasValidExpression() { return m_definition.hasValidExpression(); } + bool hasValidExpression(Poincare::Context * context) { return m_definition.hasValidExpression() && !badlyReferencesItself(context); } + bool badlyReferencesItself(Poincare::Context * context); // Approximation Poincare::Coordinate2D evaluateXYAtParameter(float x, Poincare::Context * context) const override { return Poincare::Coordinate2D(x, templatedApproximateAtAbscissa(x, static_cast(context))); diff --git a/apps/solver/equation_store.cpp b/apps/solver/equation_store.cpp index 1523ff4f0..2ad2a7d53 100644 --- a/apps/solver/equation_store.cpp +++ b/apps/solver/equation_store.cpp @@ -102,7 +102,7 @@ void EquationStore::approximateSolve(Poincare::Context * context, bool shouldRep for (int i = 0; i <= k_maxNumberOfApproximateSolutions; i++) { root = PoincareHelpers::NextRoot(undevelopedExpression, m_variables[0], start, step, m_intervalApproximateSolutions[1], context); if (i == k_maxNumberOfApproximateSolutions) { - m_hasMoreThanMaxNumberOfApproximateSolution = !isnan(root); + m_hasMoreThanMaxNumberOfApproximateSolution = !std::isnan(root); break; } m_approximateSolutions[i] = root; diff --git a/poincare/src/sequence.cpp b/poincare/src/sequence.cpp index 83957a050..e6177bc19 100644 --- a/poincare/src/sequence.cpp +++ b/poincare/src/sequence.cpp @@ -97,9 +97,12 @@ Expression Sequence::replaceSymbolWithExpression(const SymbolAbstract & symbol, } Expression Sequence::shallowReduce(ExpressionNode::ReductionContext reductionContext) { - if (reductionContext.symbolicComputation() == ExpressionNode::SymbolicComputation::ReplaceAllSymbolsWithUndefined - || childAtIndex(0).isUndefined()) - { + Expression e = Expression::defaultShallowReduce(); + e = e.defaultHandleUnitsInChildren(); + if (e.isUndefined()) { + return e; + } + if (reductionContext.symbolicComputation() == ExpressionNode::SymbolicComputation::ReplaceAllSymbolsWithUndefined) { return replaceWithUndefinedInPlace(); } return *this;