[poincare/expression] Turn getUnit into extractUnits

Addition::shallowReduce factors the unit across its terms.
Previously expressions of the form 1_m+π_m were reduced to undef.

poincare/include/poincare/division.h

@@ -25,7 +25,7 @@ public:
   // Properties
   Type type() const override { return Type::Division; }
   int polynomialDegree(Context * context, const char * symbolName) const override;
-  Expression getUnit() const override { assert(false); return ExpressionNode::getUnit(); }
+  Expression extractUnits() override { assert(false); return ExpressionNode::extractUnits(); }
 
   // Approximation
   virtual Evaluation<float> approximate(SinglePrecision p, Context * context, Preferences::ComplexFormat complexFormat, Preferences::AngleUnit angleUnit) const override {

poincare/include/poincare/empty_expression.h

@@ -22,7 +22,7 @@ public:
   // Properties
   Type type() const override { return Type::EmptyExpression; }
   int serialize(char * buffer, int bufferSize, Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override;
-  Expression getUnit() const override { assert(false); return ExpressionNode::getUnit(); }
+  Expression extractUnits() override { assert(false); return ExpressionNode::extractUnits(); }
 
   // Simplification
   LayoutShape leftLayoutShape() const override {

poincare/include/poincare/expression.h

@@ -199,7 +199,7 @@ public:
   Expression replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression) { return node()->replaceSymbolWithExpression(symbol, expression); }
 
   /* Units */
-  Expression getUnit() const { return node()->getUnit(); }
+  Expression extractUnits() { return node()->extractUnits(); }
   bool hasUnit() const;
 
   /* Complex */

poincare/include/poincare/expression_node.h

@@ -178,7 +178,7 @@ public:
   virtual float characteristicXRange(Context * context, Preferences::AngleUnit angleUnit) const;
   bool isOfType(Type * types, int length) const;
 
-  virtual Expression getUnit() const; // Only reduced nodes should answer
+  virtual Expression extractUnits(); // Only reduced nodes should answer
 
   /* Simplification */
   /* SimplificationOrder returns:

poincare/include/poincare/function.h

@@ -28,9 +28,6 @@ public:
   int getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[], ExpressionNode::SymbolicComputation symbolicComputation) const override;
   int getVariables(Context * context, isVariableTest isVariable, char * variables, int maxSizeVariable, int nextVariableIndex) const override;
   float characteristicXRange(Context * context, Preferences::AngleUnit angleUnit) const override;
-  /* getUnit() is ExpressionNode::getUnit ->
-   * as the function is reduced, it would have been replaced if it had a
-   * definition. It thus has no definition, so no unit. */
 
 private:
   char m_name[0]; // MUST be the last member variable

poincare/include/poincare/multiplication.h

@@ -25,7 +25,7 @@ public:
   int polynomialDegree(Context * context, const char * symbolName) const override;
   int getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[], ExpressionNode::SymbolicComputation symbolicComputation) const override;
   bool childAtIndexNeedsUserParentheses(const Expression & child, int childIndex) const override;
-  Expression getUnit() const override;
+  Expression extractUnits() override;
 
   // Approximation
   template<typename T> static Complex<T> compute(const std::complex<T> c, const std::complex<T> d, Preferences::ComplexFormat complexFormat) { return Complex<T>::Builder(c*d); }
@@ -76,7 +76,7 @@ public:
 
   // Properties
   int getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[], ExpressionNode::SymbolicComputation symbolicComputation) const;
-  Expression getUnit() const;
+  Expression extractUnits();
   // Approximation
   template<typename T> static void computeOnArrays(T * m, T * n, T * result, int mNumberOfColumns, int mNumberOfRows, int nNumberOfColumns);
   // Simplification

poincare/include/poincare/nth_root.h

@@ -20,7 +20,7 @@ public:
 #endif
 
 private:
