Skip to content

Commit

Permalink
fixes #185, track const ctor dependencies, plus a static field fix
Browse files Browse the repository at this point in the history
R=vsm@google.com

Review URL: https://codereview.chromium.org/1141663003
  • Loading branch information
John Messerly committed May 20, 2015
1 parent 8040611 commit 772f6f6
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 30 deletions.
38 changes: 19 additions & 19 deletions pkg/dev_compiler/lib/runtime/dart/convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,24 @@ var collection = dart.import(collection);
},
set _nameToEncoding(_) {}
});
let _name = Symbol('_name');
class HtmlEscapeMode extends core.Object {
_(name, escapeLtGt, escapeQuot, escapeApos, escapeSlash) {
this[_name] = name;
this.escapeLtGt = escapeLtGt;
this.escapeQuot = escapeQuot;
this.escapeApos = escapeApos;
this.escapeSlash = escapeSlash;
}
toString() {
return this[_name];
}
}
dart.defineNamedConstructor(HtmlEscapeMode, '_');
dart.setSignature(HtmlEscapeMode, {
methods: () => ({toString: dart.functionType(core.String, [])})
});
HtmlEscapeMode.UNKNOWN = dart.const(new HtmlEscapeMode._('unknown', true, true, true, true));
let _convert = Symbol('_convert');
class HtmlEscape extends Converter$(core.String, core.String) {
HtmlEscape(mode) {
Expand Down Expand Up @@ -702,24 +720,6 @@ var collection = dart.import(collection);
})
});
let HTML_ESCAPE = dart.const(new HtmlEscape());
let _name = Symbol('_name');
class HtmlEscapeMode extends core.Object {
_(name, escapeLtGt, escapeQuot, escapeApos, escapeSlash) {
this[_name] = name;
this.escapeLtGt = escapeLtGt;
this.escapeQuot = escapeQuot;
this.escapeApos = escapeApos;
this.escapeSlash = escapeSlash;
}
toString() {
return this[_name];
}
}
dart.defineNamedConstructor(HtmlEscapeMode, '_');
dart.setSignature(HtmlEscapeMode, {
methods: () => ({toString: dart.functionType(core.String, [])})
});
HtmlEscapeMode.UNKNOWN = dart.const(new HtmlEscapeMode._('unknown', true, true, true, true));
HtmlEscapeMode.ATTRIBUTE = dart.const(new HtmlEscapeMode._('attribute', false, true, false, false));
HtmlEscapeMode.ELEMENT = dart.const(new HtmlEscapeMode._('element', true, false, false, true));
let _escape = Symbol('_escape');
Expand Down Expand Up @@ -2807,9 +2807,9 @@ var collection = dart.import(collection);
exports.ChunkedConversionSink = ChunkedConversionSink;
exports.ByteConversionSink = ByteConversionSink;
exports.ByteConversionSinkBase = ByteConversionSinkBase;
exports.HtmlEscapeMode = HtmlEscapeMode;
exports.HtmlEscape = HtmlEscape;
exports.HTML_ESCAPE = HTML_ESCAPE;
exports.HtmlEscapeMode = HtmlEscapeMode;
exports.JsonUnsupportedObjectError = JsonUnsupportedObjectError;
exports.JsonCyclicError = JsonCyclicError;
exports.JsonCodec = JsonCodec;
Expand Down
31 changes: 21 additions & 10 deletions pkg/dev_compiler/lib/src/codegen/js_codegen.dart
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,12 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
// Generate optional/named argument value assignment. These can not have
// side effects, and may be used by the constructor's initializers, so it's
// nice to do them first.
// Also for const constructors we need to ensure default values are
// available for use by top-level constant initializers.
ClassDeclaration cls = node.parent;
if (node.constKeyword != null) _loader.startTopLevel(cls.element);
var init = _emitArgumentInitializers(node, constructor: true);
if (node.constKeyword != null) _loader.finishTopLevel(cls.element);
if (init != null) body.add(init);

// Redirecting constructors: these are not allowed to have initializers,
Expand All @@ -735,7 +740,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
// These are expanded into each non-redirecting constructor.
// In the future we may want to create an initializer function if we have
// multiple constructors, but it needs to be balanced against readability.
body.add(_initializeFields(node, fields));
body.add(_initializeFields(node.parent, fields, node));

var superCall = node.initializers.firstWhere(
(i) => i is SuperConstructorInvocation, orElse: () => null);
Expand Down Expand Up @@ -800,9 +805,12 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
/// 3. constructor field initializers,
/// 4. initialize fields not covered in 1-3
JS.Statement _initializeFields(
AstNode node, List<FieldDeclaration> fieldDecls) {
var unit = node.getAncestor((a) => a is CompilationUnit);
ClassDeclaration cls, List<FieldDeclaration> fieldDecls,
[ConstructorDeclaration ctor]) {
var unit = cls.getAncestor((a) => a is CompilationUnit);
var constField = new ConstFieldVisitor(types, unit);
bool isConst = ctor != null && ctor.constKeyword != null;
if (isConst) _loader.startTopLevel(cls.element);

// Run field initializers if they can have side-effects.
var fields = new Map<FieldElement, JS.Expression>();
Expand All @@ -819,19 +827,16 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
}

