Fixed #313: parsed functions did not handle recursive calls correctly

HISTORY.md

@@ -1,5 +1,12 @@
 # History
 
+
+## not yet released, version 1.5.2
+
+- Fixed #313: parsed functions did not handle recursive calls correctly.
+- Performance improvements in parsed functions.
+
+
 ## 2015-04-08, version 1.5.1
 
 - Fixed #316: a bug in rounding values when formatting.

lib/expression/node/FunctionAssignmentNode.js

@@ -45,23 +45,24 @@ FunctionAssignmentNode.prototype.type = 'FunctionAssignmentNode';
  * @private
  */
 FunctionAssignmentNode.prototype._compile = function (defs) {
+  // add the function arguments to defs (used by SymbolNode and UpdateNode)
+  this.params.forEach(function (variable) {
+    defs.args[variable] = true;
+  });
+
   return 'scope["' + this.name + '"] = ' +
-      '  (function (scope) {' +
-      '    scope = Object.create(scope); ' +
+      '  (function () {' +
       '    var fn = function ' + this.name + '(' + this.params.join(',') + ') {' +
       '      if (arguments.length != ' + this.params.length + ') {' +
-      // TODO: use util.error.ArgumentsError here
+      // TODO: use util.error.ArgumentsError here?
       // TODO: test arguments error
       '        throw new SyntaxError("Wrong number of arguments in function ' + this.name + ' (" + arguments.length + " provided, ' + this.params.length + ' expected)");' +
       '      }' +
-      this.params.map(function (variable, index) {
-        return 'scope["' + variable + '"] = arguments[' + index + '];';
-      }).join('') +
       '      return ' + this.expr._compile(defs) + '' +
       '    };' +
       '    fn.syntax = "' + this.name + '(' + this.params.join(', ') + ')";' +
       '    return fn;' +
-      '  })(scope);';
+      '  })();';
 };
 
 /**

lib/expression/node/IndexNode.js

@@ -94,16 +94,17 @@ IndexNode.prototype.compileSubset = function(defs, replacement) {
     var useEnd = rangesUseEnd[i];
     if (range instanceof RangeNode) {
       if (useEnd) {
+        defs.args.end = true;
+
         // resolve end and create range
-        return '(function (scope) {' +
-            '  scope = Object.create(scope); ' +
-            '  scope["end"] = size[' + i + '];' +
+        return '(function () {' +
+            '  var end = size[' + i + '];' +
             '  return range(' +
             '    ' + range.start._compile(defs) + ', ' +
             '    ' + range.end._compile(defs) + ', ' +
             '    ' + (range.step ? range.step._compile(defs) : '1') +
             '  );' +
-            '})(scope)';
+            '})()';
       }
       else {
         // create range
@@ -116,12 +117,13 @@ IndexNode.prototype.compileSubset = function(defs, replacement) {
     }
     else {
       if (useEnd) {
+        defs.args.end = true;
+
         // resolve the parameter 'end'
-        return '(function (scope) {' +
-            '  scope = Object.create(scope); ' +
-            '  scope["end"] = size[' + i + '];' +
+        return '(function () {' +
+            '  var end = size[' + i + '];' +
             '  return ' + range._compile(defs) + ';' +
-            '})(scope)'
+            '})()'
       }
       else {
         // just evaluate the expression

lib/expression/node/Node.js

@@ -39,6 +39,7 @@ Node.prototype.compile = function (math) {
   // definitions globally available inside the closure of the compiled expressions
   var defs = {
     math: _transform(math),
+    args: {}, // can be filled with names of FunctionAssignment arguments
     _validateScope: _validateScope
   };
 

lib/expression/node/SymbolNode.js

@@ -41,7 +41,12 @@ SymbolNode.prototype._compile = function (defs) {
   defs['undef'] = undef;
   defs['Unit'] = Unit;
 
-  if (this.name in defs.math) {
+  if (this.name in defs.args) {
+    // this is a FunctionAssignment argument
+    // (like an x when inside the expression of a function assignment `f(x) = ...`)
+    return this.name;
+  }
+  else if (this.name in defs.math) {
     return '("' + this.name + '" in scope ? scope["' + this.name + '"] : math["' + this.name + '"])';
   }
   else {

lib/expression/node/UpdateNode.js

@@ -40,8 +40,13 @@ UpdateNode.prototype.type = 'UpdateNode';
  * @private
  */
 UpdateNode.prototype._compile = function (defs) {
-  return 'scope["' + this.index.objectName() + '\"] = ' +
-      this.index.compileSubset(defs,  this.expr._compile(defs));
+  var lhs = (this.index.objectName() in defs.args)
+    ? this.name + ' = ' // this is a FunctionAssignment argument
+    : 'scope["' + this.index.objectName() + '\"]';
+
+  var rhs = this.index.compileSubset(defs,  this.expr._compile(defs));
+
+  return lhs + ' = ' + rhs;
 };
 
 /**

test/expression/node/FunctionAssignmentNode.test.js

@@ -5,6 +5,8 @@ var assert = require('assert'),
     Node = require('../../../lib/expression/node/Node'),
     ConstantNode = require('../../../lib/expression/node/ConstantNode'),
     OperatorNode = require('../../../lib/expression/node/OperatorNode'),
+    ConditionalNode = require('../../../lib/expression/node/ConditionalNode'),
+    FunctionNode = require('../../../lib/expression/node/FunctionNode'),
     FunctionAssignmentNode = require('../../../lib/expression/node/FunctionAssignmentNode'),
     AssignmentNode = require('../../../lib/expression/node/AssignmentNode'),
     RangeNode = require('../../../lib/expression/node/RangeNode'),
@@ -48,6 +50,74 @@ describe('FunctionAssignmentNode', function() {
 
   });
 
+  it ('should eval a recursive FunctionAssignmentNode', function () {
+    var x = new SymbolNode('x');
+    var one = new ConstantNode(1);
+    var condition = new OperatorNode('<=', 'smallerEq', [x, one]);
+    var truePart = one;
+    var falsePart = new OperatorNode('*', 'multiply', [
+      x,
+      new FunctionNode('factorial', [
+        new OperatorNode('-', 'subtract', [
+          x,
+          one
+        ])
+      ])
+    ]);
+    var n1 = new ConditionalNode(condition, truePart, falsePart);
+
+    var n2 = new FunctionAssignmentNode('factorial', ['x'], n1);
+
+    var expr = n2.compile(math);
+    var scope = {};
+    var factorial = expr.eval(scope);
+    assert.equal(typeof scope.factorial, 'function');
+    assert.equal(factorial(3), 6);
+    assert.equal(factorial(5), 120);
+  });
+
+  it ('should eval a recursive FunctionAssignmentNode with two recursive calls', function () {
+    var x = new SymbolNode('x');
+    var zero = new ConstantNode(0);
+    var one = new ConstantNode(1);
+    var two = new ConstantNode(2);
+
+    var n1 = new ConditionalNode(
+        new OperatorNode('<=', 'smallerEq', [x, zero]),
+        zero,
+        new ConditionalNode(
+            new OperatorNode('<=', 'smallerEq', [x, two]),
+            one,
+            new OperatorNode('+', 'add', [
+              new FunctionNode('fib', [
+                new OperatorNode('-', 'subtract', [ x, one ])
+              ]),
+              new FunctionNode('fib', [
+                new OperatorNode('-', 'subtract', [ x, two ])
+              ])
+            ])
+        )
+    );
+
+    var n2 = new FunctionAssignmentNode('fib', ['x'], n1);
+    //var n2 = math.parse('fib(x) = (x <= 0) ? 0 : ((x <= 2) ? 1 : (fib(x - 1) + f(fib - 2)))');
+
+    var expr = n2.compile(math);
+    var scope = {};
+    var fib = expr.eval(scope);
+
+    assert.equal(typeof fib, 'function');
+    assert.equal(fib(0), 0);
+    assert.equal(fib(1), 1);
+    assert.equal(fib(2), 1);
+    assert.equal(fib(3), 2);
+    assert.equal(fib(4), 3);
+    assert.equal(fib(5), 5);
+    assert.equal(fib(6), 8);
+    assert.equal(fib(7), 13);
+    assert.equal(fib(8), 21);
+  });
+
   it ('should filter a FunctionAssignmentNode', function () {
     var a = new ConstantNode(2);
     var x = new SymbolNode('x');