fix #569, cache constants defined in methods

R=leafp@google.com, sra@google.com

Review URL: https://codereview.chromium.org/1979013003 .

Author: John Messerly

pkg/dev_compiler/lib/runtime/dart_sdk.js

@@ -127,6 +127,8 @@ class CodeGenerator extends GeneralizingAstVisitor
   List<JS.TemporaryId> _superHelperSymbols = <JS.TemporaryId>[];
   List<JS.Method> _superHelpers = <JS.Method>[];
 
+  List<TypeParameterType> _typeParamInConst = null;
+
   /// Whether we are currently generating code for the body of a `JS()` call.
   bool _isInForeignJS = false;
 
@@ -2307,6 +2309,7 @@ class CodeGenerator extends GeneralizingAstVisitor
     }
 
     if (type is TypeParameterType) {
+      _typeParamInConst?.add(type);
       return new JS.Identifier(name);
     }
 
@@ -3392,9 +3395,22 @@ class CodeGenerator extends GeneralizingAstVisitor
   }
 
   JS.Expression _emitConst(JS.Expression expr()) {
-    // TODO(jmesserly): emit the constants at top level if possible.
-    // This wasn't quite working, so disabled for now.
-    return js.call('dart.const(#)', expr());
+    var savedTypeParams = _typeParamInConst;
+    _typeParamInConst = [];
+
+    var jsExpr = js.call('dart.const(#)', expr());
+
+    bool usesTypeParams = _typeParamInConst.isNotEmpty;
+    _typeParamInConst = savedTypeParams;
+
+    // TODO(jmesserly): if it uses type params we can still hoist it up as far
+    // as it will go, e.g. at the level the generic class is defined where type
+    // params are available.
+    if (_currentFunction == null || usesTypeParams) return jsExpr;
+
+    var temp = new JS.TemporaryId('const');
+    _moduleItems.add(js.statement('let #;', [temp]));
+    return js.call('# || (# = #)', [temp, temp, jsExpr]);
   }
 
   /// Returns a new expression, which can be be used safely *once* on the

pkg/dev_compiler/lib/src/compiler/code_generator.dart

@@ -5,9 +5,16 @@
 /// Helpers for Analyzer's Element model and corelib model.
 
 import 'package:analyzer/dart/ast/ast.dart'
-    show Expression, MethodInvocation, SimpleIdentifier;
+    show
+        ConstructorDeclaration,
+        Expression,
+        FunctionBody,
+        FunctionExpression,
+        MethodDeclaration,
+        MethodInvocation,
+        SimpleIdentifier;
 import 'package:analyzer/dart/element/element.dart'
-    show Element, FunctionElement;
+    show Element, ExecutableElement, FunctionElement;
 import 'package:analyzer/dart/element/type.dart'
     show DartType, InterfaceType, ParameterizedType;
 import 'package:analyzer/src/dart/element/type.dart' show DynamicTypeImpl;
@@ -82,6 +89,17 @@ bool isInlineJS(Element e) =>
     e.library.isInSdk &&
     e.library.source.uri.toString() == 'dart:_foreign_helper';
 
+ExecutableElement getFunctionBodyElement(FunctionBody body) {
+  var f = body.parent;
+  if (f is FunctionExpression) {
+    return f.element;
+  } else if (f is MethodDeclaration) {
+    return f.element;
+  } else {
+    return (f as ConstructorDeclaration).element;
+  }
+}
+
 /// Returns the value of the `name` field from the [match]ing annotation on
 /// [element], or `null` if we didn't find a valid match or it was not a string.
 ///

pkg/dev_compiler/lib/src/compiler/element_helpers.dart

@@ -150,7 +150,11 @@ class NodeModuleBuilder extends _ModuleBuilder {
 
 /// Escape [name] to make it into a valid identifier.
 String pathToJSIdentifier(String name) {
-  name = path.basenameWithoutExtension(name);
+  return toJSIdentifier(path.basenameWithoutExtension(name));
+}
+
+/// Escape [name] to make it into a valid identifier.
+String toJSIdentifier(String name) {
   if (name.length == 0) return r'$';
 
   // Escape any invalid characters

pkg/dev_compiler/lib/src/compiler/module_builder.dart

@@ -466,6 +466,10 @@
       'chunked_conversion_utf8_test': skip_timeout,
       'encoding_test': skip_timeout,
 
+      // TODO(jmesserly): this is in an inconsistent state between our old and
+      // newer SDKs.
+      'html_escape_test': skip_fail,
+
       // TODO(rnystrom): If this test is enabled, karma gets confused and
       // disconnects randomly.
       'json_lib_test': skip_fail,

pkg/dev_compiler/test/browser/language_tests.js

@@ -81,13 +81,14 @@ dart_library.library('DeltaBlue', null, /* Imports */[
   dart.setSignature(DeltaBlue$.DeltaBlue, {
     constructors: () => ({DeltaBlue: [DeltaBlue$.DeltaBlue, []]})
   });
+  let const$;
   DeltaBlue$.Strength = class Strength extends core.Object {
     Strength(value, name) {
       this.value = value;
       this.name = name;
     }
     nextWeaker() {
-      return dart.const(dart.list([DeltaBlue$.STRONG_PREFERRED, DeltaBlue$.PREFERRED, DeltaBlue$.STRONG_DEFAULT, DeltaBlue$.NORMAL, DeltaBlue$.WEAK_DEFAULT, DeltaBlue$.WEAKEST], DeltaBlue$.Strength))[dartx.get](this.value);
+      return (const$ || (const$ = dart.const(dart.list([DeltaBlue$.STRONG_PREFERRED, DeltaBlue$.PREFERRED, DeltaBlue$.STRONG_DEFAULT, DeltaBlue$.NORMAL, DeltaBlue$.WEAK_DEFAULT, DeltaBlue$.WEAKEST], DeltaBlue$.Strength))))[dartx.get](this.value);
     }
     static stronger(s1, s2) {
       return dart.notNull(s1.value) < dart.notNull(s2.value);