Add support for converted closures with explicit contexts to VM

pkg/kernel/test/closures/suite.dart

 // 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.md file.
 
 library test.kernel.closures.suite;
 
+import 'dart:io' show File;
+
 import 'dart:async' show Future;
 
 import 'package:front_end/physical_file_system.dart' show PhysicalFileSystem;
 
 import 'package:kernel/core_types.dart' show CoreTypes;
 
 import 'package:testing/testing.dart'
-    show Chain, ChainContext, Result, Step, TestDescription, runMe;
+    show
+        Chain,
+        ChainContext,
+        Result,
+        Step,
+        TestDescription,
+        runMe,
+        StdioProcess;
 
 import 'package:front_end/src/fasta/testing/patched_sdk_location.dart'
     show computePatchedSdk;
 
 import 'package:kernel/ast.dart' show Program, Library;
 
-import 'package:kernel/transformations/closure_conversion.dart'
-    as closure_conversion;
-
 import 'package:front_end/src/fasta/testing/kernel_chain.dart'
     show Print, MatchExpectation, WriteDill, ReadDill, Verify;
 
 import 'package:front_end/src/fasta/ticker.dart' show Ticker;
 
 import 'package:front_end/src/fasta/dill/dill_target.dart' show DillTarget;
 
 import 'package:front_end/src/fasta/kernel/kernel_target.dart'
     show KernelTarget;
 
 import 'package:front_end/src/fasta/translate_uri.dart' show TranslateUri;
 
 import 'package:front_end/src/fasta/errors.dart' show InputError;
 
 import 'package:front_end/src/fasta/testing/patched_sdk_location.dart';
 
 import 'package:kernel/kernel.dart' show loadProgramFromBinary;
 
+import 'package:kernel/transformations/closure_conversion.dart'
+    as closure_conversion;
+
 import 'package:kernel/target/targets.dart' show TargetFlags;
 
 import 'package:kernel/target/vm_fasta.dart' show VmFastaTarget;
 
 const String STRONG_MODE = " strong mode ";
 
 class ClosureConversionContext extends ChainContext {
   final bool strongMode;
 
   final TranslateUri uriTranslator;
 
   final List<Step> steps;
+  final Uri vm;
 
   ClosureConversionContext(
-      this.strongMode, bool updateExpectations, this.uriTranslator)
+      this.vm, this.strongMode, bool updateExpectations, this.uriTranslator)
       : steps = <Step>[
           const FastaCompile(),
           const Print(),
           const Verify(true),
           const ClosureConversion(),
           const Print(),
           const Verify(true),
           new MatchExpectation(".expect",
               updateExpectations: updateExpectations),
           const WriteDill(),
           const ReadDill(),
-          // TODO(29143): add `Run` step when Vectors are added to VM.
+          const Run(),
         ];
 
-  Future<Program> loadPlatform() async {
-    Uri sdk = await computePatchedSdk();
-    return loadProgramFromBinary(sdk.resolve('platform.dill').toFilePath());
+  // The platform in these tests are reloaded for each testscase, because the

[kustermann, 2017/06/20 12:13:13]

s/platform/platform.dill/
s/are reloaded/is reloaded/
s/testscase/test case/

[Dmitry Stefantsov, 2017/06/22 14:12:52]

Thanks for the suggestions!  Actually, after disabling representation of
tear-offs via closures in Kernel, the statement about closure conversion being
not idempotent holds no longer.  So, this comment is deleted, and a single
instance of the loaded 'platform.dill' is lazily cached in this method and used
for every test.

+  // closure conversion transformation is performed during the load, and it's
+  // not an idempotent transformation yet.

[kustermann, 2017/06/20 12:13:13]

Maybe add a pointer (github issues / ...), so a reader can find out why it's not
idempotent yet.

[Dmitry Stefantsov, 2017/06/22 14:12:52]

As stated above, after dealing with tear-offs was left for back-ends, closure
conversion became idempotent.  But thanks for pointing to this comment!  It
didn't make sense now, and it should have been deleted.

+  Future<Program> loadPlatform() {
+    return new Future<Program>(() async {
+      Uri sdk = await computePatchedSdk();
+      return loadProgramFromBinary(sdk.resolve('platform.dill').toFilePath());
+    });
   }

[kustermann, 2017/06/20 12:13:13]

Why the extra trip to the event loop via `new Future<Program> ...`, you can just
do

```dart

Future<Program> loadProgram() async {
  final Uri sdk = await computePatchedSdk();
  return loadProgramFronBinary(...);
}
```

[Dmitry Stefantsov, 2017/06/22 14:12:52]

Good finding!

I think I was trying to lazily cache an instance of `Future<Program>` at some
point in a code like this: `program ??= new Future<Program>(() async ...`  And
then later I wanted to reload 'platform.dill' for each test and `program ??= `
part was removed, leaving this ugly method body.  Sorry for that!

I rewrote the method now, as stated above, using lazy caching and the `program
??= ` part, but now it caches an instance of `Program`, not `Future<Program>`,
and does it without an additional async wrapper.

 
   static Future<ClosureConversionContext> create(
       Chain suite, Map<String, String> environment) async {
     Uri sdk = await computePatchedSdk();
+    Uri vm = computeDartVm(sdk);
     Uri packages = Uri.base.resolve(".packages");
     bool strongMode = environment.containsKey(STRONG_MODE);
     bool updateExpectations = environment["updateExpectations"] == "true";
     TranslateUri uriTranslator = await TranslateUri
         .parse(PhysicalFileSystem.instance, sdk, packages: packages);
     return new ClosureConversionContext(
-        strongMode, updateExpectations, uriTranslator);
+        vm, strongMode, updateExpectations, uriTranslator);
   }
 }
 
 Future<ClosureConversionContext> createContext(
     Chain suite, Map<String, String> environment) async {
   environment["updateExpectations"] =
       const String.fromEnvironment("updateExpectations");
   return ClosureConversionContext.create(suite, environment);
 }
 
@@ skipping 40 matching lines @@
       Library library = program.libraries
           .firstWhere((Library library) => library.importUri.scheme != "dart");
       closure_conversion.transformLibraries(coreTypes, <Library>[library]);
       return pass(program);
     } catch (e, s) {
       return crash(e, s);
     }
   }
 }
 
+class Run extends Step<Uri, int, ClosureConversionContext> {
+  const Run();
+
+  String get name => "run";
+
+  Future<Result<int>> run(Uri uri, ClosureConversionContext context) async {
+    File generated = new File.fromUri(uri);
+    StdioProcess process;
+    try {
+      process = await StdioProcess
+          .run(context.vm.toFilePath(), [generated.path, "Hello, World!"]);
+      print(process.output);
+    } finally {
+      generated.parent.delete(recursive: true);
+    }
+    return process.toResult();

[kustermann, 2017/06/20 12:13:13]

I think you could simplify this (slightly):

```dart

  final File generated = new File.fromUri(uri);
  try {
    final StdioProcess process = await StioProcess.run(...);
    print(process.output);
    return process.toResult();
  } finally {
    generated.parent.deleteSync(recursive: true);
  }
```

[Dmitry Stefantsov, 2017/06/22 14:12:51]

Agreed.  I like this changed version better!  I changed the code accordingly.

+  }
+}
+
 main(List<String> arguments) => runMe(arguments, createContext, "testing.json");