Migrate test block 113 to Dart 2.0.

Migrate test block 113 to Dart 2.0.

Request particular attention to generic_creation_test and generic_field_mixin6_test, but all of the following required tweaks
to the status files to pass tests:

generic_closure_test
generic_function_bounds_test
generic_function_dcall_test
generic_function_typedef_test
generic_field_mixin6_test
generalized_void_syntax_test
generic_functions_test

BUG=
R=jmesserly@google.com

Committed: https://github.com/dart-lang/sdk/commit/3a8a1ca54c521125aa2c15cc90936b9ce10f6b18

[jcollins, 2017-08-25 19:52:54]

Description was changed from

==========
Migrate test block 113 to Dart 2.0.

BUG=
==========

to

==========
Migrate test block 113 to Dart 2.0.

Request particular attention to generic_creation_test and
generic_field_mixin6_test, but all of the following required tweaks
to the status files to pass tests:

generic_closure_test
generic_function_bounds_test
generic_function_dcall_test
generic_function_typedef_test
generic_field_mixin6_test
generalized_void_syntax_test
generic_functions_test

BUG=
==========

[jcollins, 2017-08-25 19:52:54]

jcollins@google.com changed reviewers:
+ jmesserly@google.com, rnystrom@google.com

[Jennifer Messerly, 2017-08-25 21:59:48]

