[frontend] When serializing compiled expression proc, clone type params first.

If there are circular references between type parameters and types in the bounds,
currently serialization leaves original type parameters unchanged, which results
in failures to serialize compiled expression procedure.

This CL makes sure to clone all type parameters first, then passes map
with old-to-new type parameters to CloneVisitor.

Bug: https://github.com/dart-lang/sdk/issues/34052
Change-Id: Idf3e6e6e9099f93cdd7e970ab3b21921cdb29178
Reviewed-on: https://dart-review.googlesource.com/75241
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
diff --git a/pkg/front_end/lib/src/fasta/kernel/utils.dart b/pkg/front_end/lib/src/fasta/kernel/utils.dart
index 239709c..30686ba 100644
--- a/pkg/front_end/lib/src/fasta/kernel/utils.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/utils.dart
@@ -9,7 +9,15 @@
 import 'package:kernel/clone.dart' show CloneVisitor;
 
 import 'package:kernel/ast.dart'
-    show Library, Component, Procedure, Class, TypeParameter, Supertype;
+    show
+        DartType,
+        Library,
+        Component,
+        Procedure,
+        Class,
+        TypeParameter,
+        TypeParameterType,
+        Supertype;
 
 import 'package:kernel/binary/ast_to_binary.dart' show BinaryPrinter;
 
@@ -70,9 +78,18 @@
   if (procedure.parent is Class) {
     Class realClass = procedure.parent;
 
-    CloneVisitor cloner = new CloneVisitor();
-
     Class fakeClass = new Class(name: kDebugClassName);
+    Map<TypeParameter, TypeParameter> typeParams =
+        <TypeParameter, TypeParameter>{};
+    Map<TypeParameter, DartType> typeSubstitution = <TypeParameter, DartType>{};
+    for (TypeParameter typeParam in realClass.typeParameters) {
+      var newNode = new TypeParameter(typeParam.name);
+      typeParams[typeParam] = newNode;
+      typeSubstitution[typeParam] = new TypeParameterType(newNode);
+    }
+    CloneVisitor cloner = new CloneVisitor(
+        typeSubstitution: typeSubstitution, typeParams: typeParams);
+
     for (TypeParameter typeParam in realClass.typeParameters) {
       fakeClass.typeParameters.add(typeParam.accept(cloner));
     }
diff --git a/pkg/front_end/test/fasta/expression_test.dart b/pkg/front_end/test/fasta/expression_test.dart
index 9e6e402..2eeba80 100644
--- a/pkg/front_end/test/fasta/expression_test.dart
+++ b/pkg/front_end/test/fasta/expression_test.dart
@@ -46,7 +46,8 @@
 
 import '../../lib/src/fasta/testing/kernel_chain.dart' show runDiff, openWrite;
 
-import '../../lib/src/fasta/kernel/utils.dart' show writeComponentToFile;
+import '../../lib/src/fasta/kernel/utils.dart'
+    show writeComponentToFile, serializeProcedure;
 
 const JsonEncoder json = const JsonEncoder.withIndent("  ");
 
@@ -284,6 +285,37 @@
 
   String get name => "compile expression";
 
+  // Compile [test.expression], update [test.errors] with results.
+  // As a side effect - verify that generated procedure can be serialized.
+  void compileExpression(TestCase test, IncrementalCompiler compiler,
+      Component component, Context context) async {
+    Map<String, DartType> definitions = {};
+    for (String name in test.definitions) {
+      definitions[name] = new DynamicType();
+    }
+    List<TypeParameter> typeParams = [];
+    for (String name in test.typeDefinitions) {
+      typeParams.add(new TypeParameter(name, new DynamicType()));
+    }
+
+    Procedure compiledProcedure = await compiler.compileExpression(
+        test.expression,
+        definitions,
+        typeParams,
+        "debugExpr",
+        test.library,
+        test.className,
+        test.isStaticMethod);
+    List<CompilationMessage> errors = context.takeErrors();
+    test.results.add(new CompilationResult(compiledProcedure, errors));
+    if (compiledProcedure != null) {
+      // Confirm we can serialize generated procedure.
+      component.computeCanonicalNames();
+      List<int> list = serializeProcedure(compiledProcedure);
+      assert(list.length > 0);
+    }
+  }
+
   Future<Result<List<TestCase>>> run(
       List<TestCase> tests, Context context) async {
     for (var test in tests) {
@@ -305,39 +337,19 @@
         context.fileSystem.entityForUri(dillFileUri).writeAsBytesSync(
             await new File.fromUri(dillFileUri).readAsBytes());
       }
+      compileExpression(test, sourceCompiler, component, context);
 
       var dillCompiler =
           new IncrementalCompiler(context.compilerContext, dillFileUri);
-      await dillCompiler.computeDelta(entryPoint: test.entryPoint);
+      component = await dillCompiler.computeDelta(entryPoint: test.entryPoint);
+      component.computeCanonicalNames();
       await dillFile.delete();
 
       errors = context.takeErrors();
-
       // Since it compiled successfully from source, the bootstrap-from-Dill
       // should also succeed without errors.
       assert(errors.isEmpty);
-
-      Map<String, DartType> definitions = {};
-      for (String name in test.definitions) {
-        definitions[name] = new DynamicType();
-      }
-      List<TypeParameter> typeParams = [];
-      for (String name in test.typeDefinitions) {
-        typeParams.add(new TypeParameter(name, new DynamicType()));
-      }
-
-      for (var compiler in [sourceCompiler, dillCompiler]) {
-        Procedure compiledProcedure = await compiler.compileExpression(
-            test.expression,
-            definitions,
-            typeParams,
-            "debugExpr",
-            test.library,
-            test.className,
-            test.isStaticMethod);
-        var errors = context.takeErrors();
-        test.results.add(new CompilationResult(compiledProcedure, errors));
-      }
+      compileExpression(test, dillCompiler, component, context);
     }
     return new Result.pass(tests);
   }
