Restrict backend misuse

johnniwinther · johnniwinther · commit ac122ffe921a · 2015-11-25T15:49:32.000+01:00
BUG= R=sigurdm@google.com Review URL: https://codereview.chromium.org/1469353004.
diff --git a/pkg/compiler/lib/src/compiler.dart b/pkg/compiler/lib/src/compiler.dart
@@ -325,6 +325,7 @@ abstract class Compiler {
   ClassElement symbolImplementationClass;
 
   // Initialized when symbolImplementationClass has been resolved.
+  // TODO(johnniwinther): Move this to [BackendHelpers].
   FunctionElement symbolValidatedConstructor;
 
   // Initialized when mirrorsUsedClass has been resolved.
diff --git a/pkg/compiler/lib/src/js_backend/backend.dart b/pkg/compiler/lib/src/js_backend/backend.dart
@@ -581,6 +581,52 @@ class JavaScriptBackend extends Backend {
         new Namer(compiler);
   }
 
+  /// The backend must *always* call this method when enqueuing an
+  /// element. Calls done by the backend are not seen by global
+  /// optimizations, so they would make these optimizations unsound.
+  /// Therefore we need to collect the list of helpers the backend may
+  /// use.
+  // TODO(johnniwinther): Replace this with a more precise modelling; type
+  // inference of these elements is disabled.
+  Element registerBackendUse(Element element) {
+    if (element != null) {
+      bool registerUse = false;
+      if (element == helpers.streamIteratorConstructor ||
+          element == helpers.compiler.symbolConstructor ||
+          element == helpers.compiler.symbolValidatedConstructor ||
+          element == helpers.syncCompleterConstructor ||
+          element == coreClasses.symbolClass ||
+          element == helpers.objectNoSuchMethod) {
+        // TODO(johnniwinther): These are valid but we could be more precise.
+        registerUse = true;
+      } else if (element.implementationLibrary.isPatch ||
+                 element.library == helpers.jsHelperLibrary ||
+                 element.library == helpers.interceptorsLibrary ||
+                 element.library == helpers.isolateHelperLibrary) {
+        // TODO(johnniwinther): We should be more precise about these.
+        registerUse = true;
+      } else if (element == coreClasses.listClass ||
+                 element == helpers.mapLiteralClass ||
+                 element == coreClasses.functionClass ||
+                 element == coreClasses.stringClass) {
+        // TODO(johnniwinther): Avoid these.
+        registerUse = true;
+      }
+      if (!registerUse) {
+        assert(invariant(element, false,
+            message: "Backend use of $element is not allowed."));
+        return element;
+      }
+      helpersUsed.add(element.declaration);
+      if (element.isClass && element.isPatched) {
+        // Both declaration and implementation may declare fields, so we
+        // add both to the list of helpers.
+        helpersUsed.add(element.implementation);
+      }
+    }
+    return element;
+  }
+
   bool usedByBackend(Element element) {
     if (element.isParameter
         || element.isInitializingFormal
@@ -1479,23 +1525,6 @@ class JavaScriptBackend extends Backend {
            compiler.enabledRuntimeType;
   }
 
-  /// The backend must *always* call this method when enqueuing an
-  /// element. Calls done by the backend are not seen by global
-  /// optimizations, so they would make these optimizations unsound.
-  /// Therefore we need to collect the list of helpers the backend may
-  /// use.
-  Element registerBackendUse(Element element) {
-    if (element != null) {
-      helpersUsed.add(element.declaration);
-      if (element.isClass && element.isPatched) {
-        // Both declaration and implementation may declare fields, so we
-        // add both to the list of helpers.
-        helpersUsed.add(element.implementation);
-      }
-    }
-    return element;
-  }
-
   /// Enqueue [e] in [enqueuer].
   ///
   /// This method calls [registerBackendUse].
diff --git a/pkg/compiler/lib/src/js_backend/backend_helpers.dart b/pkg/compiler/lib/src/js_backend/backend_helpers.dart
@@ -6,6 +6,7 @@ library dart2js.js_backend.helpers;
 
 import '../common.dart';
 import '../common/names.dart' show
+    Identifiers,
     Uris;
 import '../common/resolution.dart' show
     Resolution;
@@ -679,4 +680,18 @@ class BackendHelpers {
   Element get convertRtiToRuntimeType {
     return findHelper('convertRtiToRuntimeType');
   }
+
+  ClassElement get stackTraceClass {
+    return findHelper('_StackTrace');
+  }
+
+  MethodElement _objectNoSuchMethod;
+
+  MethodElement get objectNoSuchMethod {
+    if (_objectNoSuchMethod == null) {
+      _objectNoSuchMethod =
+          coreClasses.objectClass.lookupLocalMember(Identifiers.noSuchMethod_);
+    }
+    return _objectNoSuchMethod;
+  }
 }
diff --git a/pkg/compiler/lib/src/js_backend/backend_impact.dart b/pkg/compiler/lib/src/js_backend/backend_impact.dart
@@ -68,8 +68,8 @@ class BackendImpacts {
             helpers.getRuntimeTypeInfo,
             helpers.computeSignature,
             helpers.getRuntimeTypeArguments],
-          instantiatedClasses: [
-            coreClasses.listClass]);
+          otherImpacts: [
+            listValues]);
     }
     return _computeSignature;
   }