-  Expression getUnit() const override { assert(false); return ExpressionNode::getUnit(); }
+  Expression extractUnits() override { assert(false); return ExpressionNode::extractUnits(); }
   // Layout
   Layout createLayout(Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override;
   int serialize(char * buffer, int bufferSize, Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override;

poincare/include/poincare/parenthesis.h

@@ -21,7 +21,7 @@ public:
   // Properties
   Type type() const override { return Type::Parenthesis; }
   int polynomialDegree(Context * context, const char * symbolName) const override;
-  Expression getUnit() const override { assert(false); return ExpressionNode::getUnit(); }
+  Expression extractUnits() override { assert(false); return ExpressionNode::extractUnits(); }
 
   // Layout
   Layout createLayout(Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override;

poincare/include/poincare/power.h

@@ -29,7 +29,7 @@ public:
   Sign sign(Context * context) const override;
   Expression setSign(Sign s, ReductionContext reductionContext) override;
   bool childAtIndexNeedsUserParentheses(const Expression & child, int childIndex) const override;
-  Expression getUnit() const override;
+  Expression extractUnits() override;
 
   int polynomialDegree(Context * context, const char * symbolName) const override;
   int getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[], ExpressionNode::SymbolicComputation symbolicComputation) const override;
@@ -79,7 +79,6 @@ public:
   int getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[]) const;
   Expression shallowReduce(ExpressionNode::ReductionContext reductionContext);
   Expression shallowBeautify(ExpressionNode::ReductionContext reductionContext);
-  Expression getUnit() const;
 
 private:
   constexpr static int k_maxExactPowerMatrix = 100;

poincare/include/poincare/subtraction.h

@@ -24,7 +24,7 @@ public:
   Type type() const override { return Type::Subtraction; }
   int polynomialDegree(Context * context, const char * symbolName) const override;
   bool childAtIndexNeedsUserParentheses(const Expression & child, int childIndex) const override;
-  Expression getUnit() const override { assert(false); return ExpressionNode::getUnit(); }
+  Expression extractUnits() override { assert(false); return ExpressionNode::extractUnits(); }
 
   // Approximation
   template<typename T> static Complex<T> compute(const std::complex<T> c, const std::complex<T> d, Preferences::ComplexFormat complexFormat) { return Complex<T>::Builder(c - d); }

poincare/include/poincare/unit.h

@@ -151,7 +151,7 @@ public:
   // Expression Properties
   Type type() const override { return Type::Unit; }
   Sign sign(Context * context) const override;
-  Expression getUnit() const override;
+  Expression extractUnits() override;
 
   /* Layout */
   Layout createLayout(Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override;
@@ -725,7 +725,6 @@ public:
 
   Unit(const UnitNode * node) : Expression(node) {}
   static Unit Builder(const Dimension * dimension, const Representative * representative, const Prefix * prefix);
-  Expression getUnit() const { return clone(); }
 
   // Simplification
   Expression shallowReduce(ExpressionNode::ReductionContext reductionContext);

poincare/include/poincare/unit_convert.h

@@ -20,7 +20,7 @@ public:
   Type type() const override { return Type::UnitConvert; }
 
 private:
-  Expression getUnit() const override { assert(false); return ExpressionNode::getUnit(); }
+  Expression extractUnits() override { assert(false); return ExpressionNode::extractUnits(); }
   // Simplification
   Expression shallowReduce(ReductionContext reductionContext) override;
   // Evalutation

poincare/src/addition.cpp

@@ -155,16 +155,33 @@ Expression Addition::shallowReduce(ExpressionNode::ReductionContext reductionCon
   /* Step 2: Handle the units. All children should have the same unit, otherwise
    * the result is not homogeneous. */
   {
-    Expression unit = childAtIndex(0).getUnit();
+    Expression unit = childAtIndex(0).extractUnits();
     const bool hasUnit = !unit.isUninitialized();
     for (int i = 1; i < childrenCount; i++) {
-      Expression otherUnit = childAtIndex(i).getUnit();
+      Expression otherUnit = childAtIndex(i).extractUnits();
       if (hasUnit == otherUnit.isUninitialized() ||
           (hasUnit && !unit.isIdenticalTo(otherUnit)))
       {
         return replaceWithUndefinedInPlace();
       }
     }
+    if (hasUnit) {
+      for (int i = 0; i < childrenCount; i++) {
+        /* Any unary hierarchy must be squashed here since it has not been
+         * done in extractUnits.
+         */
+        Expression child = childAtIndex(i);
+        if (child.type() == ExpressionNode::Type::Multiplication) {
+          static_cast<Multiplication &>(child).squashUnaryHierarchyInPlace();
+        }
+      }
+      Expression addition = shallowReduce(reductionContext);
+      Multiplication result = Multiplication::Builder(unit);
+      result.mergeSameTypeChildrenInPlace();
+      addition.replaceWithInPlace(result);
+      result.addChildAtIndexInPlace(addition, 0, 1);
+      return std::move(result);
+    }
   }
 
   // Step 3: Sort the children

poincare/src/expression_node.cpp

@@ -136,7 +136,7 @@ bool ExpressionNode::isOfType(Type * types, int length) const {
   return false;
 }
 
-Expression ExpressionNode::getUnit() const {
+Expression ExpressionNode::extractUnits() {
   return Expression();
 }
 

poincare/src/multiplication.cpp

@@ -63,8 +63,8 @@ bool MultiplicationNode::childAtIndexNeedsUserParentheses(const Expression & chi
   return child.isOfType(types, 2);
 }
 
-Expression MultiplicationNode::getUnit() const {
-  return Multiplication(this).getUnit();
+Expression MultiplicationNode::extractUnits() {
+  return Multiplication(this).extractUnits();
 }
 
 template<typename T>
@@ -255,20 +255,30 @@ int Multiplication::getPolynomialCoefficients(Context * context, const char * sy
   return deg;
 }
 
-Expression Multiplication::getUnit() const {
+Expression Multiplication::extractUnits() {
   Multiplication result = Multiplication::Builder();
   int resultChildrenCount = 0;
-  const int childrenCount = numberOfChildren();
-  for (int i = 0; i < childrenCount; i++) {
-    Expression currentUnit = childAtIndex(i).getUnit();
+  for (int i = 0; i < numberOfChildren(); i++) {
+    Expression currentUnit = childAtIndex(i).extractUnits();
     if (!currentUnit.isUninitialized()) {
+      assert(childAtIndex(i) == currentUnit);
       result.addChildAtIndexInPlace(currentUnit, resultChildrenCount, resultChildrenCount);
       resultChildrenCount++;
+      removeChildAtIndexInPlace(i--);
     }
   }
   if (resultChildrenCount == 0) {
     return Expression();
   }
+  /* squashUnaryHierarchyInPlace();
+   * That would make 'this' invalid, so we would rather keep any unary
+   * hierarchy as it is and handle it later.
+   * TODO ?
+   * A possible solution would be that the extractUnits method becomes
+   *   Expression extractUnits(Expression & units)
+   * returning the Expression that is left after extracting the units
+   * and setting the units reference instead of returning the units.
+   */
   return result.squashUnaryHierarchyInPlace();
 }
 

poincare/src/power.cpp

@@ -81,8 +81,12 @@ int PowerNode::polynomialDegree(Context * context, const char * symbolName) cons
   return -1;
 }
 
-Expression PowerNode::getUnit() const {
-  return Power(this).getUnit();
+Expression PowerNode::extractUnits() {
+  if (!childAtIndex(0)->extractUnits().isUninitialized()) {
+    assert(childAtIndex(0)->type() == ExpressionNode::Type::Unit);
+    return Power(this);
+  }
+  return ExpressionNode::extractUnits();
 }
 
 int PowerNode::getPolynomialCoefficients(Context * context, const char * symbolName, Expression coefficients[], ExpressionNode::SymbolicComputation symbolicComputation) const {
@@ -985,14 +989,6 @@ Expression Power::shallowBeautify(ExpressionNode::ReductionContext reductionCont
   return *this;
 }
 
-Expression Power::getUnit() const {
-  Expression baseUnit = childAtIndex(0).getUnit();
-  if (baseUnit.isUninitialized()) {
-    return baseUnit;
-  }
-  return Power::Builder(baseUnit, childAtIndex(1).clone());
-}
-
 // Private
 
 // Simplification

poincare/src/unit.cpp

@@ -164,8 +164,8 @@ ExpressionNode::Sign UnitNode::sign(Context * context) const {
   return Sign::Positive;
 }
 
-Expression UnitNode::getUnit() const {
-  return Unit(this).getUnit();
+Expression UnitNode::extractUnits() {
+  return Unit(this);
 }
 
 int UnitNode::simplificationOrderSameType(const ExpressionNode * e, bool ascending, bool canBeInterrupted, bool ignoreParentheses) const {

poincare/src/unit_convert.cpp

@@ -41,7 +41,7 @@ Expression UnitConvert::shallowReduce(ExpressionNode::ReductionContext reduction
         reductionContext.angleUnit(),
         reductionContext.target(),
         ExpressionNode::SymbolicComputation::ReplaceAllSymbolsWithUndefinedAndReplaceUnits);
-    Expression unit = childAtIndex(1).clone().reduce(reductionContextWithUnits).getUnit();
+    Expression unit = childAtIndex(1).clone().reduce(reductionContextWithUnits).extractUnits();
     if (unit.isUninitialized()) {
       // There is no unit on the right
       return replaceWithUndefinedInPlace();
@@ -54,7 +54,7 @@ Expression UnitConvert::shallowReduce(ExpressionNode::ReductionContext reduction
       reductionContext.angleUnit(),
       reductionContext.target(),
       ExpressionNode::SymbolicComputation::ReplaceAllSymbolsWithUndefinedAndDoNotReplaceUnits);
-  Expression finalUnit = childAtIndex(1).reduce(reductionContextWithoutUnits).getUnit();
+  Expression finalUnit = childAtIndex(1).reduce(reductionContextWithoutUnits).extractUnits();
 
   // Divide the left member by the new unit
   Expression division = Division::Builder(childAtIndex(0), finalUnit.clone());

poincare/test/expression_properties.cpp

@@ -374,10 +374,10 @@ void assert_reduced_expression_unit(const char * expression, const char * unit,
   ExpressionNode::ReductionContext redContext(&globalContext, Real, Degree, SystemForApproximation, symbolicComutation);
   Expression e = parse_expression(expression, &globalContext, false);
   e = e.reduce(redContext);
-  Expression u1 = e.getUnit();
+  Expression u1 = e.extractUnits();
   Expression u2 = parse_expression(unit, &globalContext, false);
   u2 = u2.reduce(redContext);
-  u2 = u2.getUnit();
+  u2 = u2.extractUnits();
   quiz_assert_print_if_failure(u1.isUninitialized() == u2.isUninitialized() && (u1.isUninitialized() || u1.isIdenticalTo(u2)), expression);
 }
 

poincare/test/simplification.cpp

@@ -414,6 +414,7 @@ QUIZ_CASE(poincare_simplification_units) {
   /* Valid expressions */
   assert_parsed_expression_simplify_to("-2×_A", "-2×_A");
   assert_parsed_expression_simplify_to("cos(1_s/1_s)", "cos(1)");
+  assert_parsed_expression_simplify_to("1_m+π_m+√(2)_m-cos(15)_m", "6.3154941288217×_m");
 }
 
 QUIZ_CASE(poincare_simplification_power) {