From db93c52182f0e49e55b0d773b0825e1a119a4ce1 Mon Sep 17 00:00:00 2001 From: Sebastian McKenzie Date: Wed, 11 Feb 2015 11:27:50 +1100 Subject: [PATCH] opt out of tail recursion optimisation if the owner id has been reassigned - fixes #744 --- .../transformation/transformers/es6/tail-call.js | 9 +++++++-- lib/6to5/traversal/scope.js | 11 +++++++++++ .../es6-tail-call/ignore-reassigned/actual.js | 13 +++++++++++++ .../es6-tail-call/ignore-reassigned/expected.js | 15 +++++++++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/transformation/es6-tail-call/ignore-reassigned/actual.js create mode 100644 test/fixtures/transformation/es6-tail-call/ignore-reassigned/expected.js diff --git a/lib/6to5/transformation/transformers/es6/tail-call.js b/lib/6to5/transformation/transformers/es6/tail-call.js index b40757edf9..eb47c8253e 100644 --- a/lib/6to5/transformation/transformers/es6/tail-call.js +++ b/lib/6to5/transformation/transformers/es6/tail-call.js @@ -64,11 +64,16 @@ TailCallTransformer.prototype.run = function () { var scope = this.scope; var node = this.node; - // only tail recursion can be optimized as for now, - // so we can skip anonymous functions entirely + // only tail recursion can be optimized as for now, so we can skip anonymous + // functions entirely var ownerId = this.ownerId; if (!ownerId) return; + // check if the ownerId has been reassigned, if it has then it's not safe to + // perform optimisations + var ownerIdInfo = this.scope.getBindingInfo(ownerId.name); + if (!ownerIdInfo || ownerIdInfo.reassigned) return; + // traverse the function and look for tail recursion scope.traverse(node, firstPass, this); diff --git a/lib/6to5/traversal/scope.js b/lib/6to5/traversal/scope.js index 15a3100513..683bca76ce 100644 --- a/lib/6to5/traversal/scope.js +++ b/lib/6to5/traversal/scope.js @@ -229,6 +229,14 @@ Scope.prototype.registerDeclaration = function (node) { } }; +Scope.prototype.registerBindingReassignment = function (node) { + var ids = t.getBindingIdentifiers(node); + for (var name in ids) { + var info = this.getBindingInfo(name); + if (info) info.reassigned = true; + } +}; + Scope.prototype.registerBinding = function (kind, node) { if (!kind) throw new ReferenceError("no `kind`"); @@ -241,6 +249,7 @@ Scope.prototype.registerBinding = function (kind, node) { this.bindings[name] = { typeAnnotation: this.getTypeAnnotation(name, id, node), + reassigned: false, identifier: id, scope: this, kind: kind @@ -302,6 +311,8 @@ var programReferenceVisitor = { state.addGlobal(node); } else if (t.isLabeledStatement(node)) { state.addGlobal(node); + } else if (t.isAssignmentExpression(node)) { + scope.registerBindingReassignment(node); } } }; diff --git a/test/fixtures/transformation/es6-tail-call/ignore-reassigned/actual.js b/test/fixtures/transformation/es6-tail-call/ignore-reassigned/actual.js new file mode 100644 index 0000000000..cfb6182a00 --- /dev/null +++ b/test/fixtures/transformation/es6-tail-call/ignore-reassigned/actual.js @@ -0,0 +1,13 @@ +// we need to deopt `test` if it's reassigned as we can't be certain of it's +// state, ie. it could have been rebound or dereferenced + +function test(exit) { + if (exit) { + return this.x; + } + return test(true); +} + +test = test.bind({ x: "yay" }); + +console.log(test()); diff --git a/test/fixtures/transformation/es6-tail-call/ignore-reassigned/expected.js b/test/fixtures/transformation/es6-tail-call/ignore-reassigned/expected.js new file mode 100644 index 0000000000..f658848e82 --- /dev/null +++ b/test/fixtures/transformation/es6-tail-call/ignore-reassigned/expected.js @@ -0,0 +1,15 @@ +"use strict"; + +// we need to deopt `test` if it's reassigned as we can't be certain of it's +// state, ie. it could have been rebound or dereferenced + +function test(exit) { + if (exit) { + return this.x; + } + return test(true); +} + +test = test.bind({ x: "yay" }); + +console.log(test());