Fix parsing of ClosureCreation kernel nodes in VM.

Fix parsing of ClosureCreation kernel nodes in VM.

Summary:

Previously, the VM would crash when it encountered a ClosureCreation node
because it was not aware of the new type arguments field.

Now, it skips the type arguemnts field, which allows many tests to pass again,
even though it doesn't correct forward the type arguments at runtime.

Test Plan:

Removed expected failure lines for all the tests added in my prior
CL (introducing the new field).

BUG=
R=dmitryas@google.com, jensj@google.com

Reviewers: dmitryas@google.com
Committed: https://github.com/dart-lang/sdk/commit/e80b42a2358c8d1275a639aab99986b6dd6cb7ba

[sjindel, 2017-07-31 12:37:20]

sjindel@google.com changed reviewers:
+ dmitryas@google.com

[Dmitry Stefantsov, 2017-07-31 15:15:06]

dmitryas@google.com changed reviewers:
+ kustermann@google.com

[Dmitry Stefantsov, 2017-07-31 15:15:07]

LGTM if you respond to the comments below.

Also adding Martin, because I may not be the best reviewer of the VM code out
there :) Martin, can you please take a look?

https://codereview.chromium.org/2987143002/diff/1/runtime/vm/kernel_binary_fl...
File runtime/vm/kernel_binary_flowgraph.cc (right):

https://codereview.chromium.org/2987143002/diff/1/runtime/vm/kernel_binary_fl...
runtime/vm/kernel_binary_flowgraph.cc:700: builder_->SkipListOfDartTypes();  //
read type arguments
As I mentioned, we maintain these comments related to reading the code from the
.dill file.  The purpose is to make the readings more visible, but also to point
to the parts of AST nodes that are being read.  The informal format is (as I
understood it from the chats with @jensj) is this:

// read|skip <name of the field turned in to words with spaces between them>.

So, the full-stop at the end is missing :)

https://codereview.chromium.org/2987143002/diff/1/runtime/vm/kernel_binary_fl...
runtime/vm/kernel_binary_flowgraph.cc:4013: SkipListOfDartTypes();         //
read type arguments
Please, add a full-stop at the end.

https://codereview.chromium.org/2987143002/diff/1/runtime/vm/kernel_binary_fl...
runtime/vm/kernel_binary_flowgraph.cc:5909: SkipListOfDartTypes();  // skip list
of type arguments (for now)
Please, remove "(for now)" and add a full-stop.

[sjindel, 2017-07-31 15:44:31]

https://codereview.chromium.org/2987143002/diff/1/runtime/vm/kernel_binary_fl...
File runtime/vm/kernel_binary_flowgraph.cc (right):

https://codereview.chromium.org/2987143002/diff/1/runtime/vm/kernel_binary_fl...
runtime/vm/kernel_binary_flowgraph.cc:700: builder_->SkipListOfDartTypes();  //
read type arguments
On 2017/07/31 15:15:06, Dmitry Stefantsov wrote:
> As I mentioned, we maintain these comments related to reading the code from
the
> .dill file.  The purpose is to make the readings more visible, but also to
point
> to the parts of AST nodes that are being read.  The informal format is (as I
> understood it from the chats with @jensj) is this:
> 
> // read|skip <name of the field turned in to words with spaces between them>.
> 
> So, the full-stop at the end is missing :)

Done.

https://codereview.chromium.org/2987143002/diff/1/runtime/vm/kernel_binary_fl...
runtime/vm/kernel_binary_flowgraph.cc:4013: SkipListOfDartTypes();         //
read type arguments
On 2017/07/31 15:15:06, Dmitry Stefantsov wrote:
> Please, add a full-stop at the end.

Done.

https://codereview.chromium.org/2987143002/diff/1/runtime/vm/kernel_binary_fl...
runtime/vm/kernel_binary_flowgraph.cc:5909: SkipListOfDartTypes();  // skip list
of type arguments (for now)
On 2017/07/31 15:15:06, Dmitry Stefantsov wrote:
> Please, remove "(for now)" and add a full-stop.

Done.

[sjindel, 2017-08-03 10:58:04]

Description was changed from

==========
Fix parsing of ClosureCreation kernel nodes in VM.

Summary:

Previously, the VM would crash when it encountered a ClosureCreation node
because it was not aware of the new type arguments field.

Now, it skips the type arguemnts field, which allows many tests to pass again,
even though it doesn't correct forward the type arguments at runtime.

Test Plan:

Removed expected failure lines for all the tests added in my prior
CL (introducing the new field).

BUG=

Reviewers: dmitryas@google.com
==========

to

==========
Fix parsing of ClosureCreation kernel nodes in VM.

Summary:

Previously, the VM would crash when it encountered a ClosureCreation node
because it was not aware of the new type arguments field.

Now, it skips the type arguemnts field, which allows many tests to pass again,
even though it doesn't correct forward the type arguments at runtime.

Test Plan:

Removed expected failure lines for all the tests added in my prior
CL (introducing the new field).

BUG=

Reviewers: dmitryas@google.com
==========

[sjindel, 2017-08-03 10:58:04]

sjindel@google.com changed reviewers:
+ jensj@google.com
- kustermann@google.com

[jensj, 2017-08-03 11:01:31]

lgtm

[sjindel, 2017-08-03 14:43:02]

Description was changed from

==========
Fix parsing of ClosureCreation kernel nodes in VM.

Summary:

Previously, the VM would crash when it encountered a ClosureCreation node
because it was not aware of the new type arguments field.

Now, it skips the type arguemnts field, which allows many tests to pass again,
even though it doesn't correct forward the type arguments at runtime.

Test Plan:

Removed expected failure lines for all the tests added in my prior
CL (introducing the new field).

BUG=

Reviewers: dmitryas@google.com
==========

to

==========
Fix parsing of ClosureCreation kernel nodes in VM.

Summary:

Previously, the VM would crash when it encountered a ClosureCreation node
because it was not aware of the new type arguments field.

Now, it skips the type arguemnts field, which allows many tests to pass again,
even though it doesn't correct forward the type arguments at runtime.

Test Plan:

Removed expected failure lines for all the tests added in my prior
CL (introducing the new field).

BUG=
R=dmitryas@google.com, jensj@google.com

Reviewers: dmitryas@google.com
Committed:
https://github.com/dart-lang/sdk/commit/e80b42a2358c8d1275a639aab99986b6dd6cb7ba
==========

[sjindel, 2017-08-03 14:43:04]

Committed patchset #2 (id:20001) manually as
e80b42a2358c8d1275a639aab99986b6dd6cb7ba (presubmit successful).