Looking great! Some suggestions for some of the tests

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_as...
File tests/language_2/generic_async_star_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_as...
tests/language_2/generic_async_star_test.dart:9: Stream/*<T>*/ foo/*<T>*/(/*=T*/
x) async* {
change the generic comments into actual syntax?

Stream<T> foo<T>(T x) async* {

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_as...
tests/language_2/generic_async_star_test.dart:16: await for (var x in
foo/*<int>*/(1)) {
foo<int>(1) ?

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_as...
File tests/language_2/generic_async_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_as...
tests/language_2/generic_async_test.dart:9: Future/*<T>*/ foo/*<T>*/(/*=T*/ x)
async => x;
Future<T> foo<T>(T x) async => x;

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_as...
tests/language_2/generic_async_test.dart:12: Expect.equals(1, await
foo/*<int>*/(1));
foo<int>

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_cl...
File tests/language_2/generic_closure_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_cl...
tests/language_2/generic_closure_test.dart:23: Expect.equals("(int) => int",
f.runtimeType.toString()); //# 01: ok
FYI, I filed https://github.com/dart-lang/sdk/issues/30551 about the difference
in how our different implementations print types.

... I prefer DDC's slim arrow `->` slightly over VM's `=>` (it's less
ambiguous). But happy to change DDC if it means more consistency/easier porting
of tests. :)

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_cl...
tests/language_2/generic_closure_test.dart:24: Expect.equals("(int) => int",
g.runtimeType.toString()); //# 01: ok
this should be (Object) -> int

because of covariant generics, we must reify the type of C<T>.g's parameter as
`Object`

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_cl...
tests/language_2/generic_closure_test.dart:38: Expect.isTrue(g is! G<int,
bool>);
could we add these two tests, to illustrate the difference between `f` and `g`:

Expect.isTrue(f is! G<Object, int>);
Expect.isTrue(g is G<Object, int>);

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_cl...
tests/language_2/generic_closure_test.dart:46: Expect.equals("(bool) => bool",
g.runtimeType.toString()); //# 01: ok
this should be `(Object) -> bool`

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_cl...
tests/language_2/generic_closure_test.dart:60: Expect.equals("(dynamic) =>
dynamic", g.runtimeType.toString()); //# 01: ok
this should be `(Object) -> dynamic`

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fi...
File tests/language_2/generic_field_mixin6_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fi...
tests/language_2/generic_field_mixin6_test.dart:20: checkNoDynamicTypeError(()
=> new C1<int>());
i'm not sure this is testing anything now, since the "T field" line will not be
part of the test (in "none" multittest). So all we're doing is executing a bunch
of constructors with no fields.

Here's a suggested change:

class M<T> {
  T field1 = 0; //# 01: compile-time error
  T field2 = 0 as dynamic;
}

main() {
  // no error: 0 is an int
  new C1<int>();
  new C2();

  // type error: 0 is not a string
  Expect.throws(() {
    new C1<String>();
  }, (e) => e is TypeError);
  Expect.throws(() {
    new C3();
  }, (e) => e is TypeError);
}

... this now validates the runtime behavior, that the implementation performs
the correct type check `is T` on the value of 0 at runtime

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
File tests/language_2/generic_function_dcall_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
tests/language_2/generic_function_dcall_test.dart:51:
Expect.equals(f.runtimeType.toString(), "<T>(T) -> T");
note that this test depends on DDC's implementation of Type.toString()  ("->" vs
"=>")

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
File tests/language_2/generic_function_typedef2_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
tests/language_2/generic_function_typedef2_test.dart:37: Expect.isFalse(b is G);
// //# 00: continued
this will never execute, I think? Since it is part of the "00" multitest, but
that one is already a compile time error. It doesn't hurt anything either...
it's kind of like, dead code in the test. But might be nice to clean up

Same for the other ones below.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
File tests/language_2/generic_function_typedef_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
tests/language_2/generic_function_typedef_test.dart:6: //
VMOptions=--generic-method-syntax --no-reify-generic-functions
we should always be reifying generic functions in Dart 2

so we should remove: --no-reify-generic-functions


(we could change it to --reify-generic-functions, which would make the VM appear
to be passing, not sure if we want to do that)

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
tests/language_2/generic_function_typedef_test.dart:18: Expect.isTrue(foo is F);
this is not true in strong mode.

Here's a fixed up version of this test. The two lines with "sound function
types" messages could be added to a multitest if you want to see everything else
passing on the VM.



// 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.
// Dart test for a function type test that cannot be eliminated at compile time.

// VMOptions=--generic-method-syntax --reify-generic-functions

import "package:expect/expect.dart";

class A {}

typedef F<S> = Function(List<S>, Function(String), num);
typedef G<T> = Function<S>(List<S>, Function<A>(A), T);

foo(List<dynamic> x, bar(String y), num z) {}
foo2(List<int> x, bar(String y), num z) {}
foo3<T>(List<T> x, Function<S>(S) y, num z) {}

main() {
  Expect.isTrue(foo is F);
  Expect.isTrue(foo is F<int>);
  Expect.isTrue(foo is F<bool>);

  Expect.isFalse(foo2 is F,
      "sound function types: cannot allow passing List to List<int>");
  Expect.isTrue(foo2 is F<int>);
  Expect.isFalse(foo2 is F<bool>);

  Expect.isFalse(foo3 is F);
  Expect.isFalse(foo3 is F<int>);
  Expect.isFalse(foo3 is F<bool>);

  Expect.isFalse(foo3 is F);
  Expect.isFalse(foo3 is F<int>);
  Expect.isFalse(foo3 is F<bool>);

  Expect.isFalse(foo is G);
  Expect.isFalse(foo is G<int>);
  Expect.isFalse(foo is G<bool>);

  Expect.isFalse(foo2 is G);
  Expect.isFalse(foo2 is G<int>);
  Expect.isFalse(foo2 is G<bool>);

  Expect.isFalse(foo3 is G<Object>,
      "sound function types: cannot allow passing any Object to num");
  Expect.isTrue(foo3 is G<int>);
  Expect.isFalse(foo3 is G<bool>);
}

[jcollins, 2017-08-29 15:41:06]

Addressed comments, also dropped a StaticWarning that got migrated over into the
analyzer status file.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_as...
File tests/language_2/generic_async_star_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_as...
tests/language_2/generic_async_star_test.dart:9: Stream/*<T>*/ foo/*<T>*/(/*=T*/
x) async* {
On 2017/08/25 21:59:47, Jennifer Messerly wrote:
> change the generic comments into actual syntax?
> 
> Stream<T> foo<T>(T x) async* {

Done.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_as...
tests/language_2/generic_async_star_test.dart:16: await for (var x in
foo/*<int>*/(1)) {
On 2017/08/25 21:59:47, Jennifer Messerly wrote:
> foo<int>(1) ?

Done.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_as...
File tests/language_2/generic_async_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_as...
tests/language_2/generic_async_test.dart:9: Future/*<T>*/ foo/*<T>*/(/*=T*/ x)
async => x;
On 2017/08/25 21:59:47, Jennifer Messerly wrote:
> Future<T> foo<T>(T x) async => x;

Done.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_as...
tests/language_2/generic_async_test.dart:12: Expect.equals(1, await
foo/*<int>*/(1));
On 2017/08/25 21:59:47, Jennifer Messerly wrote:
> foo<int>

Done.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_cl...
File tests/language_2/generic_closure_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_cl...
tests/language_2/generic_closure_test.dart:23: Expect.equals("(int) => int",
f.runtimeType.toString()); //# 01: ok
On 2017/08/25 21:59:47, Jennifer Messerly wrote:
> FYI, I filed https://github.com/dart-lang/sdk/issues/30551 about the
difference
> in how our different implementations print types.
> 
> ... I prefer DDC's slim arrow `->` slightly over VM's `=>` (it's less
> ambiguous). But happy to change DDC if it means more consistency/easier
porting
> of tests. :)

I slightly prefer the VM since it (at a limited glance) matches better with the
language syntax, but consistency would win over all options AFAIAC. :-D

For now, switched to DDC syntax since at least that way this test can pass on
one configuration.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_cl...
tests/language_2/generic_closure_test.dart:24: Expect.equals("(int) => int",
g.runtimeType.toString()); //# 01: ok
On 2017/08/25 21:59:47, Jennifer Messerly wrote:
> this should be (Object) -> int
> 
> because of covariant generics, we must reify the type of C<T>.g's parameter as
> `Object`

Done.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_cl...
tests/language_2/generic_closure_test.dart:38: Expect.isTrue(g is! G<int,
bool>);
On 2017/08/25 21:59:48, Jennifer Messerly wrote:
> could we add these two tests, to illustrate the difference between `f` and
`g`:
> 
> Expect.isTrue(f is! G<Object, int>);
> Expect.isTrue(g is G<Object, int>);

Done.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_cl...
tests/language_2/generic_closure_test.dart:46: Expect.equals("(bool) => bool",
g.runtimeType.toString()); //# 01: ok
On 2017/08/25 21:59:48, Jennifer Messerly wrote:
> this should be `(Object) -> bool`

Done.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_cl...
tests/language_2/generic_closure_test.dart:60: Expect.equals("(dynamic) =>
dynamic", g.runtimeType.toString()); //# 01: ok
On 2017/08/25 21:59:48, Jennifer Messerly wrote:
> this should be `(Object) -> dynamic`

Done.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fi...
File tests/language_2/generic_field_mixin6_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fi...
tests/language_2/generic_field_mixin6_test.dart:20: checkNoDynamicTypeError(()
=> new C1<int>());
On 2017/08/25 21:59:48, Jennifer Messerly wrote:
> i'm not sure this is testing anything now, since the "T field" line will not
be
> part of the test (in "none" multittest). So all we're doing is executing a
bunch
> of constructors with no fields.
> 
> Here's a suggested change:
> 
> class M<T> {
>   T field1 = 0; //# 01: compile-time error
>   T field2 = 0 as dynamic;
> }
> 
> main() {
>   // no error: 0 is an int
>   new C1<int>();
>   new C2();
> 
>   // type error: 0 is not a string
>   Expect.throws(() {
>     new C1<String>();
>   }, (e) => e is TypeError);
>   Expect.throws(() {
>     new C3();
>   }, (e) => e is TypeError);
> }
> 
> ... this now validates the runtime behavior, that the implementation performs
> the correct type check `is T` on the value of 0 at runtime

agreed, this is better.  Done.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
File tests/language_2/generic_function_dcall_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
tests/language_2/generic_function_dcall_test.dart:51:
Expect.equals(f.runtimeType.toString(), "<T>(T) -> T");
On 2017/08/25 21:59:48, Jennifer Messerly wrote:
> note that this test depends on DDC's implementation of Type.toString()  ("->"
vs
> "=>")

Acknowledged.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
File tests/language_2/generic_function_typedef2_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
tests/language_2/generic_function_typedef2_test.dart:37: Expect.isFalse(b is G);
// //# 00: continued
On 2017/08/25 21:59:48, Jennifer Messerly wrote:
> this will never execute, I think? Since it is part of the "00" multitest, but
> that one is already a compile time error. It doesn't hurt anything either...
> it's kind of like, dead code in the test. But might be nice to clean up
> 
> Same for the other ones below.

It may never execute, but it seems to trigger a dart2js crash simply by
existing.  Done and switched 00-03 in the dart2js status file to
MissingCompileTimeError, but I'm of two minds about whether that's a good idea.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
File tests/language_2/generic_function_typedef_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
tests/language_2/generic_function_typedef_test.dart:6: //
VMOptions=--generic-method-syntax --no-reify-generic-functions
On 2017/08/25 21:59:48, Jennifer Messerly wrote:
> we should always be reifying generic functions in Dart 2
> 
> so we should remove: --no-reify-generic-functions
> 
> 
> (we could change it to --reify-generic-functions, which would make the VM
appear
> to be passing, not sure if we want to do that)

Dropped the parameter.

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
tests/language_2/generic_function_typedef_test.dart:18: Expect.isTrue(foo is F);
On 2017/08/25 21:59:48, Jennifer Messerly wrote:
> this is not true in strong mode.
> 
> Here's a fixed up version of this test. The two lines with "sound function
> types" messages could be added to a multitest if you want to see everything
else
> passing on the VM.

More or less, done.  It turns out those two lines aren't quite the only thing
failing on the VM.   Marked up all expectations that fail with a multitest
(after testing them each in their own multitest) and adjusted status files
appropriately.  Failures are consistent across implementations with the
exception of DDC.

> 
> 
> 
> // 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.
> // Dart test for a function type test that cannot be eliminated at compile
time.
> 
> // VMOptions=--generic-method-syntax --reify-generic-functions
> 
> import "package:expect/expect.dart";
> 
> class A {}
> 
> typedef F<S> = Function(List<S>, Function(String), num);
> typedef G<T> = Function<S>(List<S>, Function<A>(A), T);
> 
> foo(List<dynamic> x, bar(String y), num z) {}
> foo2(List<int> x, bar(String y), num z) {}
> foo3<T>(List<T> x, Function<S>(S) y, num z) {}
> 
> main() {
>   Expect.isTrue(foo is F);
>   Expect.isTrue(foo is F<int>);
>   Expect.isTrue(foo is F<bool>);
> 
>   Expect.isFalse(foo2 is F,
>       "sound function types: cannot allow passing List to List<int>");
>   Expect.isTrue(foo2 is F<int>);
>   Expect.isFalse(foo2 is F<bool>);
> 
>   Expect.isFalse(foo3 is F);
>   Expect.isFalse(foo3 is F<int>);
>   Expect.isFalse(foo3 is F<bool>);
> 
>   Expect.isFalse(foo3 is F);
>   Expect.isFalse(foo3 is F<int>);
>   Expect.isFalse(foo3 is F<bool>);
> 
>   Expect.isFalse(foo is G);
>   Expect.isFalse(foo is G<int>);
>   Expect.isFalse(foo is G<bool>);
> 
>   Expect.isFalse(foo2 is G);
>   Expect.isFalse(foo2 is G<int>);
>   Expect.isFalse(foo2 is G<bool>);
> 
>   Expect.isFalse(foo3 is G<Object>,
>       "sound function types: cannot allow passing any Object to num");
>   Expect.isTrue(foo3 is G<int>);
>   Expect.isFalse(foo3 is G<bool>);
> }

[Jennifer Messerly, 2017-08-29 21:16:23]

lgtm!

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
File tests/language_2/generic_function_typedef2_test.dart (right):

https://codereview.chromium.org/3008573002/diff/1/tests/language_2/generic_fu...
tests/language_2/generic_function_typedef2_test.dart:37: Expect.isFalse(b is G);
// //# 00: continued
On 2017/08/29 15:41:06, jcollins wrote:
> On 2017/08/25 21:59:48, Jennifer Messerly wrote:
> > this will never execute, I think? Since it is part of the "00" multitest,
but
> > that one is already a compile time error. It doesn't hurt anything either...
> > it's kind of like, dead code in the test. But might be nice to clean up
> > 
> > Same for the other ones below.
> 
> It may never execute, but it seems to trigger a dart2js crash simply by
> existing.  Done and switched 00-03 in the dart2js status file to
> MissingCompileTimeError, but I'm of two minds about whether that's a good
idea.

oh interesting! that's a great point. yeah either way, I can see the value in
keeping the original code too

[jcollins, 2017-08-30 15:50:24]

Description was changed from

==========
Migrate test block 113 to Dart 2.0.

Request particular attention to generic_creation_test and
generic_field_mixin6_test, but all of the following required tweaks
to the status files to pass tests:

generic_closure_test
generic_function_bounds_test
generic_function_dcall_test
generic_function_typedef_test
generic_field_mixin6_test
generalized_void_syntax_test
generic_functions_test

BUG=
==========

to

==========
Migrate test block 113 to Dart 2.0.

Request particular attention to generic_creation_test and
generic_field_mixin6_test, but all of the following required tweaks
to the status files to pass tests:

generic_closure_test
generic_function_bounds_test
generic_function_dcall_test
generic_function_typedef_test
generic_field_mixin6_test
generalized_void_syntax_test
generic_functions_test

BUG=
R=jmesserly@google.com

Committed:
https://github.com/dart-lang/sdk/commit/3a8a1ca54c521125aa2c15cc90936b9ce10f6b18
==========

[jcollins, 2017-08-30 15:50:26]

Committed patchset #3 (id:40001) manually as
3a8a1ca54c521125aa2c15cc90936b9ce10f6b18.