// Initialize fields from `this.fieldName` parameters.
if (node is ConstructorDeclaration) {
var parameters = node.parameters;
var initializers = node.initializers;

for (var p in parameters.parameters) {
if (ctor != null) {
for (var p in ctor.parameters.parameters) {
var element = p.element;
if (element is FieldFormalParameterElement) {
fields[element.field] = _visit(p);
}
}

// Run constructor field initializers such as `: foo = bar.baz`
for (var init in initializers) {
for (var init in ctor.initializers) {
if (init is ConstructorFieldInitializer) {
fields[init.fieldName.staticElement] = _visit(init.expression);
}
Expand Down Expand Up @@ -860,6 +865,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
var access = _emitMemberName(e.name, type: e.enclosingElement.type);
body.add(js.statement('this.# = #;', [access, initialValue]));
});

if (isConst) _loader.finishTopLevel(cls.element);
return _statement(body);
}

Expand Down Expand Up @@ -1981,7 +1988,11 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
/// Shared code for [PrefixedIdentifier] and [PropertyAccess].
JS.Expression _emitGet(Expression target, SimpleIdentifier memberId) {
var member = memberId.staticElement;
bool isStatic = member is ExecutableElement && member.isStatic;
if (member is PropertyAccessorElement) member = member.variable;
bool isStatic = member is ClassMemberElement && member.isStatic;
if (isStatic) {
_loader.declareBeforeUse(member);
}
var name = _emitMemberName(memberId.name,
type: getStaticType(target), isStatic: isStatic);
if (rules.isDynamicTarget(target)) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/dev_compiler/lib/src/dependency_graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -503,10 +503,10 @@ const corelibOrder = const [
'dart.typed_data',
'dart._isolate_helper',
'dart._js_primitives',
'dart.convert',

// TODO(jmesserly): add others
/*
'dart.convert',
'dart._foreign_helper',
'dart._interceptors',
'dart._native_typed_data',
Expand Down
14 changes: 14 additions & 0 deletions pkg/dev_compiler/test/codegen/expect/fieldtest.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,18 @@ var core = dart.import(core);
});
let Generic = Generic$();
Generic.bar = 'hello';
class StaticFieldOrder1 extends core.Object {}
dart.setSignature(StaticFieldOrder1, {});
StaticFieldOrder1.d = 4;
StaticFieldOrder1.c = dart.notNull(StaticFieldOrder1.d) + 2;
StaticFieldOrder1.b = dart.notNull(StaticFieldOrder1.c) + 3;
StaticFieldOrder1.a = dart.notNull(StaticFieldOrder1.b) + 1;
class StaticFieldOrder2 extends core.Object {}
dart.setSignature(StaticFieldOrder2, {});
StaticFieldOrder2.d = 4;
StaticFieldOrder2.c = dart.notNull(StaticFieldOrder2.d) + 2;
StaticFieldOrder2.b = dart.notNull(StaticFieldOrder2.c) + 3;
StaticFieldOrder2.a = dart.notNull(StaticFieldOrder2.b) + 1;
function main() {
let a = new A();
foo(a);
Expand All @@ -103,5 +115,7 @@ var core = dart.import(core);
exports.Derived = Derived;
exports.Generic$ = Generic$;
exports.Generic = Generic;
exports.StaticFieldOrder1 = StaticFieldOrder1;
exports.StaticFieldOrder2 = StaticFieldOrder2;
exports.main = main;
})(fieldtest, core);
1 change: 1 addition & 0 deletions pkg/dev_compiler/test/codegen/expect/html_input.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
<script src="dev_compiler/runtime/dart/typed_data.js"></script>
<script src="dev_compiler/runtime/dart/_isolate_helper.js"></script>
<script src="dev_compiler/runtime/dart/_js_primitives.js"></script>
<script src="dev_compiler/runtime/dart/convert.js"></script>
<script src="dir/html_input_d.js"></script>
<script src="dir/html_input_b.js"></script>
<script src="dir/html_input_e.js"></script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ <h1>drfibonacci's Sunflower Spectacular</h1>
<script src="dev_compiler/runtime/dart/typed_data.js"></script>
<script src="dev_compiler/runtime/dart/_isolate_helper.js"></script>
<script src="dev_compiler/runtime/dart/_js_primitives.js"></script>
<script src="dev_compiler/runtime/dart/convert.js"></script>
<script src="dom.js"></script>
<script src="sunflower.js"></script>
<script>_isolate_helper.startRootIsolate(sunflower.main, []);</script>
Expand Down
13 changes: 13 additions & 0 deletions pkg/dev_compiler/test/codegen/fieldtest.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,19 @@ class Generic<T> {
static String bar = 'hello';
}

class StaticFieldOrder1 {
static const a = b + 1;
static const c = d + 2;
static const b = c + 3;
static const d = 4;
}
class StaticFieldOrder2 {
static const a = StaticFieldOrder2.b + 1;
static const c = StaticFieldOrder2.d + 2;
static const b = StaticFieldOrder2.c + 3;
static const d = 4;
}

void main() {
var a = new A();
foo(a);
Expand Down

0 comments on commit 772f6f6

Please sign in to comment.