Skip to content

Commit

Permalink
Fix class support inside generators
Browse files Browse the repository at this point in the history
  • Loading branch information
@nicolo-ribaudo @benjamn
nicolo-ribaudo authored and benjamn committed Jul 23, 2021
1 parent 4599723 commit 0164e2a
Show file tree
Hide file tree
Showing 4 changed files with 185 additions and 60 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Expand Up @@ -5,6 +5,7 @@ test/mocha.js
test/mocha.css

test/*.es5.js
test/*.regenerator.js
test/tests.browser.js

.idea
158 changes: 98 additions & 60 deletions packages/transform/src/emit.js
View file
Expand Up @@ -764,6 +764,10 @@ Ep.explodeStatement = function(path, labelId) {

break;

case "ClassDeclaration":
self.emit(self.explodeClass(path));
break;

default:
throw new Error(
"unknown Statement of type " +
Expand Down Expand Up @@ -900,6 +904,50 @@ Ep.updateContextPrevLoc = function(loc) {
this.emitAssign(this.contextProperty("prev"), loc);
};


// In order to save the rest of explodeExpression from a combinatorial
// trainwreck of special cases, explodeViaTempVar is responsible for
// deciding when a subexpression needs to be "exploded," which is my
// very technical term for emitting the subexpression as an assignment
// to a temporary variable and the substituting the temporary variable
// for the original subexpression. Think of exploded view diagrams, not
// Michael Bay movies. The point of exploding subexpressions is to
// control the precise order in which the generated code realizes the
// side effects of those subexpressions.
Ep.explodeViaTempVar = function(tempVar, childPath, hasLeapingChildren, ignoreChildResult) {
assert.ok(
!ignoreChildResult || !tempVar,
"Ignoring the result of a child expression but forcing it to " +
"be assigned to a temporary variable?"
);
const t = util.getTypes();

let result = this.explodeExpression(childPath, ignoreChildResult);

if (ignoreChildResult) {
// Side effects already emitted above.

} else if (tempVar || (hasLeapingChildren &&
!t.isLiteral(result))) {
// If tempVar was provided, then the result will always be assigned
// to it, even if the result does not otherwise need to be assigned
// to a temporary variable. When no tempVar is provided, we have
// the flexibility to decide whether a temporary variable is really
// necessary. Unfortunately, in general, a temporary variable is
// required whenever any child contains a yield expression, since it
// is difficult to prove (at all, let alone efficiently) whether
// this result would evaluate to the same value before and after the
// yield (see #206). One narrow case where we can prove it doesn't
// matter (and thus we do not need a temporary variable) is when the
// result in question is a Literal value.
result = this.emitAssign(
tempVar || this.makeTempVar(),
result
);
}
return result;
};

Ep.explodeExpression = function(path, ignoreResult) {
const t = util.getTypes();
let expr = path.node;
Expand All @@ -917,6 +965,7 @@ Ep.explodeExpression = function(path, ignoreResult) {
t.assertExpression(expr);
if (ignoreResult) {
self.emit(expr);
return expr;
} else {
return expr;
}
Expand All @@ -934,48 +983,6 @@ Ep.explodeExpression = function(path, ignoreResult) {
// side effects relative to the leaping child(ren).
let hasLeapingChildren = meta.containsLeap.onlyChildren(expr);

// In order to save the rest of explodeExpression from a combinatorial
// trainwreck of special cases, explodeViaTempVar is responsible for
// deciding when a subexpression needs to be "exploded," which is my
// very technical term for emitting the subexpression as an assignment
// to a temporary variable and the substituting the temporary variable
// for the original subexpression. Think of exploded view diagrams, not
// Michael Bay movies. The point of exploding subexpressions is to
// control the precise order in which the generated code realizes the
// side effects of those subexpressions.
function explodeViaTempVar(tempVar, childPath, ignoreChildResult) {
assert.ok(
!ignoreChildResult || !tempVar,
"Ignoring the result of a child expression but forcing it to " +
"be assigned to a temporary variable?"
);

let result = self.explodeExpression(childPath, ignoreChildResult);

if (ignoreChildResult) {
// Side effects already emitted above.

} else if (tempVar || (hasLeapingChildren &&
!t.isLiteral(result))) {
// If tempVar was provided, then the result will always be assigned
// to it, even if the result does not otherwise need to be assigned
// to a temporary variable. When no tempVar is provided, we have
// the flexibility to decide whether a temporary variable is really
// necessary. Unfortunately, in general, a temporary variable is
// required whenever any child contains a yield expression, since it
// is difficult to prove (at all, let alone efficiently) whether
// this result would evaluate to the same value before and after the
// yield (see #206). One narrow case where we can prove it doesn't
// matter (and thus we do not need a temporary variable) is when the
// result in question is a Literal value.
result = self.emitAssign(
tempVar || self.makeTempVar(),
result
);
}
return result;
}

// If ignoreResult is true, then we must take full responsibility for
// emitting the expression with all its side effects, and we should not
// return a result.
Expand All @@ -985,7 +992,7 @@ Ep.explodeExpression = function(path, ignoreResult) {
return finish(t.memberExpression(
self.explodeExpression(path.get("object")),
expr.computed
? explodeViaTempVar(null, path.get("property"))
? self.explodeViaTempVar(null, path.get("property"), hasLeapingChildren)
: expr.property,
expr.computed
));
Expand All @@ -1011,15 +1018,16 @@ Ep.explodeExpression = function(path, ignoreResult) {
// expression, then we must be careful that the object of the
// member expression still gets bound to `this` for the call.

let newObject = explodeViaTempVar(
let newObject = self.explodeViaTempVar(
// Assign the exploded callee.object expression to a temporary
// variable so that we can use it twice without reevaluating it.
self.makeTempVar(),
calleePath.get("object")
calleePath.get("object"),
hasLeapingChildren
);

let newProperty = calleePath.node.computed
? explodeViaTempVar(null, calleePath.get("property"))
? self.explodeViaTempVar(null, calleePath.get("property"), hasLeapingChildren)
: calleePath.node.property;

injectFirstArg = newObject;
Expand All @@ -1039,7 +1047,7 @@ Ep.explodeExpression = function(path, ignoreResult) {
}

} else {
newCallee = explodeViaTempVar(null, calleePath);
newCallee = self.explodeViaTempVar(null, calleePath, hasLeapingChildren);

if (t.isMemberExpression(newCallee)) {
// If the callee was not previously a MemberExpression, then the
Expand All @@ -1058,7 +1066,7 @@ Ep.explodeExpression = function(path, ignoreResult) {
}

if (hasLeapingArgs) {
newArgs = argsPath.map(argPath => explodeViaTempVar(null, argPath));
newArgs = argsPath.map(argPath => self.explodeViaTempVar(null, argPath, hasLeapingChildren));
if (injectFirstArg) newArgs.unshift(injectFirstArg);

newArgs = newArgs.map(arg => t.cloneDeep(arg));
Expand All @@ -1070,9 +1078,9 @@ Ep.explodeExpression = function(path, ignoreResult) {

case "NewExpression":
return finish(t.newExpression(
explodeViaTempVar(null, path.get("callee")),
path.get("arguments").map(function(argPath) {
return explodeViaTempVar(null, argPath);
self.explodeViaTempVar(null, path.get("callee"), hasLeapingChildren),
path.get("arguments").map(function(argPath) {
return self.explodeViaTempVar(null, argPath, hasLeapingChildren);
})
));

Expand All @@ -1082,7 +1090,7 @@ Ep.explodeExpression = function(path, ignoreResult) {
if (propPath.isObjectProperty()) {
return t.objectProperty(
propPath.node.key,
explodeViaTempVar(null, propPath.get("value")),
self.explodeViaTempVar(null, propPath.get("value"), hasLeapingChildren),
propPath.node.computed
);
} else {
Expand All @@ -1096,10 +1104,10 @@ Ep.explodeExpression = function(path, ignoreResult) {
path.get("elements").map(function(elemPath) {
if (elemPath.isSpreadElement()) {
return t.spreadElement(
explodeViaTempVar(null, elemPath.get("argument"))
self.explodeViaTempVar(null, elemPath.get("argument"), hasLeapingChildren)
);
} else {
return explodeViaTempVar(null, elemPath);
return self.explodeViaTempVar(null, elemPath, hasLeapingChildren);
}
})
));
Expand All @@ -1124,7 +1132,7 @@ Ep.explodeExpression = function(path, ignoreResult) {
result = self.makeTempVar();
}

let left = explodeViaTempVar(result, path.get("left"));
let left = self.explodeViaTempVar(result, path.get("left"), hasLeapingChildren);

if (expr.operator === "&&") {
self.jumpIfNot(left, after);
Expand All @@ -1133,7 +1141,7 @@ Ep.explodeExpression = function(path, ignoreResult) {
self.jumpIf(left, after);
}

explodeViaTempVar(result, path.get("right"), ignoreResult);
self.explodeViaTempVar(result, path.get("right"), hasLeapingChildren, ignoreResult);

self.mark(after);

Expand All @@ -1150,11 +1158,11 @@ Ep.explodeExpression = function(path, ignoreResult) {
result = self.makeTempVar();
}

explodeViaTempVar(result, path.get("consequent"), ignoreResult);
self.explodeViaTempVar(result, path.get("consequent"), hasLeapingChildren, ignoreResult);
self.jump(after);

self.mark(elseLoc);
explodeViaTempVar(result, path.get("alternate"), ignoreResult);
self.explodeViaTempVar(result, path.get("alternate"), hasLeapingChildren, ignoreResult);

self.mark(after);

Expand All @@ -1172,8 +1180,8 @@ Ep.explodeExpression = function(path, ignoreResult) {
case "BinaryExpression":
return finish(t.binaryExpression(
expr.operator,
explodeViaTempVar(null, path.get("left")),
explodeViaTempVar(null, path.get("right"))
self.explodeViaTempVar(null, path.get("left"), hasLeapingChildren),
self.explodeViaTempVar(null, path.get("right"), hasLeapingChildren)
));

case "AssignmentExpression":
Expand Down Expand Up @@ -1254,9 +1262,39 @@ Ep.explodeExpression = function(path, ignoreResult) {

return self.contextProperty("sent");

case "ClassExpression":
return finish(self.explodeClass(path));

default:
throw new Error(
"unknown Expression of type " +
JSON.stringify(expr.type));
}
};

Ep.explodeClass = function(path) {
const explodingChildren = [];

if (path.node.superClass) {
explodingChildren.push(path.get("superClass"));
}
for (let i = 0; i < path.node.body.body.length; i++) {
const member = path.get("body.body")[i];
if (member.node.computed) {
explodingChildren.push(member.get("key"));
}
}

for (let i = 0; i < explodingChildren.length; i++) {
const child = explodingChildren[i];
const isLast = i === explodingChildren.length - 1;

if (isLast) {
child.replaceWith(this.explodeExpression(child));
} else {
child.replaceWith(this.explodeViaTempVar(null, child, true));
}
}

return path.node;
};
37 changes: 37 additions & 0 deletions test/class.js
View file
Expand Up @@ -68,4 +68,41 @@ describe("class methods", function () {
assert.strictEqual(args[1], 2);
assert.strictEqual(args[2], 3)
});

it("should allow yield as super expression", function () {
function* gen() {
return class extends (yield) {}
}

class B {}

const it = gen();
it.next();
const res = it.next(B).value;

assert.ok(new res instanceof B);
});

it("should allow yield as computed key", function () {
if (class {}.toString().indexOf("class") !== 0) {
return;
// The class transform is broken:
// https://github.com/babel/babel/issues/8300
}

function* gen() {
return class {
[yield]() { return 1 }
[yield]() { return 2 }
}
}

const it = gen();
it.next();
it.next("one");
const res = it.next("two").value;

assert.strictEqual(new res().one(), 1);
assert.strictEqual(new res().two(), 2);
});
});
49 changes: 49 additions & 0 deletions test/run.js
View file
Expand Up @@ -129,6 +129,47 @@ enqueue(convert, [
"./test/class.es5.js"
]);

enqueue(convertWithRegeneratorPluginOnly, [
"./test/class.js",
"./test/class.regenerator.js"
]);

Error.stackTraceLimit = 1000;

/**
* Comvert without using the preset (which also transforms things like classes and arrows)
*/
function convertWithRegeneratorPluginOnly(inputFile, outputFile, callback) {
var transformOptions = {
plugins:[require("regenerator-transform")],
parserOpts: {
sourceType: "module",
allowImportExportEverywhere: true,
allowReturnOutsideFunction: true,
allowSuperOutsideMethod: true,
strictMode: false,
plugins: ["*", "jsx", "flow"]
},
ast: true
};

fs.readFile(inputFile, "utf-8", function(err, input) {
if (err) {
return callback(err);
}

var { code: output, ast } = babel.transformSync(input, transformOptions);
fs.writeFileSync(outputFile, output);
try {
checkDuplicatedNodes(babel, ast);
} catch (err) {
err.message = "Occured while transforming: " + inputFile + "\n" + err.message;
throw err;
}
callback();
});
}

function convertWithParamsTransform(es6File, es5File, callback) {
var transformOptions = {
presets:[require("regenerator-preset")],
Expand Down Expand Up @@ -343,6 +384,14 @@ enqueue("mocha", [
"./test/async.es5.js",
]);

if (semver.gte(process.version, "6.0.0")) {
enqueue("mocha", [
"--harmony",
"--reporter", "spec",
"--require", "./test/runtime.js",
"./test/class.regenerator.js",
]);
}
enqueue("mocha", [
"--harmony",
"--reporter", "spec",
Expand Down

0 comments on commit 0164e2a

Please sign in to comment.