@@ -187,16 +187,84 @@ class BackendImpacts {
     return _throwNoSuchMethod;
   }
 
+  BackendImpact _stringValues;
+
+  BackendImpact get stringValues {
+    if (_stringValues == null) {
+      _stringValues = new BackendImpact(
+          instantiatedClasses: [
+            helpers.jsStringClass]);
+    }
+    return _stringValues;
+  }
+
+  BackendImpact _numValues;
+
+  BackendImpact get numValues {
+    if (_numValues == null) {
+      _numValues = new BackendImpact(
+          instantiatedClasses: [
+            helpers.jsIntClass,
+            helpers.jsPositiveIntClass,
+            helpers.jsUInt32Class,
+            helpers.jsUInt31Class,
+            helpers.jsNumberClass,
+            helpers.jsDoubleClass]);
+    }
+    return _numValues;
+  }
+
+  BackendImpact get intValues => numValues;
+
+  BackendImpact get doubleValues => numValues;
+
+  BackendImpact _boolValues;
+
+  BackendImpact get boolValues {
+    if (_boolValues == null) {
+      _boolValues = new BackendImpact(
+          instantiatedClasses: [
+            helpers.jsBoolClass]);
+    }
+    return _boolValues;
+  }
+
+  BackendImpact _nullValue;
+
+  BackendImpact get nullValue {
+    if (_nullValue == null) {
+      _nullValue = new BackendImpact(
+          instantiatedClasses: [
+            helpers.jsNullClass]);
+    }
+    return _nullValue;
+  }
+
+  BackendImpact _listValues;
+
+  BackendImpact get listValues {
+    if (_listValues == null) {
+      _listValues = new BackendImpact(
+          instantiatedClasses: [
+            helpers.jsArrayClass,
+            helpers.jsMutableArrayClass,
+            helpers.jsFixedArrayClass,
+            helpers.jsExtendableArrayClass,
+            helpers.jsUnmodifiableArrayClass]);
+    }
+    return _listValues;
+  }
+
   BackendImpact _throwRuntimeError;
 
   BackendImpact get throwRuntimeError {
     if (_throwRuntimeError == null) {
       _throwRuntimeError = new BackendImpact(
           staticUses: [
             helpers.throwRuntimeError],
-          // Also register the types of the arguments passed to this method.
-          instantiatedClasses: [
-            coreClasses.stringClass]);
+          otherImpacts: [
+            // Also register the types of the arguments passed to this method.
+            stringValues]);
     }
     return _throwRuntimeError;
   }
