Skip to content

Commit

Permalink
work around issue #51, super restrictions in V8
Browse files Browse the repository at this point in the history 
Adds an explicit "extends dart.Object", which we probably want for other reasons (e.g. toString, runtimeType, hashCode)

Attempts to recover some readability by tweaking the name of the initialize functions. This closes #51, but we should open another tracking bug because there are concerning aspects to this workaround.

R=sigmund@google.com, vsm@google.com

Review URL: https://chromereviews.googleplex.com/150417015
  • Loading branch information
John Messerly committed Feb 19, 2015
1 parent 260cfc0 commit 7c1ab1f
Show file tree
Hide file tree
Showing 17 changed files with 811 additions and 804 deletions.
54 changes: 44 additions & 10 deletions pkg/dev_compiler/lib/runtime/dart_runtime.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,22 +191,34 @@ var dart;
* superclass (prototype).
*/
function mixin(base/*, ...mixins*/) {
// Build the mixin constructor. This runs the superclass as well as each
// of the mixins' constructors.
var mixins = Array.prototype.slice.call(arguments, 1);
function Mixin() {
base.apply(this, arguments);
// Run mixin constructors. They cannot have arguments.
for (var i = 0; i< mixins.length; i++) mixins[i].call(this);
}
// Inherit statics from Base to simulate ES6 class inheritance
// Conceptually this is: `class Mixin extends base {}`
function Mixin() {
// TODO(jmesserly): since we're using initializers and not constructors,
// we can just skip directly to DartObject.
DartObject.apply(this, arguments);
}
Mixin.__proto__ = base;
Mixin.prototype = Object.create(base.prototype);
Mixin.prototype.constructor = Mixin;
// Copy each mixin, with later ones overwriting earlier entries.
for (var i = 0; i< mixins.length; i++) {
var mixins = Array.prototype.slice.call(arguments, 1);
for (var i = 0; i < mixins.length; i++) {
copyProperties(Mixin.prototype, mixins[i].prototype);
}
// Create an initializer for the mixin, so when derived constructor calls
// super, we can correctly initialize base and mixins.
var baseCtor = base.prototype[base.name];
Mixin.prototype[base.name] = function() {
// Run mixin initializers. They cannot have arguments.
// Run them backwards so most-derived mixin is initialized first.
for (var i = mixins.length - 1; i >= 0; i--) {
var mixin = mixins[i];
mixin.prototype[mixin.name].call(this);
}
// Run base initializer.
baseCtor.apply(this, arguments);
}
return Mixin;
}
dart.mixin = mixin;
Expand Down Expand Up @@ -256,7 +268,7 @@ var dart;
*/
function defineNamedConstructor(clazz, name) {
var proto = clazz.prototype;
var initMethod = proto[name];
var initMethod = proto[clazz.name + '$' + name];
var ctor = function() { return initMethod.apply(this, arguments); }
ctor.prototype = proto;
clazz[name] = ctor;
Expand Down Expand Up @@ -310,4 +322,26 @@ var dart;
}
dart.generic = generic;


/**
* Implements Dart constructor behavior. Because of V8 `super` [constructor
* restrictions](https://code.google.com/p/v8/issues/detail?id=3330#c65) we
* cannot currently emit actual ES6 constructors with super calls. Instead
* we use the same trick as named constructors, and do them as instance
* methods that perform initialization.
*/
// TODO(jmesserly): we'll need to rethink this once the ES6 spec and V8
// settles. See <https://github.com/dart-lang/dart-dev-compiler/issues/51>.
// Performance of this pattern is likely to be bad.
dart.Object = function Object() {
// Get the class name for this instance.
var name = this.constructor.name;
// Call the default constructor.
var result = this[name].apply(this, arguments);
return result === void 0 ? this : result;
};
// The initializer for dart.Object
dart.Object.prototype.Object = function() {};
dart.Object.prototype.constructor = dart.Object;

})(dart || (dart = {}));
78 changes: 36 additions & 42 deletions pkg/dev_compiler/lib/src/codegen/js_codegen.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,22 +240,20 @@ var $_libraryName;
currentClass = node;

var name = node.name.name;

_beginTypeParameters(node.typeParameters, name);
out.write('class $name');
out.write('class $name extends ');

