make PathHoister much more flexible, now ignores global references and will not deopt on reassignments and will instead hoist as high as it can, this also fixes #1529 since the order of operations has changed

This commit is contained in:
Sebastian McKenzie
2015-05-14 23:29:02 +01:00
parent d4fb924b6a
commit 14dddcda36
21 changed files with 192 additions and 52 deletions

View File

@@ -25,8 +25,6 @@ export default {
"spec.blockScopedFunctions": require("./spec/block-scoped-functions"),
"optimisation.react.constantElements": require("./optimisation/react.constant-elements"),
"optimisation.react.inlineElements": require("./optimisation/react.inline-elements"),
reactCompat: require("./other/react-compat"),
react: require("./other/react"),
"es7.comprehensions": require("./es7/comprehensions"),
"es6.classes": require("./es6/classes"),
asyncToGenerator: require("./other/async-to-generator"),
@@ -60,6 +58,8 @@ export default {
"es6.destructuring": require("./es6/destructuring"),
"es6.blockScoping": require("./es6/block-scoping"),
"es6.spec.blockScoping": require("./es6/spec.block-scoping"),
reactCompat: require("./other/react-compat"),
react: require("./other/react"),
// es6 syntax transformation is **forbidden** past this point since regenerator will chuck a massive
// hissy fit

View File

@@ -35,9 +35,8 @@ export function JSXElement(node, parent, scope, file) {
this.traverse(immutabilityVisitor, state);
if (state.isImmutable) {
this.hoist();
this.skip();
return this.hoist();
} else {
node._hoisted = true;
}
node._hoisted = true;
}

View File

@@ -6,7 +6,8 @@ export function manipulateOptions(opts) {
}
export var metadata = {
optional: true
optional: true,
group: "builtin-advanced"
};
require("../../helpers/build-react-transformer")(exports, {

View File

@@ -3,6 +3,10 @@ import * as t from "../../../types";
var JSX_ANNOTATION_REGEX = /^\*\s*@jsx\s+([^\s]+)/;
export var metadata = {
group: "builtin-advanced"
};
export function Program(node, parent, scope, file) {
var id = file.opts.jsxPragma;

View File

@@ -2,28 +2,24 @@ import * as react from "../../transformation/helpers/react";
import * as t from "../../types";
var referenceVisitor = {
enter(node, parent, scope, state) {
ReferencedIdentifier(node, parent, scope, state) {
if (this.isJSXIdentifier() && react.isCompatTag(node.name)) {
return;
}
if (this.isJSXIdentifier() || this.isIdentifier()) {
// direct references that we need to track to hoist this to the highest scope we can
if (this.isReferenced()) {
var bindingInfo = scope.getBinding(node.name);
// direct references that we need to track to hoist this to the highest scope we can
var bindingInfo = scope.getBinding(node.name);
if (!bindingInfo) return;
// this binding isn't accessible from the parent scope so we can safely ignore it
// eg. it's in a closure etc
if (bindingInfo !== state.scope.getBinding(node.name)) return;
// this binding isn't accessible from the parent scope so we can safely ignore it
// eg. it's in a closure etc
if (bindingInfo !== state.scope.getBinding(node.name)) return;
if (bindingInfo) {
if (bindingInfo.constant) {
state.bindings[node.name] = bindingInfo;
} else {
state.foundIncompatible = true;
this.stop();
}
}
if (bindingInfo.constant) {
state.bindings[node.name] = bindingInfo;
} else {
for (var violationPath of (bindingInfo.constantViolations: Array)) {
state.breakOnScopePaths.push(violationPath.scope.path);
}
}
}
@@ -31,10 +27,10 @@ var referenceVisitor = {
export default class PathHoister {
constructor(path, scope) {
this.foundIncompatible = false;
this.breakOnScopePaths = [];
this.bindings = {};
this.scope = scope;
this.scopes = [];
this.scope = scope;
this.path = path;
}
@@ -45,32 +41,41 @@ export default class PathHoister {
return false;
}
}
return true;
}
getCompatibleScopes() {
var checkScope = this.path.scope;
var scope = this.path.scope;
do {
if (this.isCompatibleScope(checkScope)) {
this.scopes.push(checkScope);
if (this.isCompatibleScope(scope)) {
this.scopes.push(scope);
} else {
break;
}
} while(checkScope = checkScope.parent);
if (this.breakOnScopePaths.indexOf(scope.path) >= 0) {
break;
}
} while(scope = scope.parent);
}
getAttachmentPath() {
var scopes = this.scopes;
var scope = scopes.pop();
if (!scope) return;
if (scope.path.isFunction()) {
if (this.hasNonParamBindings()) {
// can't be attached to this scope
return this.getNextScopeStatementParent();
} else {
if (this.hasOwnParamBindings(scope)) {
// should ignore this scope since it's ourselves
if (this.scope.is(scope)) return;
// needs to be attached to the body
return scope.path.get("body").get("body")[0];
} else {
// doesn't need to be be attached to this scope
return this.getNextScopeStatementParent();
}
} else if (scope.path.isProgram()) {
return this.getNextScopeStatementParent();
@@ -82,10 +87,12 @@ export default class PathHoister {
if (scope) return scope.path.getStatementParent();
}
hasNonParamBindings() {
hasOwnParamBindings(scope) {
for (var name in this.bindings) {
if (!scope.hasOwnBinding(name)) continue
var binding = this.bindings[name];
if (binding.kind !== "param") return true;
if (binding.kind === "param") return true;
}
return false;
}
@@ -96,7 +103,6 @@ export default class PathHoister {
node._hoisted = true;
this.path.traverse(referenceVisitor, this);
if (this.foundIncompatible) return;
this.getCompatibleScopes();
@@ -119,6 +125,6 @@ export default class PathHoister {
uid = t.jSXExpressionContainer(uid);
}
this.path.replaceWith(uid);
return uid;
}
}

View File

@@ -2,10 +2,13 @@ import * as t from "../../types";
export default class Binding {
constructor({ identifier, scope, path, kind }) {
this.constantViolations = [];
this.constant = true;
this.identifier = identifier;
this.references = 0;
this.referenced = false;
this.constant = true;
this.scope = scope;
this.path = path;
this.kind = kind;
@@ -51,8 +54,9 @@ export default class Binding {
* Description
*/
reassign() {
reassign(path) {
this.constant = false;
this.constantViolations.push(path);
if (this.typeAnnotationInferred) {
// destroy the inferred typeAnnotation

View File

@@ -163,6 +163,17 @@ export default class Scope {
traverse(node, opts, this, state, this.path);
}
/**
* Since `Scope` instances are unique to their traversal we need some other
* way to compare if scopes are the same. Here we just compare `this.bindings`
* as it will be the same across all instances.
*/
is(scope) {
return this.bindings === scope.bindings;
}
/**
* Description
*/
@@ -435,11 +446,13 @@ export default class Scope {
for (var name in ids) {
var binding = this.getBinding(name);
if (!binding) continue;
if (right) {
var rightType = right.typeAnnotation;
if (rightType && binding.isCompatibleWithType(rightType)) continue;
}
binding.reassign();
binding.reassign(left, right);
}
}