@@ -208,8 +276,7 @@ class BackendImpacts {
       _superNoSuchMethod = new BackendImpact(
           staticUses: [
             helpers.createInvocationMirror,
-            coreClasses.objectClass.lookupLocalMember(
-                Identifiers.noSuchMethod_)],
+            helpers.objectNoSuchMethod],
           otherImpacts: [
             _needsInt(
                 'Needed to encode the invocation kind of super.noSuchMethod.'),
@@ -277,23 +344,19 @@ class BackendImpacts {
   /// Helper for registering that `int` is needed.
   BackendImpact _needsInt(String reason) {
     // TODO(johnniwinther): Register [reason] for use in dump-info.
-    return new BackendImpact(
-        instantiatedClasses: [coreClasses.intClass]);
+    return intValues;
   }
 
   /// Helper for registering that `List` is needed.
   BackendImpact _needsList(String reason) {
     // TODO(johnniwinther): Register [reason] for use in dump-info.
-    return new BackendImpact(
-        instantiatedClasses: [coreClasses.listClass]);
+    return listValues;
   }
 
   /// Helper for registering that `String` is needed.
   BackendImpact _needsString(String reason) {
     // TODO(johnniwinther): Register [reason] for use in dump-info.
-    return new BackendImpact(
-        instantiatedClasses: [
-          coreClasses.stringClass]);
+    return stringValues;
   }
 
   BackendImpact _assertWithoutMessage;
@@ -352,62 +415,15 @@ class BackendImpacts {
     return _stringJuxtaposition;
   }
 
-  // TODO(johnniwinther): Point to to the JavaScript classes instead of the Dart
-  // classes in these impacts.
-  BackendImpact _nullLiteral;
+  BackendImpact get nullLiteral => nullValue;
 
-  BackendImpact get nullLiteral {
-    if (_nullLiteral == null) {
-      _nullLiteral = new BackendImpact(
-          instantiatedClasses: [
-            coreClasses.nullClass]);
-    }
-    return _nullLiteral;
-  }
+  BackendImpact get boolLiteral => boolValues;
 
-  BackendImpact _boolLiteral;
+  BackendImpact get intLiteral => intValues;
 
-  BackendImpact get boolLiteral {
-    if (_boolLiteral == null) {
-      _boolLiteral = new BackendImpact(
-          instantiatedClasses: [
-            coreClasses.boolClass]);
-    }
-    return _boolLiteral;
-  }
+  BackendImpact get doubleLiteral => doubleValues;
 
-  BackendImpact _intLiteral;
-
-  BackendImpact get intLiteral {
-    if (_intLiteral == null) {
-      _intLiteral = new BackendImpact(
-          instantiatedClasses: [
-            coreClasses.intClass]);
-    }
-    return _intLiteral;
-  }
-
-  BackendImpact _doubleLiteral;
-
-  BackendImpact get doubleLiteral {
-    if (_doubleLiteral == null) {
-      _doubleLiteral = new BackendImpact(
-          instantiatedClasses: [
-            coreClasses.doubleClass]);
-    }
-    return _doubleLiteral;
-  }
-
-  BackendImpact _stringLiteral;
-
-  BackendImpact get stringLiteral {
-    if (_stringLiteral == null) {
-      _stringLiteral = new BackendImpact(
-          instantiatedClasses: [
-            coreClasses.stringClass]);
-    }
-    return _stringLiteral;
-  }
+  BackendImpact get stringLiteral => stringValues;
 
   BackendImpact _catchStatement;
 
@@ -468,7 +484,7 @@ class BackendImpacts {
     if (_stackTraceInCatch == null) {
       _stackTraceInCatch = new BackendImpact(
           instantiatedClasses: [
-            coreClasses.stackTraceClass],
+            helpers.stackTraceClass],
           staticUses: [
             helpers.traceFromException]);
     }
@@ -498,9 +514,8 @@ class BackendImpacts {
             helpers.getRuntimeTypeInfo,
             helpers.runtimeTypeToString,
             helpers.createRuntimeType],
-          instantiatedClasses: [
-            coreClasses.listClass],
           otherImpacts: [
+            listValues,
             getRuntimeTypeArgument,
             _needsInt('Needed for accessing a type variable literal on this.')
           ]);
@@ -513,8 +528,8 @@ class BackendImpacts {
   BackendImpact get typeCheck {
     if (_typeCheck == null) {
       _typeCheck = new BackendImpact(
-          instantiatedClasses: [
-            coreClasses.boolClass]);
+          otherImpacts: [
+            boolValues]);
     }
     return _typeCheck;
   }
@@ -551,9 +566,8 @@ class BackendImpacts {
             // TODO(johnniwinther): Investigate why this is needed.
             helpers.setRuntimeTypeInfo,
             helpers.getRuntimeTypeInfo],
-          instantiatedClasses: [
-            coreClasses.listClass],
           otherImpacts: [
+            listValues,
             getRuntimeTypeArgument]);
     }
     return _genericTypeCheck;
@@ -564,8 +578,8 @@ class BackendImpacts {
   BackendImpact get genericIsCheck {
     if (_genericIsCheck == null) {
       _genericIsCheck = new BackendImpact(
-          instantiatedClasses: [
-            coreClasses.listClass]);
+          otherImpacts: [
+            intValues]);
     }
     return _genericIsCheck;
   }
diff --git a/tests/compiler/dart2js/mock_libraries.dart b/tests/compiler/dart2js/mock_libraries.dart
@@ -64,10 +64,6 @@ const Map<String, String> DEFAULT_CORE_LIBRARY = const <String, String>{
       }''',
   'LinkedHashMap': r'''
       class LinkedHashMap<K, V> implements Map<K, V> {
-        factory LinkedHashMap._empty() => null;
-        factory LinkedHashMap._literal(elements) => null;
-        static _makeEmpty() => null;
-        static _makeLiteral(elements) => null;
       }''',
   'List': r'''
       class List<E> extends Iterable<E> {
@@ -113,6 +109,14 @@ import 'dart:_js_helper';
 import 'dart:_interceptors';
 import 'dart:_isolate_helper';
 import 'dart:async';
+
+@patch
+class LinkedHashMap<K, V> {
+  factory LinkedHashMap._empty() => null;
+  factory LinkedHashMap._literal(elements) => null;
+  static _makeEmpty() => null;
+  static _makeLiteral(elements) => null;
+}
 ''';
 
 const Map<String, String> DEFAULT_JS_HELPER_LIBRARY = const <String, String>{
diff --git a/tests/compiler/dart2js/simple_inferrer_test.dart b/tests/compiler/dart2js/simple_inferrer_test.dart
diff --git a/tests/compiler/dart2js/size_test.dart b/tests/compiler/dart2js/size_test.dart
