Support BottomType in the VM Kernel reader

Support BottomType in the VM Kernel reader

Implement the boilerplate necessary to support BottomType.  In the VM
we use the type Null.

BUG=
R=paulberry@google.com

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

[Kevin Millikin (Google), 2017-05-03 21:01:05]

kmillikin@google.com changed reviewers:
+ leafp@google.com, paulberry@google.com, regis@google.com

[Kevin Millikin (Google), 2017-05-03 21:01:06]

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

https://codereview.chromium.org/2863493002/diff/1/runtime/vm/kernel_to_il.cc#...
runtime/vm/kernel_to_il.cc:4848: dart::Class::Handle(Z,
I->object_store()->null_class()).CanonicalType();
This is the only interesting code.  Is this the right type?  Regis, is there a
better way?  There are accessors for dynamic_type and void_type in Object, but
nothing for this type.

[Paul Berry, 2017-05-03 21:12:48]

lgtm

I'm not sure if equating Bottom == Null in the VM is the behavior we want in the
long term.  Can we be sure that this won't cause any effects that would be
noticable at runtime?

E.g. something like this:

main() {
  var x = () => throw 'foo';
  var y = () => null;
  var xType = x.runtimeType;  // Should be () -> Bottom
  var yType = y.runtimeType;  // Should be () -> Null
  assert(xType != yType);
}

In any case, lgtm as a short term fix.

[Kevin Millikin (Google), 2017-05-03 21:17:40]

Description was changed from

==========
Support BottomType in the VM Kernel reader

Implement the boilerplate necessary to support BottomType.  In the VM
we use the type Null.

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

to

==========
Support BottomType in the VM Kernel reader

Implement the boilerplate necessary to support BottomType.  In the VM
we use the type Null.

BUG=
R=paulberry@google.com

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

[Kevin Millikin (Google), 2017-05-03 21:17:41]

Committed patchset #1 (id:1) manually as
f13bcbcab6a9ae1120eaed176c9be2854e7664a4 (presubmit successful).

[regis, 2017-05-04 09:35:10]

The VM already handles Null as the bottom type, but not when it is the type of
an instance in an instanceof test. In other words, the null instance is still
only an instance of Null, Object, and dynamic.

For details, see the cl moving Null to the bottom in the VM:
https://mail.google.com/mail/u/0/#search/regis+bottom/15966723b66a866f

Cheers,
Regis

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

https://codereview.chromium.org/2863493002/diff/1/runtime/vm/kernel_to_il.cc#...
runtime/vm/kernel_to_il.cc:4848: dart::Class::Handle(Z,
I->object_store()->null_class()).CanonicalType();
On 2017/05/03 21:01:06, Kevin Millikin (Google) wrote:
> This is the only interesting code.  Is this the right type?  Regis, is there a
> better way?  There are accessors for dynamic_type and void_type in Object, but
> nothing for this type.

This should be fine.

[Paul Berry, 2017-05-04 15:49:45]

On 2017/05/04 09:35:10, regis wrote:
> The VM already handles Null as the bottom type, but not when it is the type of
> an instance in an instanceof test. In other words, the null instance is still
> only an instance of Null, Object, and dynamic.
> 
> For details, see the cl moving Null to the bottom in the VM:
> https://mail.google.com/mail/u/0/#search/regis+bottom/15966723b66a866f

That link just takes me to my own gmail.  Could you give us a link to
codereview.chromium.org, or the SHA of a git commit?

Thanks,
Paul

[regis, 2017-05-04 16:38:43]

On 2017/05/04 15:49:45, Paul Berry wrote:
> On 2017/05/04 09:35:10, regis wrote:
> > The VM already handles Null as the bottom type, but not when it is the type
of
> > an instance in an instanceof test. In other words, the null instance is
still
> > only an instance of Null, Object, and dynamic.
> > 
> > For details, see the cl moving Null to the bottom in the VM:
> > https://mail.google.com/mail/u/0/#search/regis+bottom/15966723b66a866f
> 
> That link just takes me to my own gmail.  Could you give us a link to
> http://codereview.chromium.org, or the SHA of a git commit?
> 
> Thanks,
> Paul

Sorry for pasting the wrong link. Here is the cl:
https://codereview.chromium.org/2608373002/