Compute ClassFunctionType from signature function in Dart 2

- and reinsert assertion.

Change-Id: I2edd119dc53c46e764c07d095213183a186d9a22
Reviewed-on: https://dart-review.googlesource.com/47042
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Emily Fortuna <efortuna@google.com>

pkg/compiler/lib/src/js_backend/runtime_types.dart

@@ -1684,13 +1684,26 @@ class RuntimeTypesImpl extends _RuntimeTypesBase
 ClassFunctionType _computeFunctionType(
     ElementEnvironment _elementEnvironment, ClassEntity cls,
     {bool strongMode}) {
-  if (strongMode && !cls.isClosure) return null;
+  FunctionEntity signatureFunction;
+  if (cls.isClosure) {
+    // Use signature function if available.
+    signatureFunction =
+        _elementEnvironment.lookupLocalClassMember(cls, Identifiers.signature);
+    if (signatureFunction == null && strongMode) {
+      // In Dart 2, a closure only needs its function type if it has a
+      // signature function.
+      return null;
+    }
+  } else if (strongMode) {
+    // Only closures have function type in Dart 2.
+    return null;
+  }
   MemberEntity call =
       _elementEnvironment.lookupLocalClassMember(cls, Identifiers.call);
   if (call != null && call.isFunction) {
     FunctionEntity callFunction = call;
     FunctionType callType = _elementEnvironment.getFunctionType(callFunction);
-    return new ClassFunctionType(callFunction, callType);
+    return new ClassFunctionType(callFunction, callType, signatureFunction);
   }
   return null;
 }
