From 772f6f6d1b5cae438bd6f07834602cc095a9382c Mon Sep 17 00:00:00 2001 From: John Messerly Date: Wed, 20 May 2015 09:19:08 -0700 Subject: [PATCH] fixes #185, track const ctor dependencies, plus a static field fix R=vsm@google.com Review URL: https://codereview.chromium.org/1141663003 --- pkg/dev_compiler/lib/runtime/dart/convert.js | 38 +++++++++---------- .../lib/src/codegen/js_codegen.dart | 31 ++++++++++----- .../lib/src/dependency_graph.dart | 2 +- .../test/codegen/expect/fieldtest.js | 14 +++++++ .../test/codegen/expect/html_input.html | 1 + .../codegen/expect/sunflower/sunflower.html | 1 + pkg/dev_compiler/test/codegen/fieldtest.dart | 13 +++++++ 7 files changed, 70 insertions(+), 30 deletions(-) diff --git a/pkg/dev_compiler/lib/runtime/dart/convert.js b/pkg/dev_compiler/lib/runtime/dart/convert.js index 629444f14578..df5cfa88b786 100644 --- a/pkg/dev_compiler/lib/runtime/dart/convert.js +++ b/pkg/dev_compiler/lib/runtime/dart/convert.js @@ -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) { @@ -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'); @@ -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; diff --git a/pkg/dev_compiler/lib/src/codegen/js_codegen.dart b/pkg/dev_compiler/lib/src/codegen/js_codegen.dart index 2a85f77cae73..d4bf93ca59f4 100644 --- a/pkg/dev_compiler/lib/src/codegen/js_codegen.dart +++ b/pkg/dev_compiler/lib/src/codegen/js_codegen.dart @@ -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, @@ -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); @@ -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 fieldDecls) { - var unit = node.getAncestor((a) => a is CompilationUnit); + ClassDeclaration cls, List 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(); @@ -819,11 +827,8 @@ 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); @@ -831,7 +836,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor { } // 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); } @@ -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); } @@ -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)) { diff --git a/pkg/dev_compiler/lib/src/dependency_graph.dart b/pkg/dev_compiler/lib/src/dependency_graph.dart index 941df6d0ff18..6f78db184346 100644 --- a/pkg/dev_compiler/lib/src/dependency_graph.dart +++ b/pkg/dev_compiler/lib/src/dependency_graph.dart @@ -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', diff --git a/pkg/dev_compiler/test/codegen/expect/fieldtest.js b/pkg/dev_compiler/test/codegen/expect/fieldtest.js index 6fb5192f0a4d..c9b7decfaad0 100644 --- a/pkg/dev_compiler/test/codegen/expect/fieldtest.js +++ b/pkg/dev_compiler/test/codegen/expect/fieldtest.js @@ -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); @@ -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); diff --git a/pkg/dev_compiler/test/codegen/expect/html_input.html b/pkg/dev_compiler/test/codegen/expect/html_input.html index a5c8d4cae71e..b8057457974c 100644 --- a/pkg/dev_compiler/test/codegen/expect/html_input.html +++ b/pkg/dev_compiler/test/codegen/expect/html_input.html @@ -14,6 +14,7 @@ + diff --git a/pkg/dev_compiler/test/codegen/expect/sunflower/sunflower.html b/pkg/dev_compiler/test/codegen/expect/sunflower/sunflower.html index 91d3a5cbf249..b1a410ffae23 100644 --- a/pkg/dev_compiler/test/codegen/expect/sunflower/sunflower.html +++ b/pkg/dev_compiler/test/codegen/expect/sunflower/sunflower.html @@ -39,6 +39,7 @@

drfibonacci's Sunflower Spectacular

+ diff --git a/pkg/dev_compiler/test/codegen/fieldtest.dart b/pkg/dev_compiler/test/codegen/fieldtest.dart index 72df12e38cb7..77ceff397076 100644 --- a/pkg/dev_compiler/test/codegen/fieldtest.dart +++ b/pkg/dev_compiler/test/codegen/fieldtest.dart @@ -51,6 +51,19 @@ class Generic { 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);