Merge changes I090f8bd1,I5e91414e,I21150a58,I43c333d9,I8b55a228, ...

* changes:
  Fix the recursiveness of simplification.
  Add multiplication rules.
  Fix bad assert.
  make isIdenticalTo aware of commutativity.
  Add is_identical_to util in tests.
  Add assert in operand() for comuttaive operator.
  Fix bad assert in binary operation.
  Fix size allocation in expression selector.
  Add a test for the recursive simplification.
  Refactor the simplification tests.

poincare/Makefile

@@ -35,7 +35,9 @@ tests += $(addprefix poincare/test/,\
   addition.cpp\
   fraction.cpp\
   integer.cpp\
+  identity.cpp\
   product.cpp\
+  simplify_utils.cpp\
   simplify_addition.cpp\
   simplify_product.cpp\
   trigo.cpp\

poincare/include/poincare/expression.h

@@ -45,6 +45,11 @@ class Expression {
     virtual bool isCommutative();
 
     virtual float approximate(Context& context) = 0;
+  private:
+    bool sequentialOperandsIdentity(Expression * e);
+    bool commutativeOperandsIdentity(Expression * e);
+    bool combinatoryCommutativeOperandsIdentity(Expression * e, bool * operandMatched,
+        int leftToMatch);
 };
 
 #endif

poincare/src/binary_operation.cpp

@@ -26,7 +26,8 @@ int BinaryOperation::numberOfOperands() {
 }
 
 Expression * BinaryOperation::operand(int i) {
-  assert(i>0 && i<=2);
+  assert(i >= 0);
+  assert(i < 2);
   return m_operands[i];
 }
 

poincare/src/commutative_operation.cpp

@@ -34,6 +34,7 @@ int CommutativeOperation::numberOfOperands() {
 }
 
 Expression * CommutativeOperation::operand(int i) {
+  assert(i >= 0);
   assert(i < m_numberOfOperands);
   return m_operands[i];
 }

poincare/src/expression.cpp

@@ -34,23 +34,28 @@ Expression * Expression::simplify() {
     return this->clone();
   }
 
-  /* We recursively simplify the children expressions.
-   * Note that we are sure to get the samne number of children as we had before
-   */
-  Expression ** simplifiedOperands = (Expression**) malloc(this->numberOfOperands() * sizeof(Expression*));
-  for (int i = 0; i < this->numberOfOperands(); i++) {
-    simplifiedOperands[i] = this->operand(i)->simplify();
-  }
-
-  /* Note that we don't need to clone the simplified children because they are
-   * already cloned before. */
-  Expression * result = this->cloneWithDifferentOperands(simplifiedOperands, this->numberOfOperands(), false);
-
-  // The table is no longer needed.
-  free(simplifiedOperands);
+  Expression * result = this->clone();
+  Expression * tmp = nullptr;
 
   bool simplification_pass_was_useful = true;
   while (simplification_pass_was_useful) {
+    /* We recursively simplify the children expressions.
+     * Note that we are sure to get the samne number of children as we had before
+     */
+    Expression ** simplifiedOperands = (Expression**) malloc(result->numberOfOperands() * sizeof(Expression*));
+    for (int i = 0; i < result->numberOfOperands(); i++) {
+      simplifiedOperands[i] = result->operand(i)->simplify();
+    }
+
+    /* Note that we don't need to clone the simplified children because they are
+     * already cloned before. */
+    tmp = result->cloneWithDifferentOperands(simplifiedOperands, result->numberOfOperands(), false);
+    delete result;
+    result = tmp;
+
+    // The table is no longer needed.
+    free(simplifiedOperands);
+
     simplification_pass_was_useful = false;
     for (int i=0; i<knumberOfSimplifications; i++) {
       const Simplification * simplification = (simplifications + i); // Pointer arithmetics
@@ -59,6 +64,7 @@ Expression * Expression::simplify() {
         simplification_pass_was_useful = true;
         delete result;
         result = simplified;
+        break;
       }
     }
   }
@@ -66,12 +72,75 @@ Expression * Expression::simplify() {
   return result;
 }
 
+bool Expression::sequentialOperandsIdentity(Expression * e) {
+  /* Here we simply test all operands for identity in the order they are defined
+   * in. */
+  for (int i=0; i<this->numberOfOperands(); i++) {
+    if (!e->operand(i)->isIdenticalTo(this->operand(i))) {
+      return false;
+    }
+  }
+  return true;
+}
+
+bool Expression::combinatoryCommutativeOperandsIdentity(Expression * e,
+    bool * operandMatched, int leftToMatch) {
+  if (leftToMatch == 0) {
+    return true;
+  }
+
+  // We try to test for equality the i-th operand of our first expression.
+  int i = this->numberOfOperands() - leftToMatch;
+  for (int j = 0; j<e->numberOfOperands(); j++) {
+    /* If the operand of the second expression has already been associated with
+     * a previous operand we skip it */
+    if (operandMatched[j]) {
+      continue;
+    }
+    if (this->operand(i)->isIdenticalTo(e->operand(j))) {
+      // We managed to match this operand.
+      operandMatched[j] = true;
+      /* We check that we can match the rest in this configuration, if so we
+       * are good. */
+      if (this->combinatoryCommutativeOperandsIdentity(e, operandMatched, leftToMatch - 1)) {
+        return true;
+      }
+      // Otherwise we backtrack.
+      operandMatched[j] = false;
+    }
+  }
+
+  return false;
+}
+
+bool Expression::commutativeOperandsIdentity(Expression * e) {
+  int leftToMatch = this->numberOfOperands();
+
+  /* We create a table allowing us to know which operands of the second
+   * expression have been associated with one of the operands of the first
+   * expression */
+  bool * operandMatched = (bool *) malloc (this->numberOfOperands() * sizeof(bool));
+  for (int i(0); i<this->numberOfOperands(); i++) {
+    operandMatched[i] = false;
+  }
+
+  // We call our recursive helper.
+  bool commutativelyIdentical = this->combinatoryCommutativeOperandsIdentity(e, operandMatched, leftToMatch);
+
+  free(operandMatched);
+  return commutativelyIdentical;
+}
+
 bool Expression::isIdenticalTo(Expression * e) {
   if (e->type() != this->type() || e->numberOfOperands() != this->numberOfOperands()) {
     return false;
   }
-  for (int i=0; i<this->numberOfOperands(); i++) {
-    if (!e->operand(i)->isIdenticalTo(this->operand(i))) {
+  if (this->isCommutative()) {
+    if (!this->commutativeOperandsIdentity(e)) {
+      return false;
+    }
+  } else {
+    if (!this->sequentialOperandsIdentity(e)) {
       return false;
     }
   }

poincare/src/leaf_expression.cpp

@@ -15,6 +15,6 @@ Expression * LeafExpression::operand(int i) {
 
 Expression * LeafExpression::cloneWithDifferentOperands(Expression** newOperands,
         int numberOfOperands, bool cloneOperands) {
-  assert(false);
-  return nullptr;
+  assert(numberOfOperands == 0);
+  return this->clone();
 }

poincare/src/simplify/expression_selector.cpp

@@ -119,7 +119,7 @@ int ExpressionSelector::commutativeMatch(Expression * e, ExpressionMatch * match
   }
 
   bool hasWildcard = child(m_numberOfChildren-1)->m_match == ExpressionSelector::Match::Wildcard;
-  uint8_t * selectorMatched = (uint8_t *)malloc(e->numberOfOperands());
+  uint8_t * selectorMatched = (uint8_t *)malloc(e->numberOfOperands()*sizeof(uint8_t));
 
   /* Initialize the selectors matched to unmatched (0xff), here we assume that
    * we never have more than 255 direct children selector. */

poincare/src/simplify/rules.pr

@@ -3,3 +3,5 @@ Addition(Integer.a,Integer.b)->$AddIntegers(a,b);
 Addition(Integer.a,Integer.b,c*)->Addition($AddIntegers(a,b),c*);
 Product(Product(a*),b*)->Product(a*,b*);
 Product(Integer[0],a*)->Integer[0];
+Product(Integer.a,Integer.b)->$MultiplyIntegers(a,b);
+Product(Integer.a,Integer.b,c*)->Product($MultiplyIntegers(a,b),c*);

poincare/src/simplify/simplification_generator.cpp

@@ -8,8 +8,22 @@ Expression * SimplificationGenerator::AddIntegers(Expression ** parameters, int
   Integer * result = new Integer((native_int_t)0);
   for (int i=0; i<numberOfParameters; i++) {
     assert(parameters[i]->type() == Expression::Type::Integer);
-    // FIXME: get rid of this operator overloading.
-    *result = *result + *(Integer *)parameters[i];
+    *result = result->add(*(Integer *)parameters[i], false);
+    /* Note We have to delete the parameters as they have been cloned before and
+     * we are the last ones to use them here. */
+    delete parameters[i];
+  }
+  return result;
+}
+
+Expression * SimplificationGenerator::MultiplyIntegers(Expression ** parameters, int numberOfParameters) {
+  Integer * result = new Integer((native_int_t)1);
+  for (int i=0; i<numberOfParameters; i++) {
+    assert(parameters[i]->type() == Expression::Type::Integer);
+    // FIXME: get rid of this operator overloading, there are to many stars.
+    *result = *result * *(Integer *)parameters[i];
+    /* Note We have to delete the parameters as they have been cloned before and
+     * we are the last ones to use them here. */
     delete parameters[i];
   }
   return result;

poincare/src/simplify/simplification_generator.h

@@ -6,6 +6,7 @@
 class SimplificationGenerator {
 public:
   static Expression * AddIntegers(Expression ** parameters, int numberOfParameters);
+  static Expression * MultiplyIntegers(Expression ** parameters, int numberOfParameters);
 };
 
 #endif

poincare/test/identity.cpp

@@ -0,0 +1,36 @@
+#include <assert.h>
+#include <quiz.h>
+#include "simplify_utils.h"
+
+QUIZ_CASE(poincare_identity_simple_term) {
+  assert(identical_to("1", "1"));
+  assert(!identical_to("1", "2"));
+
+  assert(identical_to("a", "a"));
+  assert(!identical_to("a", "b"));
+
+  assert(identical_to("1+2", "1+2"));
+  assert(!identical_to("1+2", "1+3"));
+
+  assert(identical_to("1-2", "1-2"));
+  assert(!identical_to("1-2", "1-3"));
+
+  assert(identical_to("1*2", "1*2"));
+  assert(!identical_to("1*2", "1*3"));
+
+  assert(identical_to("1/2", "1/2"));
+  assert(!identical_to("1/2", "1/3"));
+
+  assert(identical_to("1^2", "1^2"));
+  assert(!identical_to("1^2", "1^3"));
+
+  assert(identical_to("cos(1)", "cos(1)"));
+  assert(!identical_to("cos(1)", "cos(2)"));
+}
+
+QUIZ_CASE(poincare_identity_commutativity) {
+  assert(identical_to("1+2", "2+1"));
+  assert(identical_to("1*2", "2*1"));
+  assert(!identical_to("1-2", "2-1"));
+  assert(!identical_to("1/2", "2/1"));
+}

poincare/test/simplify_addition.cpp

@@ -1,53 +1,12 @@
-#include <quiz.h>
-#include <poincare.h>
 #include <assert.h>
-
-#if POINCARE_TESTS_PRINT_EXPRESSIONS
-#include "../src/expression_debug.h"
-#include <iostream>
-using namespace std;
-#endif
-
-void assert_simplifies_to(const char * input_string, const char * expected_string) {
-#if POINCARE_TESTS_PRINT_EXPRESSIONS
-  cout << "---- Simplification Run ----"  << endl;
-  cout << input_string << " -> " << expected_string << endl;
-#endif
-  //Expression* tab[3] = {new Integer(1), new Integer(2), new Integer(3)};
-  //Expression* input = new Addition(tab, 3, false);
-  Expression * input = Expression::parse(input_string);
-  assert(input != nullptr);
-#if POINCARE_TESTS_PRINT_EXPRESSIONS
-  cout << "Input = " << endl;
-  print_expression(input);
-#endif
-
-  Expression * simplified = input->simplify();
-  assert(simplified != nullptr);
-#if POINCARE_TESTS_PRINT_EXPRESSIONS
-  cout << "Simplified = " << endl;
-  print_expression(simplified);
-#endif
-
-  Expression * expected = Expression::parse(expected_string);
-  assert(expected != nullptr);
-#if POINCARE_TESTS_PRINT_EXPRESSIONS
-  cout << "Expected = " << endl;
-  print_expression(expected);
-#endif
-
-  assert(simplified->isIdenticalTo(expected));
-  delete expected;
-  if (simplified != input) {
-    delete simplified;
-  }
-  delete input;
-}
+#include <quiz.h>
+#include "simplify_utils.h"
 
 QUIZ_CASE(poincare_simplify_addition_integer) {
-  assert_simplifies_to("1", "1");
-  assert_simplifies_to("1+2", "3");
-  assert_simplifies_to("1+a", "1+a");
-  assert_simplifies_to("1+2+3+4+5+6+7", "28");
-  assert_simplifies_to("1+2+3+4+5+a+6+7", "28+a");
+  assert(simplifies_to("1", "1"));
+  assert(simplifies_to("1+2", "3"));
+  assert(simplifies_to("1+a", "1+a"));
+  assert(simplifies_to("1+2+3+4+5+6+7", "28"));
+  assert(simplifies_to("1+2+3+4+5+a+6+7", "28+a"));
+  assert(simplifies_to("a*(0+0)", "0"));
 }

poincare/test/simplify_product.cpp

@@ -1,16 +1,16 @@
-#include <quiz.h>
-#include <poincare.h>
 #include <assert.h>
+#include <quiz.h>
+#include "simplify_utils.h"
 
-QUIZ_CASE(poincare_simplify) {
-  {
-    Expression * e = Expression::parse("3*0");
-    Expression * e2 = e->simplify();
-    assert(e2->type() == Expression::Type::Integer);
-  }
-  {
-    Expression * e = Expression::parse("0*foo");
-    Expression * e2 = e->simplify();
-    assert(e2->type() == Expression::Type::Integer);
-  }
+QUIZ_CASE(poincare_simplify_product_by_zero) {
+  assert(simplifies_to("3*0", "0"));
+  assert(simplifies_to("foo*0", "0"));
+
+  assert(simplifies_to("0*3", "0"));
+  assert(simplifies_to("0*foo", "0"));
+
+  assert(simplifies_to("3*5", "15"));
+  assert(simplifies_to("8*6", "48"));
+
+  assert(simplifies_to("3*(5+4)", "27"));
 }

poincare/test/simplify_utils.cpp

@@ -0,0 +1,66 @@
+#include <poincare.h>
+#include <assert.h>
+
+#if POINCARE_TESTS_PRINT_EXPRESSIONS
+#include "../src/expression_debug.h"
+#include <iostream>
+using namespace std;
+#endif
+
+bool simplifies_to(const char * input_string, const char * expected_string) {
+#if POINCARE_TESTS_PRINT_EXPRESSIONS
+  cout << "---- Simplification Run ----"  << endl;
+  cout << input_string << " -> " << expected_string << endl;
+#endif
+  Expression * input = Expression::parse(input_string);
+  assert(input != nullptr);
+#if POINCARE_TESTS_PRINT_EXPRESSIONS
+  cout << "Input = " << endl;
+  print_expression(input);
+#endif
+
+  Expression * simplified = input->simplify();
+  assert(simplified != nullptr);
+#if POINCARE_TESTS_PRINT_EXPRESSIONS
+  cout << "Simplified = " << endl;
+  print_expression(simplified);
+#endif
+
+  Expression * expected = Expression::parse(expected_string);
+  assert(expected != nullptr);
+#if POINCARE_TESTS_PRINT_EXPRESSIONS
+  cout << "Expected = " << endl;
+  print_expression(expected);
+#endif
+
+  bool isIdentical = simplified->isIdenticalTo(expected);
+
+  delete expected;
+  delete simplified;
+  delete input;
+
+  return isIdentical;
+}
+
+bool identical_to(const char * input_string, const char * expected_string) {
+  Expression * input = Expression::parse(input_string);
+  assert(input != nullptr);
+#if POINCARE_TESTS_PRINT_EXPRESSIONS
+  cout << "Input = " << endl;
+  print_expression(input);
+#endif
+
+  Expression * expected = Expression::parse(expected_string);
+  assert(expected != nullptr);
+#if POINCARE_TESTS_PRINT_EXPRESSIONS
+  cout << "Expected = " << endl;
+  print_expression(expected);
+#endif
+
+  bool isIdentical = input->isIdenticalTo(expected);
+
+  delete expected;
+  delete input;
+
+  return isIdentical;
+}

poincare/test/simplify_utils.h

@@ -0,0 +1,10 @@
+#ifndef POINCARE_TEST_SIMPLIFY_UTILS_H
+#define POINCARE_TEST_SIMPLIFY_UTILS_H
+
+/* Tests that the first expression simplifies to the second. */
+bool simplifies_to(const char * input_string, const char * expected_string);
+
+/* Tests that the first expression is identical to the second. */
+bool identical_to(const char * input_string, const char * expected_string);
+
+#endif // POINCARE_TEST_SIMPLIFY_UTILS_H