From bcfa58a6a2f9fb568b4f81edfeeffa9300c38643 Mon Sep 17 00:00:00 2001 From: Jesse McCarthy Date: Mon, 11 Jan 2016 11:48:29 -0500 Subject: [PATCH 1/6] Remove unnecessary intermediate var. --- packages/babel-plugin-transform-es2015-parameters/src/rest.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/src/rest.js b/packages/babel-plugin-transform-es2015-parameters/src/rest.js index 572afeb7e1..85324b22d2 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/rest.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/rest.js @@ -242,16 +242,14 @@ export let visitor = { let target = path.getEarliestCommonAncestorFrom(state.references).getStatementParent(); // don't perform the allocation inside a loop - let highestLoop; target.findParent(function (path) { if (path.isLoop()) { - highestLoop = path; + target = path; } else if (path.isFunction()) { // stop crawling up for functions return true; } }); - if (highestLoop) target = highestLoop; target.insertBefore(loop); } From afd98ebd2afe87f16ab95c923892e22c3030419f Mon Sep 17 00:00:00 2001 From: Jesse McCarthy Date: Mon, 11 Jan 2016 11:52:52 -0500 Subject: [PATCH 2/6] Remove unnecessary literal return value. --- .../babel-plugin-transform-es2015-parameters/src/rest.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/src/rest.js b/packages/babel-plugin-transform-es2015-parameters/src/rest.js index 85324b22d2..d88d115fcd 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/rest.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/rest.js @@ -245,9 +245,9 @@ export let visitor = { target.findParent(function (path) { if (path.isLoop()) { target = path; - } else if (path.isFunction()) { - // stop crawling up for functions - return true; + } else { + // Stop crawling up if this is a function. + return path.isFunction(); } }); From 4c69f1cda5c6db3e807bf5e97f9845e9ff22316d Mon Sep 17 00:00:00 2001 From: Jesse McCarthy Date: Mon, 11 Jan 2016 11:55:30 -0500 Subject: [PATCH 3/6] Replace func expr with arrow. --- packages/babel-plugin-transform-es2015-parameters/src/rest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/src/rest.js b/packages/babel-plugin-transform-es2015-parameters/src/rest.js index d88d115fcd..30eef962c3 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/rest.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/rest.js @@ -242,7 +242,7 @@ export let visitor = { let target = path.getEarliestCommonAncestorFrom(state.references).getStatementParent(); // don't perform the allocation inside a loop - target.findParent(function (path) { + target.findParent(path => { if (path.isLoop()) { target = path; } else { From e4044062c39c839972ecc83294c48f3df2abdbe1 Mon Sep 17 00:00:00 2001 From: Jesse McCarthy Date: Mon, 11 Jan 2016 13:08:58 -0500 Subject: [PATCH 4/6] Remove unnecessary `length` check & nesting. --- .../src/rest.js | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/src/rest.js b/packages/babel-plugin-transform-es2015-parameters/src/rest.js index 30eef962c3..dd81839573 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/rest.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/rest.js @@ -171,20 +171,18 @@ export let visitor = { path.traverse(memberExpressionOptimisationVisitor, state); + // There are only "shorthand" references if (!state.deopted && !state.references.length) { - // we only have shorthands and there are no other references - if (state.candidates.length) { - for (let {path, cause} of (state.candidates: Array)) { - switch (cause) { - case "indexGetter": - optimiseIndexGetter(path, argsId, state.offset); - break; - case "lengthGetter": - optimiseLengthGetter(path, argsLengthExpression, argsId, state.offset); - break; - default: - path.replaceWith(argsId); - } + for (let {path, cause} of (state.candidates: Array)) { + switch (cause) { + case "indexGetter": + optimiseIndexGetter(path, argsId, state.offset); + break; + case "lengthGetter": + optimiseLengthGetter(path, argsLengthExpression, argsId, state.offset); + break; + default: + path.replaceWith(argsId); } } return; From 059d7123db291201f2fbc02cd4421090aba8accf Mon Sep 17 00:00:00 2001 From: Jesse McCarthy Date: Mon, 11 Jan 2016 13:10:26 -0500 Subject: [PATCH 5/6] Remove unnecessary `else`. --- .../babel-plugin-transform-es2015-parameters/src/rest.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/src/rest.js b/packages/babel-plugin-transform-es2015-parameters/src/rest.js index dd81839573..3f0bd764d7 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/rest.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/rest.js @@ -186,12 +186,12 @@ export let visitor = { } } return; - } else { - state.references = state.references.concat( - state.candidates.map(({path}) => path) - ); } + state.references = state.references.concat( + state.candidates.map(({path}) => path) + ); + // deopt shadowed functions as transforms like regenerator may try touch the allocation loop state.deopted = state.deopted || !!node.shadow; From b17965ab8b65fbc22e9168a31b4ab14621fdcff8 Mon Sep 17 00:00:00 2001 From: Jesse McCarthy Date: Mon, 11 Jan 2016 12:15:55 -0500 Subject: [PATCH 6/6] Improve comments. --- .../src/rest.js | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/babel-plugin-transform-es2015-parameters/src/rest.js b/packages/babel-plugin-transform-es2015-parameters/src/rest.js index 3f0bd764d7..6a3b15e8d4 100644 --- a/packages/babel-plugin-transform-es2015-parameters/src/rest.js +++ b/packages/babel-plugin-transform-es2015-parameters/src/rest.js @@ -29,12 +29,15 @@ let memberExpressionOptimisationVisitor = { }, Function(path, state) { - // skip over functions as whatever `arguments` we reference inside will refer - // to the wrong function + // Detect whether any reference to rest is contained in nested functions to + // determine if deopt is necessary. let oldNoOptimise = state.noOptimise; state.noOptimise = true; path.traverse(memberExpressionOptimisationVisitor, state); state.noOptimise = oldNoOptimise; + + // Skip because optimizing references to rest would refer to the `arguments` + // of the nested function. path.skip(); }, @@ -165,7 +168,18 @@ export let visitor = { // local rest binding name name: rest.name, - // whether any references to the rest parameter were made in a function + /* + It may be possible to optimize the output code in certain ways, such as + not generating code to initialize an array (perhaps substituting direct + references to arguments[i] or arguments.length for reads of the + corresponding rest parameter property) or positioning the initialization + code so that it may not have to execute depending on runtime conditions. + + This property tracks eligibility for optimization. "deopted" means give up + and don't perform optimization. For example, when any of rest's elements / + properties is assigned to at the top level, or referenced at all in a + nested function. + */ deopted: false, };