VM: [Kernel] Add toString() support for generated enum classes

VM: [Kernel] Add toString() support for generated enum classes

R=vegorov@google.com

Committed: https://github.com/dart-lang/sdk/commit/23cc9cca4158a78ac58272acd33d3e1dea9cc71e

[kustermann, 2017-01-11 12:25:05]

kustermann@google.com changed reviewers:
+ kmillikin@google.com, vegorov@google.com

[kustermann, 2017-01-11 12:25:06]

https://codereview.chromium.org/2625053003/diff/1/tests/language/enum_test.dart
File tests/language/enum_test.dart (right):

https://codereview.chromium.org/2625053003/diff/1/tests/language/enum_test.da...
tests/language/enum_test.dart:24: identical(const <Enum2>[Enum2.A],
Enum2.values));
I think dartk does the right thing by making the const list be a `const
<Enum2>[]` instead of a `const <dynamic>[]`. So I modified the test.

[Vyacheslav Egorov (Google), 2017-01-11 17:28:28]

LGTM though I don't like how the test is written with ||.

/cc Florian & Lasse. Apparently VM and dart2js have incorrect reified type for
E.values where E is an enum.

https://codereview.chromium.org/2625053003/diff/1/tests/language/enum_test.dart
File tests/language/enum_test.dart (right):

https://codereview.chromium.org/2625053003/diff/1/tests/language/enum_test.da...
tests/language/enum_test.dart:24: identical(const <Enum2>[Enum2.A],
Enum2.values));
On 2017/01/11 12:25:06, kustermann wrote:
> I think dartk does the right thing by making the const list be a `const
> <Enum2>[]` instead of a `const <dynamic>[]`. So I modified the test.

I think it should be only one or another. 

Per specification E.values has type List<E> so the test was wrong.

We might want to file bugs against VM and dart2js about it.

[Lasse Reichstein Nielsen, 2017-01-11 17:55:00]

lrn@google.com changed reviewers:
+ lrn@google.com

[Lasse Reichstein Nielsen, 2017-01-11 17:55:01]

https://codereview.chromium.org/2625053003/diff/1/tests/language/enum_test.dart
File tests/language/enum_test.dart (right):

https://codereview.chromium.org/2625053003/diff/1/tests/language/enum_test.da...
tests/language/enum_test.dart:24: identical(const <Enum2>[Enum2.A],
Enum2.values));
Agree, the spec is clear. Do file bugs (or say so, and I'll do it).

[Vyacheslav Egorov (Google), 2017-01-11 17:56:29]

https://codereview.chromium.org/2625053003/diff/1/tests/language/enum_test.dart
File tests/language/enum_test.dart (right):

https://codereview.chromium.org/2625053003/diff/1/tests/language/enum_test.da...
tests/language/enum_test.dart:24: identical(const <Enum2>[Enum2.A],
Enum2.values));
On 2017/01/11 17:55:00, Lasse Reichstein Nielsen wrote:
> Agree, the spec is clear. Do file bugs (or say so, and I'll do it).
> 

It would be nice if you do that :) Thanks a (metric) ton!

[floitsch, 2017-01-11 17:57:00]

floitsch@google.com changed reviewers:
+ floitsch@google.com

[floitsch, 2017-01-11 17:57:01]

Agreed.
Imho, it should be a typed list. Even more so, if the spec already says so.

Since the unified frontend will replace the broken ones soon, I don't think it's
a pressing matter to fix dart2js or the VM, but filing bugs is probably a good
idea. Maybe it's really easy to fix.
Do you, or should I file them?

[kustermann, 2017-01-11 18:50:28]

Description was changed from

==========
VM: [Kernel] Add toString() support for generated enum classes
==========

to

==========
VM: [Kernel] Add toString() support for generated enum classes

R=vegorov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/23cc9cca4158a78ac58272acd33d3e1dea9cc71e
==========

[kustermann, 2017-01-11 18:50:30]

Committed patchset #3 (id:40001) manually as
23cc9cca4158a78ac58272acd33d3e1dea9cc71e (presubmit successful).