if (node.withClause != null) {
out.write(' extends dart.mixin(');
if (node.extendsClause != null) {
_visitNode(node.extendsClause.superclass);
} else {
out.write('Object');
}
out.write('dart.mixin(');
}
if (node.extendsClause != null) {
_visitNode(node.extendsClause.superclass);
} else {
out.write('dart.Object');
}
if (node.withClause != null) {
_visitNodeList(node.withClause.mixinTypes, prefix: ', ', separator: ', ');
out.write(')');
} else if (node.extendsClause != null) {
out.write(' extends ');
_visitNode(node.extendsClause.superclass);
}

out.write(' {\n', 2);
Expand All @@ -274,7 +272,7 @@ var $_libraryName;
// Iff no constructor is specified for a class C, it implicitly has a
// default constructor `C() : super() {}`, unless C is class Object.
if (ctors.isEmpty && !node.element.type.isObject) {
_generateImplicitConstructor(node, fields);
_generateImplicitConstructor(node, name, fields);
}

for (var member in node.members) {
Expand Down Expand Up @@ -344,12 +342,13 @@ $name.prototype[Symbol.iterator] = function() {
/// Generates the implicit default constructor for class C of the form
/// `C() : super() {}`.
void _generateImplicitConstructor(
ClassDeclaration node, List<FieldDeclaration> fields) {
// If we don't have a method body, use the implicit JS ctor.
ClassDeclaration node, String name, List<FieldDeclaration> fields) {
// If we don't have a method body, skip this.
if (fields.isEmpty) return;
out.write('constructor() {\n', 2);

out.write('$name() {\n', 2);
_initializeFields(fields);
out.write('super();\n');
_superConstructorCall(node);
out.write('}\n', -2);
}

Expand All @@ -360,13 +359,14 @@ $name.prototype[Symbol.iterator] = function() {
return;
}

// We generate constructors as initializer methods in the class;
// this allows use of `super` for instance methods/properties.
// It also avoids V8 restrictions on `super` in default constructors.
out.write(className);
if (node.name != null) {
// We generate named constructors as initializer methods in the class;
// this allows use of `super` for instance methods/properties.
out.write('/*constructor*/ ${node.name.name}(');
} else {
out.write('constructor(');
out.write('\$${node.name.name}');
}
out.write('(');
_visitNode(node.parameters);
out.write(') {\n', 2);
_generateConstructorBody(node, fields);
Expand Down Expand Up @@ -414,14 +414,11 @@ $name.prototype[Symbol.iterator] = function() {
// If no superinitializer is provided, an implicit superinitializer of the
// form `super()` is added at the end of the initializer list, unless the
// enclosing class is class Object.
ClassElement element = (node.parent as ClassDeclaration).element;
if (superCall == null) {
if (!element.type.isObject && !element.supertype.isObject) {
_superConstructorCall(node);
}
_superConstructorCall(node.parent);
} else {
_superConstructorCall(
node, superCall.constructorName, superCall.argumentList);
_superConstructorCall(node.parent, node.name, superCall.constructorName,
superCall.argumentList);
}
}

Expand Down Expand Up @@ -450,22 +447,19 @@ $name.prototype[Symbol.iterator] = function() {
out.write(');\n');
}

void _superConstructorCall(ConstructorDeclaration ctor,
[SimpleIdentifier superName, ArgumentList args]) {

// If we're calling default super from a named initializer method, we need
// to do ES5 style `TypeName.call(this, <args>)`, otherwise we use `super`.
if (ctor.name != null && superName == null) {
_writeTypeName((ctor.parent as ClassDeclaration).element.supertype);
out.write('.call(this');
if (args != null && args.arguments.isNotEmpty) out.write(', ');
_visitNode(args);
} else {
out.write('super');
if (superName != null) out.write('.${superName.name}');
out.write('(');
_visitNode(args);
void _superConstructorCall(ClassDeclaration clazz, [SimpleIdentifier ctorName,
SimpleIdentifier superCtorName, ArgumentList args]) {
var element = clazz.element;
if (superCtorName == null &&
(element.type.isObject || element.supertype.isObject)) {
return;
}

var supertypeName = element.supertype.name;
out.write('super.$supertypeName');
if (superCtorName != null) out.write('\$${superCtorName.name}');
out.write('(');
_visitNode(args);
out.write(');\n');
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/dev_compiler/test/codegen/expect/BenchmarkBase/BenchmarkBase.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
var BenchmarkBase;
(function (BenchmarkBase) {
'use strict';
class Expect {
class Expect extends dart.Object {
static equals(expected, actual) {
if (!dart.equals(expected, actual)) {
throw `Values not equal: ${expected} vs ${actual}`;
Expand All @@ -20,8 +20,8 @@ var BenchmarkBase;
}
}

class BenchmarkBase {
constructor(name) {
class BenchmarkBase extends dart.Object {
BenchmarkBase(name) {
this.name = name;
}
run() {
Expand Down
50 changes: 24 additions & 26 deletions pkg/dev_compiler/test/codegen/expect/DeltaBlue/DeltaBlue.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,17 @@ var DeltaBlue;
}

class DeltaBlue extends BenchmarkBase.BenchmarkBase {
constructor() {
super("DeltaBlue");
DeltaBlue() {
super.BenchmarkBase("DeltaBlue");
}
run() {
chainTest(100);
projectionTest(100);
}
}

class Strength {
constructor(value, name) {
class Strength extends dart.Object {
Strength(value, name) {
this.value = value;
this.name = name;
}
Expand All @@ -43,8 +43,8 @@ var DeltaBlue;
let NORMAL = new Strength(4, "normal");
let WEAK_DEFAULT = new Strength(5, "weakDefault");
let WEAKEST = new Strength(6, "weakest");
class Constraint {
constructor(strength) {
class Constraint extends dart.Object {
Constraint(strength) {
this.strength = strength;
}
addConstraint() {
Expand Down Expand Up @@ -76,10 +76,10 @@ var DeltaBlue;
}

class UnaryConstraint extends Constraint {
constructor(myOutput, strength) {
UnaryConstraint(myOutput, strength) {
this.myOutput = myOutput;
this.satisfied = false;
super(strength);
super.Constraint(strength);
this.addConstraint();
}
addToGraph() {
Expand Down Expand Up @@ -109,16 +109,16 @@ var DeltaBlue;
}

class StayConstraint extends UnaryConstraint {
constructor(v, str) {
super(v, str);
StayConstraint(v, str) {
super.UnaryConstraint(v, str);
}
execute() {
}
}

class EditConstraint extends UnaryConstraint {
constructor(v, str) {
super(v, str);
EditConstraint(v, str) {
super.UnaryConstraint(v, str);
}
isInput() { return true; }
execute() {
Expand All @@ -129,11 +129,11 @@ var DeltaBlue;
let FORWARD = 2;
let BACKWARD = 0;
class BinaryConstraint extends Constraint {
constructor(v1, v2, strength) {
BinaryConstraint(v1, v2, strength) {
this.v1 = v1;
this.v2 = v2;
this.direction = NONE;
super(strength);
super.Constraint(strength);
this.addConstraint();
}
chooseMethod(mark) {
Expand Down Expand Up @@ -181,10 +181,10 @@ var DeltaBlue;
}

class ScaleConstraint extends BinaryConstraint {
constructor(src, scale, offset, dest, strength) {
ScaleConstraint(src, scale, offset, dest, strength) {
this.scale = scale;
this.offset = offset;
super(src, dest, strength);
super.BinaryConstraint(src, dest, strength);
}
addToGraph() {
super.addToGraph();
Expand Down Expand Up @@ -216,16 +216,16 @@ var DeltaBlue;
}

class EqualityConstraint extends BinaryConstraint {
constructor(v1, v2, strength) {
super(v1, v2, strength);
EqualityConstraint(v1, v2, strength) {
super.BinaryConstraint(v1, v2, strength);
}
execute() {
this.output().value = this.input().value;
}
}

class Variable {
constructor(name, value) {
class Variable extends dart.Object {
Variable(name, value) {
this.constraints = new List.from([]);
this.name = name;
this.value = value;
Expand All @@ -243,10 +243,9 @@ var DeltaBlue;
}
}

class Planner {
constructor() {
class Planner extends dart.Object {
Planner() {
this.currentMark = 0;
super();
}
incrementalAdd(c) {
let mark = this.newMark();
Expand Down Expand Up @@ -335,10 +334,9 @@ var DeltaBlue;
}
}

class Plan {
constructor() {
class Plan extends dart.Object {
Plan() {
this.list = new List.from([]);
super();
}
addConstraint(c) {
this.list.add(c);
Expand Down

0 comments on commit 7c1ab1f

Please sign in to comment.