Update subclasses array and is_implemented bit when loading script snapshots.

Update subclasses array and is_implemented bit when loading script snapshots.

Issue #29774

R=asiva@google.com

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

[rmacnak, 2017-06-12 22:28:12]

rmacnak@google.com changed reviewers:
+ asiva@google.com, vegorov@google.com

[siva, 2017-06-12 22:59:42]

LGTM with one comment.

https://codereview.chromium.org/2933223002/diff/1/runtime/vm/snapshot.cc
File runtime/vm/snapshot.cc (right):

https://codereview.chromium.org/2933223002/diff/1/runtime/vm/snapshot.cc#newc...
runtime/vm/snapshot.cc:245: FixSubclassesAndImplementors();
should only be done when kind == kScript?

This path is also executed for kind == kMessage.

https://codereview.chromium.org/2933223002/diff/1/runtime/vm/snapshot_test.cc
File runtime/vm/snapshot_test.cc (right):

https://codereview.chromium.org/2933223002/diff/1/runtime/vm/snapshot_test.cc...
runtime/vm/snapshot_test.cc:1005:
VM_UNIT_TEST_CASE(CanonicalizationInScriptSnapshots) {
The comment for !defined(PRODUCT) is incorrect, I believe it not defined for
product because the test uses mirrors (dart::mirrors).

Maybe you can get rid of "import 'dart:mirrors';" doesn't seem to be used in the
test.

[rmacnak, 2017-06-12 23:35:52]

Description was changed from

==========
Update subclasses array and is_implemented bit when loading script snapshots.

Fix incorrectly supressed test.

Issue #29774
==========

to

==========
Update subclasses array and is_implemented bit when loading script snapshots.

Issue #29774
==========

[rmacnak, 2017-06-12 23:37:43]

https://codereview.chromium.org/2933223002/diff/1/runtime/vm/snapshot.cc
File runtime/vm/snapshot.cc (right):

https://codereview.chromium.org/2933223002/diff/1/runtime/vm/snapshot.cc#newc...
runtime/vm/snapshot.cc:245: FixSubclassesAndImplementors();
On 2017/06/12 22:59:42, siva wrote:
> should only be done when kind == kScript?
> 
> This path is also executed for kind == kMessage.

Yes, added check for kScript

https://codereview.chromium.org/2933223002/diff/1/runtime/vm/snapshot_test.cc
File runtime/vm/snapshot_test.cc (right):

https://codereview.chromium.org/2933223002/diff/1/runtime/vm/snapshot_test.cc...
runtime/vm/snapshot_test.cc:1005:
VM_UNIT_TEST_CASE(CanonicalizationInScriptSnapshots) {
On 2017/06/12 22:59:42, siva wrote:
> The comment for !defined(PRODUCT) is incorrect, I believe it not defined for
> product because the test uses mirrors (dart::mirrors).
> 
> Maybe you can get rid of "import 'dart:mirrors';" doesn't seem to be used in
the
> test.

It uses reflectClass. Updated comment instead.

[rmacnak, 2017-06-12 23:38:37]

Description was changed from

==========
Update subclasses array and is_implemented bit when loading script snapshots.

Issue #29774
==========

to

==========
Update subclasses array and is_implemented bit when loading script snapshots.

Issue #29774

R=asiva@google.com

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

[rmacnak, 2017-06-12 23:38:38]

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