@@ -2340,12 +2353,17 @@ class ClassChecks {
 /// a class.
 class ClassFunctionType {
   /// The `call` function that defines the function type.
-  final MemberEntity callFunction;
+  final FunctionEntity callFunction;
 
   /// The type of the `call` function.
   final FunctionType callType;
 
-  ClassFunctionType(this.callFunction, this.callType);
+  /// The signature function for the function type.
+  ///
+  /// This is used for Dart 2.
+  final FunctionEntity signatureFunction;
+
+  ClassFunctionType(this.callFunction, this.callType, this.signatureFunction);
 }
 
 /// Runtime type usage for a class.

pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart

@@ -678,7 +678,6 @@ class ProgramBuilder {
         _task.emitter, _commonElements, _namer, _worldBuilder, _closedWorld,
         enableMinification: _options.enableMinification);
     RuntimeTypeGenerator runtimeTypeGenerator = new RuntimeTypeGenerator(
-        _elementEnvironment,
         _commonElements,
         _closureDataLookup,
         _outputUnitData,
@@ -688,8 +687,7 @@ class ProgramBuilder {
         _rtiEncoder,
         _jsInteropAnalysis,
         _options.useKernel,
-        _options.strongMode,
-        _options.disableRtiOptimization);
+        _options.strongMode);
 
     void visitMember(MemberEntity member) {
       if (member.isInstanceMember && !member.isAbstract && !member.isField) {

pkg/compiler/lib/src/js_emitter/runtime_type_generator.dart

@@ -11,8 +11,7 @@ import '../closure.dart'
         ClosureConversionTask,
         ScopeInfo;
 import '../common.dart';
-import '../common/names.dart' show Identifiers;
-import '../common_elements.dart' show CommonElements, ElementEnvironment;
+import '../common_elements.dart' show CommonElements;
 import '../deferred_load.dart' show OutputUnit, OutputUnitData;
 import '../elements/elements.dart' show ClassElement, MethodElement;
 import '../elements/entities.dart';
@@ -24,6 +23,7 @@ import '../js_backend/namer.dart' show Namer;
 import '../js_backend/runtime_types.dart'
     show
         ClassChecks,
+        ClassFunctionType,
         RuntimeTypesChecks,
         RuntimeTypesEncoder,
         Substitution,
@@ -35,8 +35,7 @@ import '../util/util.dart' show Setlet;
 import 'code_emitter_task.dart' show CodeEmitterTask;
 
 // Function signatures used in the generation of runtime type information.
-typedef void FunctionTypeSignatureEmitter(
-    FunctionEntity method, FunctionType methodType);
+typedef void FunctionTypeSignatureEmitter(ClassFunctionType classFunctionType);
 
 class TypeTest {
   final jsAst.Name name;
@@ -99,7 +98,6 @@ class TypeTestProperties {
 }
 
 class RuntimeTypeGenerator {
-  final ElementEnvironment _elementEnvironment;
   final CommonElements _commonElements;
   final ClosureConversionTask _closureDataLookup;
   final OutputUnitData _outputUnitData;
@@ -109,15 +107,9 @@ class RuntimeTypeGenerator {
   final RuntimeTypesEncoder _rtiEncoder;
   final JsInteropAnalysis _jsInteropAnalysis;
   final bool _useKernel;
-
-  /// ignore: UNUSED_FIELD
   final bool _strongMode;
 
-  /// ignore: UNUSED_FIELD
-  final bool _disableRtiOptimization;
-
   RuntimeTypeGenerator(
-      this._elementEnvironment,
       this._commonElements,
       this._closureDataLookup,
       this._outputUnitData,
@@ -127,8 +119,15 @@ class RuntimeTypeGenerator {
       this._rtiEncoder,
       this._jsInteropAnalysis,
       this._useKernel,
-      this._strongMode,
-      this._disableRtiOptimization);
+      this._strongMode);
+
+  /**
+   * Generate "is tests" for [cls] itself, and the "is tests" for the
+   * classes it implements and type argument substitution functions for these
+   * tests.   We don't need to add the "is tests" of the super class because
+   * they will be inherited at runtime, but we may need to generate the
+   * substitutions, because they may have changed.
+   */
 
   /// Generates all properties necessary for is-checks on the [classElement].
   ///
@@ -149,49 +148,54 @@ class RuntimeTypeGenerator {
         failedAt(classElement));
 
     // TODO(johnniwinther): Include function signatures in [ClassChecks].
-    void generateFunctionTypeSignature(
-        FunctionEntity method, FunctionType type) {
+    void generateFunctionTypeSignature(ClassFunctionType classFunctionType) {
+      FunctionEntity method = classFunctionType.callFunction;
+      FunctionType type = classFunctionType.callType;
       assert(!(method is MethodElement && !method.isImplementation));
-      jsAst.Expression thisAccess = new jsAst.This();
-      if (method.enclosingClass.isClosure) {
-        ScopeInfo scopeInfo = _closureDataLookup.getScopeInfo(method);
-        if (scopeInfo is ClosureRepresentationInfo) {
-          FieldEntity thisLocal = scopeInfo.thisFieldEntity;
-          if (thisLocal != null) {
-            assert(
-                thisLocal is ClosureFieldElement || thisLocal is JClosureField);
-            jsAst.Name thisName = _namer.instanceFieldPropertyName(thisLocal);
-            thisAccess = js('this.#', thisName);
-          }
-        }
-      }
 
       // TODO(johnniwinther): Avoid unneeded function type indices or
       // signatures. We either need them for mirrors or because [type] is
       // potentially a subtype of a checked function. Currently we eagerly
       // generate a function type index or signature for all callable classes.
       if (storeFunctionTypeInMetadata && !type.containsTypeVariables) {
+        // TODO(johnniwinther,efortuna): Should we use the scheme for Dart 2?
         // TODO(sigmund): use output unit of `method` (Issue #31032)
         OutputUnit outputUnit = _outputUnitData.mainOutputUnit;
         result.functionTypeIndex =
             emitterTask.metadataCollector.reifyType(type, outputUnit);
       } else {
-        jsAst.Expression encoding;
-        MemberEntity signature = _elementEnvironment.lookupLocalClassMember(
-            method.enclosingClass, Identifiers.signature);
-        if (_useKernel &&
-            signature != null &&
-            generatedCode[signature] != null) {
-          // Use precomputed signature function.
-          encoding = generatedCode[signature];
+        jsAst.Expression encoding =
+            generatedCode[classFunctionType.signatureFunction];
+        if (classFunctionType.signatureFunction != null) {
+          // Use precomputed signature function if live.
         } else {
-          // TODO(efortuna): Reinsert assertion.
-          // Generate the signature on the fly.
+          assert(!_useKernel || !_strongMode);
+          // Generate the signature on the fly. This is only supported for
+          // Dart 1.
+
+          jsAst.Expression thisAccess = new jsAst.This();
+          if (method.enclosingClass.isClosure) {
+            ScopeInfo scopeInfo = _closureDataLookup.getScopeInfo(method);
+            if (scopeInfo is ClosureRepresentationInfo) {
+              FieldEntity thisLocal = scopeInfo.thisFieldEntity;
+              if (thisLocal != null) {
+                assert(thisLocal is ClosureFieldElement ||
+                    thisLocal is JClosureField);
+                jsAst.Name thisName =
+                    _namer.instanceFieldPropertyName(thisLocal);
+                thisAccess = js('this.#', thisName);
+              }
+            }
+          }
+
           encoding = _rtiEncoder.getSignatureEncoding(
               emitterTask.emitter, type, thisAccess);
         }
-        jsAst.Name operatorSignature = _namer.asName(_namer.operatorSignature);
-        result.addSignature(classElement, operatorSignature, encoding);
+        if (encoding != null) {
+          jsAst.Name operatorSignature =
+              _namer.asName(_namer.operatorSignature);
+          result.addSignature(classElement, operatorSignature, encoding);
+        }
       }
     }
 
@@ -226,13 +230,6 @@ class RuntimeTypeGenerator {
     return result;
   }
 
-  /**
-   * Generate "is tests" for [cls] itself, and the "is tests" for the
-   * classes it implements and type argument substitution functions for these
-   * tests.   We don't need to add the "is tests" of the super class because
-   * they will be inherited at runtime, but we may need to generate the
-   * substitutions, because they may have changed.
-   */
   void _generateIsTestsOn(
       ClassEntity cls,
       FunctionTypeSignatureEmitter generateFunctionTypeSignature,
@@ -252,8 +249,7 @@ class RuntimeTypeGenerator {
     }
 
     if (classChecks.functionType != null) {
-      generateFunctionTypeSignature(classChecks.functionType.callFunction,
-          classChecks.functionType.callType);
+      generateFunctionTypeSignature(classChecks.functionType);
     }
   }
 }

tests/compiler/dart2js/closure/show.dart

@@ -0,0 +1,22 @@
+// Copyright (c) 2018, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+/// Helper program that shows the closure data on a dart program.
+
+import 'package:args/args.dart';
+import '../equivalence/id_equivalence_helper.dart';
+import '../equivalence/show_helper.dart';
+import 'closure_test.dart';
+
+main(List<String> args) async {
+  ArgParser argParser = createArgParser();
+  argParser.addFlag('inference', defaultsTo: true);
+  argParser.addFlag('side-effects', defaultsTo: false);
+  argParser.addFlag('callers', defaultsTo: false);
+  ArgResults results = argParser.parse(args);
+
+  ComputeMemberDataFunction astFunction = computeClosureData;
+  ComputeMemberDataFunction kernelFunction = computeKernelClosureData;
+  await show(results, astFunction, kernelFunction);
+}

tests/corelib_2/corelib_2.status

@@ -216,10 +216,8 @@ map_entry_test: RuntimeError
 
 [ $compiler == dart2js && $fasta && $host_checked && $strong ]
 apply3_test: RuntimeError
-apply_test: Crash # 'file:*/pkg/compiler/lib/src/js_emitter/runtime_type_generator.dart': Failed assertion: line 208 pos 18: '!(_useKernel && _strongMode && !_disableRtiOptimization) ||
 bigint_test: RuntimeError
 cast_test: RuntimeError
-date_time11_test: Crash # 'file:*/pkg/compiler/lib/src/js_emitter/runtime_type_generator.dart': Failed assertion: line 208 pos 18: '!(_useKernel && _strongMode && !_disableRtiOptimization) ||
 dynamic_nosuchmethod_test: RuntimeError
 error_stack_trace1_test: RuntimeError # Issue 12399
 growable_list_test: RuntimeError # Concurrent modifications test always runs
@@ -235,7 +233,6 @@ iterable_reduce_test: Crash # NoSuchMethodError: The getter 'isDynamic' was call
 iterable_return_type_test/01: RuntimeError # Issue 20085
 iterable_return_type_test/02: RuntimeError # Dart2js does not support Uint64*.
 iterable_to_list_test/01: RuntimeError # Issue 26501
-iterable_where_type_test: Crash # 'file:*/pkg/compiler/lib/src/js_emitter/runtime_type_generator.dart': Failed assertion: line 208 pos 18: '!(_useKernel && _strongMode && !_disableRtiOptimization) ||
 list_concurrent_modify_test: RuntimeError # dart2js does not fully implement these
 list_test/01: RuntimeError
 list_test/none: RuntimeError

tests/language_2/language_2_dart2js.status

@@ -1418,7 +1418,6 @@ generic_methods_unused_parameter_test: Crash # Assertion failure: kind=special,m
 generic_no_such_method_dispatcher_simple_test: CompileTimeError
 generic_no_such_method_dispatcher_test: CompileTimeError
 generic_tearoff_test: CompileTimeError
-generic_test: RuntimeError
 getter_override2_test/02: MissingCompileTimeError
 getter_override_test/00: MissingCompileTimeError
 getter_override_test/01: MissingCompileTimeError