Move Null type to the Bottom in the VM (fixes #28025).

Move Null type to the Bottom in the VM (fixes #28025).
Fix wrong comments.

R=asiva@google.com

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

[regis, 2017-01-03 22:27:58]

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

[regis, 2017-01-03 22:27:58]

Ryan, all the mirror tests passes, but, when you are back, I'd like you to check
that mirrors are not using NullType or NullClass in a IsSubtypeOf test in order
to test a null instance in an instanceOf test. Indeed, from now on, the Null
type is at Bottom, but not the null instance. See comments in the cl. Thanks!

[siva, 2017-01-05 01:51:04]

lgtm with one comment.

https://codereview.chromium.org/2608373002/diff/20001/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler_mips.cc (right):

https://codereview.chromium.org/2608373002/diff/20001/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_mips.cc:555: ASSERT(!type.IsObjectType() &&
!type.IsDynamicType());
Why didn't you add the same ASSERT to the arm/arm64 and ia32 target files?

[regis, 2017-01-05 18:02:01]

Thanks!

https://codereview.chromium.org/2608373002/diff/20001/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler_mips.cc (right):

https://codereview.chromium.org/2608373002/diff/20001/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler_mips.cc:555: ASSERT(!type.IsObjectType() &&
!type.IsDynamicType());
On 2017/01/05 01:51:04, siva wrote:
> Why didn't you add the same ASSERT to the arm/arm64 and ia32 target files?

Good catch! No reason. Added.

[regis, 2017-01-05 18:05:36]

Description was changed from

==========
Move Null type to the Bottom in the VM (fixes #28025).
Fix wrong comments.
==========

to

==========
Move Null type to the Bottom in the VM (fixes #28025).
Fix wrong comments.

R=asiva@google.com

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

[regis, 2017-01-05 18:05:37]

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

[rmacnak, 2017-01-05 18:22:22]

On 2017/01/03 22:27:58, regis wrote:
> Ryan, all the mirror tests passes, but, when you are back, I'd like you to
check
> that mirrors are not using NullType or NullClass in a IsSubtypeOf test in
order
> to test a null instance in an instanceOf test. Indeed, from now on, the Null
> type is at Bottom, but not the null instance. See comments in the cl. Thanks!

I think mirrors do not use a test like that that would need to be updated. They
do, however, expose TypeMirror.isAssignableTo and TypeMirror.isSubtypeOf. So
could you add some cases with the Null type to
tests/lib/mirrors/relation_assignable_test.dart and relation_subtype_test.dart?

[regis, 2017-01-05 19:23:08]

On 2017/01/05 18:22:22, rmacnak wrote:
> On 2017/01/03 22:27:58, regis wrote:
> > Ryan, all the mirror tests passes, but, when you are back, I'd like you to
> check
> > that mirrors are not using NullType or NullClass in a IsSubtypeOf test in
> order
> > to test a null instance in an instanceOf test. Indeed, from now on, the Null
> > type is at Bottom, but not the null instance. See comments in the cl.
Thanks!
> 
> I think mirrors do not use a test like that that would need to be updated.
They
> do, however, expose TypeMirror.isAssignableTo and TypeMirror.isSubtypeOf. So
> could you add some cases with the Null type to
> tests/lib/mirrors/relation_assignable_test.dart and
relation_subtype_test.dart?

OK, will do.
Do I understand correctly that TypeMirror.isAssignableTo, although applied to a
TypeMirror, reflects the behavior of instances of the mirrored type beeing
assigned? In contrast, TypeMirror.isSubtypeOf reflects the behavior of types and
not instances, correct?

[regis, 2017-01-05 20:26:25]

On 2017/01/05 19:23:08, regis wrote:
> On 2017/01/05 18:22:22, rmacnak wrote:
> > On 2017/01/03 22:27:58, regis wrote:
> > > Ryan, all the mirror tests passes, but, when you are back, I'd like you to
> > check
> > > that mirrors are not using NullType or NullClass in a IsSubtypeOf test in
> > order
> > > to test a null instance in an instanceOf test. Indeed, from now on, the
Null
> > > type is at Bottom, but not the null instance. See comments in the cl.
> Thanks!
> > 
> > I think mirrors do not use a test like that that would need to be updated.
> They
> > do, however, expose TypeMirror.isAssignableTo and TypeMirror.isSubtypeOf. So
> > could you add some cases with the Null type to
> > tests/lib/mirrors/relation_assignable_test.dart and
> relation_subtype_test.dart?
> 
> OK, will do.
> Do I understand correctly that TypeMirror.isAssignableTo, although applied to
a
> TypeMirror, reflects the behavior of instances of the mirrored type beeing
> assigned? In contrast, TypeMirror.isSubtypeOf reflects the behavior of types
and
> not instances, correct?

[Answering my own questions]
Not quite. The difference is that TypeMirror.isAssignableTo is checking the <=>
relation of the spec and TypeMirror.isSubtypeOf the <: of the spec. Both are
applied to types, not instances of that type. Cl adding tests on its way.