diff --git a/pkg/front_end/testcases/expression/class_type_param_circular_reference.expression.yaml b/pkg/front_end/testcases/expression/class_type_param_circular_reference.expression.yaml
new file mode 100644
index 0000000..086d26f
--- /dev/null
+++ b/pkg/front_end/testcases/expression/class_type_param_circular_reference.expression.yaml
@@ -0,0 +1,9 @@
+# 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.
+
+entry_point: "main.dart"
+definitions: []
+position: "main.dart#MiddlewareApi"
+expression: |
+  toString()
diff --git a/pkg/front_end/testcases/expression/class_type_param_circular_reference.expression.yaml.expect b/pkg/front_end/testcases/expression/class_type_param_circular_reference.expression.yaml.expect
new file mode 100644
index 0000000..e380b76
--- /dev/null
+++ b/pkg/front_end/testcases/expression/class_type_param_circular_reference.expression.yaml.expect
@@ -0,0 +1,4 @@
+Errors: {
+}
+method /* from org-dartlang-debug:synthetic_debug_expression */ debugExpr() → dynamic
+  return this.{dart.core::Object::toString}();
\ No newline at end of file
diff --git a/pkg/front_end/testcases/expression/main.dart b/pkg/front_end/testcases/expression/main.dart
index 66153cb..a889b39 100644
--- a/pkg/front_end/testcases/expression/main.dart
+++ b/pkg/front_end/testcases/expression/main.dart
@@ -48,6 +48,13 @@
   }
 }
 
+abstract class Built<V extends Built<V, B>, B extends Builder<V, B>> {}
+
+abstract class Builder<V extends Built<V, B>, B extends Builder<V, B>> {}
+
+class MiddlewareApi<State extends Built<State, StateBuilder>,
+    StateBuilder extends Builder<State, StateBuilder>> {}
+
 main() {
   exit(0);
 }
diff --git a/pkg/front_end/testcases/expression/type_param_bound.expression.yaml.expect b/pkg/front_end/testcases/expression/type_param_bound.expression.yaml.expect
index 285eca0..b18dd26 100644
--- a/pkg/front_end/testcases/expression/type_param_bound.expression.yaml.expect
+++ b/pkg/front_end/testcases/expression/type_param_bound.expression.yaml.expect
@@ -1,4 +1,4 @@
 Errors: {
 }
 method /* from org-dartlang-debug:synthetic_debug_expression */ debugExpr<T extends dynamic>() → dynamic
-  return main::hasBound<main::debugExpr::T>();
+  return main::hasBound<#lib1::debugExpr::T>();
diff --git a/pkg/kernel/lib/clone.dart b/pkg/kernel/lib/clone.dart
index 2fae8ce..137b71a3 100644
--- a/pkg/kernel/lib/clone.dart
+++ b/pkg/kernel/lib/clone.dart
@@ -20,6 +20,7 @@
       <LabeledStatement, LabeledStatement>{};
   final Map<SwitchCase, SwitchCase> switchCases = <SwitchCase, SwitchCase>{};
   final Map<TypeParameter, DartType> typeSubstitution;
+  final Map<TypeParameter, TypeParameter> typeParams;
   bool cloneAnnotations;
 
   /// Creates an instance of the cloning visitor for Kernel ASTs.
@@ -29,8 +30,10 @@
   /// annotations in procedure bodies are cloned unconditionally.
   CloneVisitor(
       {Map<TypeParameter, DartType> typeSubstitution,
+      Map<TypeParameter, TypeParameter> typeParams,
       this.cloneAnnotations = true})
-      : this.typeSubstitution = ensureMutable(typeSubstitution);
+      : this.typeSubstitution = ensureMutable(typeSubstitution),
+        this.typeParams = typeParams ?? <TypeParameter, TypeParameter>{};
 
   static Map<TypeParameter, DartType> ensureMutable(
       Map<TypeParameter, DartType> map) {
@@ -460,8 +463,11 @@
   }
 
   visitTypeParameter(TypeParameter node) {
-    var newNode = new TypeParameter(node.name);
-    typeSubstitution[node] = new TypeParameterType(newNode);
+    TypeParameter newNode = typeParams[node];
+    if (newNode == null) {
+      newNode = new TypeParameter(node.name);
+      typeSubstitution[node] = new TypeParameterType(newNode);
+    }
     newNode.bound = visitType(node.bound);
     if (node.defaultType != null) {
       newNode.defaultType = visitType(node.defaultType);