dart2js: Capture typedef arguments once

The type Map<T, Foo<Set<T>>> contains one type variable referenced twice,
so there are two inputs into the HTypeInfoExpression instruction.

If Foo is a typedef, T can be reused, e.g.

    typedef E Foo<E>(E a, E b);

As the typedef is expanded (to Function(Set<T>, Set<T>) => Set<T>) it
should not consume additional types from the to-level input.  We
prevent this by capturing the types and using the captured type
expressions inside the typedef expansion.

TODO: We should make the type subexpression Foo<...> be a second
HTypeInfoExpression, with Set<T> as its input (a third
HTypeInfoExpression). This would share all the Set<T> subexpressions
instead of duplicating them. This would require HTypeInfoExpression
inputs to correspond to type variables AND typedefs.

BUG= https://github.com/dart-lang/sdk/issues/28749
R=efortuna@google.com

Review-Url: https://codereview.chromium.org/2812393003 .

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

@@ -878,6 +878,8 @@ class TypeRepresentationGenerator implements DartTypeVisitor {
   OnVariableCallback onVariable;
   ShouldEncodeTypedefCallback shouldEncodeTypedef;
 
+  Map<ResolutionTypeVariableType, jsAst.Expression> typedefBindings;
+
   TypeRepresentationGenerator(this.namer, this.emitter);
 
   /**
@@ -888,6 +890,7 @@ class TypeRepresentationGenerator implements DartTypeVisitor {
       ResolutionDartType type,
       OnVariableCallback onVariable,
       ShouldEncodeTypedefCallback encodeTypedef) {
+    assert(typedefBindings == null);
     this.onVariable = onVariable;
     this.shouldEncodeTypedef = (encodeTypedef != null)
         ? encodeTypedef
@@ -906,6 +909,10 @@ class TypeRepresentationGenerator implements DartTypeVisitor {
   visit(ResolutionDartType type, [_]) => type.accept(this, null);
 
   visitTypeVariableType(ResolutionTypeVariableType type, _) {
+    if (typedefBindings != null) {
+      assert(typedefBindings[type] != null);
+      return typedefBindings[type];
+    }
     return onVariable(type);
   }
 
@@ -1001,6 +1008,42 @@ class TypeRepresentationGenerator implements DartTypeVisitor {
   visitTypedefType(ResolutionTypedefType type, _) {
     bool shouldEncode = shouldEncodeTypedef(type);
     ResolutionDartType unaliasedType = type.unaliased;
+
+    var oldBindings = typedefBindings;
+    if (typedefBindings == null) {
+      // First level typedef - capture arguments for re-use within typedef body.
+      //
+      // The type `Map<T, Foo<Set<T>>>` contains one type variable referenced
+      // twice, so there are two inputs into the HTypeInfoExpression
+      // instruction.
+      //
+      // If Foo is a typedef, T can be reused, e.g.
+      //
+      //     typedef E Foo<E>(E a, E b);
+      //
+      // As the typedef is expanded (to (Set<T>, Set<T>) => Set<T>) it should
+      // not consume additional types from the to-level input.  We prevent this
+      // by capturing the types and using the captured type expressions inside
+      // the typedef expansion.
+      //
+      // TODO(sra): We should make the type subexpression Foo<...> be a second
+      // HTypeInfoExpression, with Set<T> as its input (a third
+      // HTypeInfoExpression). This would share all the Set<T> subexpressions
+      // instead of duplicating them. This would require HTypeInfoExpression
+      // inputs to correspond to type variables AND typedefs.
+      typedefBindings = <ResolutionTypeVariableType, jsAst.Expression>{};
+      type.forEachTypeVariable((variable) {
+        if (variable is! MethodTypeVariableType) {
+          typedefBindings[variable] = onVariable(variable);
+        }
+      });
+    }
+
+    jsAst.Expression finish(jsAst.Expression result) {
+      typedefBindings = oldBindings;
+      return result;
+    }
+
     if (shouldEncode) {
       jsAst.ObjectInitializer initializer = unaliasedType.accept(this, null);
       // We have to encode the aliased type.
@@ -1011,9 +1054,9 @@ class TypeRepresentationGenerator implements DartTypeVisitor {
       // Add it to the function-type object.
       jsAst.LiteralString tag = js.string(namer.typedefTag);
       initializer.properties.add(new jsAst.Property(tag, encodedTypedef));
-      return initializer;
+      return finish(initializer);
     } else {
-      return unaliasedType.accept(this, null);
+      return finish(unaliasedType.accept(this, null));
     }
   }
 }

pkg/js_ast/lib/src/nodes.dart

@@ -233,6 +233,8 @@ abstract class Node {
   Statement toStatement() {
     throw new UnsupportedError('toStatement');
   }
+
+  String debugPrint() => DebugPrint(this);
 }
 
 class Program extends Node {

pkg/js_ast/lib/src/printer.dart

@@ -61,6 +61,15 @@ class SimpleJavaScriptPrintingContext extends JavaScriptPrintingContext {
   String getText() => buffer.toString();
 }
 
+String DebugPrint(Node node) {
+  JavaScriptPrintingOptions options = new JavaScriptPrintingOptions();
+  SimpleJavaScriptPrintingContext context =
+      new SimpleJavaScriptPrintingContext();
+  Printer printer = new Printer(options, context);
+  printer.visit(node);
+  return context.getText();
+}
+
 class Printer implements NodeVisitor {
   final JavaScriptPrintingOptions options;
   final JavaScriptPrintingContext context;

tests/compiler/dart2js_extra/28749_test.dart

@@ -0,0 +1,57 @@
+// Copyright (c) 2017, 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.
+
+// Regression test for http://dartbug.com/28749.
+//
+// This would crash at compile time because inner typedefs remain after calling
+// [type.unalias].  Expanding the typedef causes the inputs to be used multiple
+// types, breaking the invariant of HTypeInfoExpression that the type variable
+// occurrences correspond to inputs.
+
+import 'package:expect/expect.dart';
+
+typedef void F<T>(T value);
+typedef F<U> Converter<U>(F<U> function);
+typedef Converter<V> ConvertFactory<V>(int input);
+
+class B<W> {
+  final field = new Wrap<ConvertFactory<W>>();
+  @NoInline()
+  B();
+}
+
+class Wrap<X> {
+  @NoInline()
+  Wrap();
+}
+
+foo<Y>(int x) {
+  if (x == 0)
+    return new Wrap<ConvertFactory<Y>>().runtimeType;
+  else
+    return new B<Y>().field.runtimeType;
+}
+
+void main() {
+  var name = '${Wrap}';
+  if (name.length < 4) return; // minified.
+
+  Expect.equals(
+    'Wrap<(int) => ((int) => void) => (int) => void>',
+    '${new B<int>().field.runtimeType}',
+  );
+  Expect.equals(
+    'Wrap<(int) => ((bool) => void) => (bool) => void>',
+    '${new B<bool>().field.runtimeType}',
+  );
+
+  Expect.equals(
+    'Wrap<(int) => ((dynamic) => void) => (dynamic) => void>',
+    '${foo<int>(0)}',
+  );
+  Expect.equals(
+    'Wrap<(int) => ((dynamic) => void) => (dynamic) => void>',
+    '${foo<String>(1)}',
+  );
+}