Introduce a VM type cast to avoid repeating a type test and spare a handle.

Introduce a VM type cast to avoid repeating a type test and spare a handle.
Committed: https://code.google.com/p/dart/source/detail?r=9226

[regis, 2012-06-28 16:24:07]

If this is acceptable, I'll use this type cast where applicable in a following
change list.

Thanks,
Regis

[Ivan Posva, 2012-06-28 16:35:09]

Can you convert a select few cases of ^= usage as well to validate that it will
reduce the need for doing those as well?

Otherwise LGTM.

Thanks,
-Ivan

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.cc#n...
runtime/vm/object.cc:2445: const TypeParameter* type_param =
TypeParameter::Cast(*this);
I am wondering if we will need a
  static const object* Cast(const Object& obj)
eventually.

[siva, 2012-06-28 17:02:51]

LGTM with one suggestion you could consider to make the calling code uniform

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.cc#n...
runtime/vm/object.cc:3315: if ((type_param->index() != i)) {
For example this would be
const TypeParameter& type_param = TypeParameter::Cast(type);
if (type_param.index()) != i) {
   ...
}

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.h#ne...
runtime/vm/object.h:98: }                                                       
                    \
Do you think 
static const object& Cast(const Object& obj) {
  ASSERT(obj.Is##object());
  return *(reinterpret_cast<const object*>(&obj));
}
would make the caller code a lot more uniform? i.e we 
would be using the returned handle as 'handle.' like how we do in most of our
code instead of a '(*handle).' or 'handle->'

[Ivan Posva, 2012-06-28 17:35:57]

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.h#ne...
runtime/vm/object.h:98: }                                                       
                    \
On 2012/06/28 17:02:51, asiva wrote:
> Do you think 
> static const object& Cast(const Object& obj) {
>   ASSERT(obj.Is##object());
>   return *(reinterpret_cast<const object*>(&obj));
> }
> would make the caller code a lot more uniform? i.e we 
> would be using the returned handle as 'handle.' like how we do in most of our
> code instead of a '(*handle).' or 'handle->'

I really like this suggestion because it keeps the code uniform on the outside.

Also, one reason why we have not been doing this casting in the past is that you
now suddenly have aliased handles. This should not be a problem for const
handles, but we should watch out if we start making mistakes as they will be
hard to find.

[regis, 2012-06-28 18:44:51]

Please, have another look.
I added a few more Cast's in object.cc for illustration.

Thanks,
Regis

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.cc#n...
runtime/vm/object.cc:2445: const TypeParameter* type_param =
TypeParameter::Cast(*this);
On 2012/06/28 16:35:09, Ivan Posva wrote:
> I am wondering if we will need a
>   static const object* Cast(const Object& obj)
> eventually.

Isn't it exactly what we have? That is, before I changed it to static const
object& Cast(const Object& obj), as suggested by Siva.

Are you asking if we will need the non-const version, maybe?
static object& Cast(const Object& obj)
I think that could be dangerous.

However, a Cast in the Raw#object implementation would be useful, because we
often need to cast a raw pointer via a handle before returning it:
static Raw#object* Cast(RawObject* obj)

I'll do it in a following cl, if appropriate.

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.cc#n...
runtime/vm/object.cc:3315: if ((type_param->index() != i)) {
On 2012/06/28 17:02:51, asiva wrote:
> For example this would be
> const TypeParameter& type_param = TypeParameter::Cast(type);
> if (type_param.index()) != i) {
>    ...
> }

Done.

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.h#ne...
runtime/vm/object.h:98: }                                                       
                    \
On 2012/06/28 17:02:51, asiva wrote:
> Do you think 
> static const object& Cast(const Object& obj) {
>   ASSERT(obj.Is##object());
>   return *(reinterpret_cast<const object*>(&obj));
> }
> would make the caller code a lot more uniform? i.e we 
> would be using the returned handle as 'handle.' like how we do in most of our
> code instead of a '(*handle).' or 'handle->'

I like it. Done.

https://chromiumcodereview.appspot.com/10713008/diff/1/runtime/vm/object.h#ne...
runtime/vm/object.h:98: }                                                       
                    \
On 2012/06/28 17:35:58, Ivan Posva wrote:
> On 2012/06/28 17:02:51, asiva wrote:
> > Do you think 
> > static const object& Cast(const Object& obj) {
> >   ASSERT(obj.Is##object());
> >   return *(reinterpret_cast<const object*>(&obj));
> > }
> > would make the caller code a lot more uniform? i.e we 
> > would be using the returned handle as 'handle.' like how we do in most of
our
> > code instead of a '(*handle).' or 'handle->'
> 
> I really like this suggestion because it keeps the code uniform on the
outside.
> 
> Also, one reason why we have not been doing this casting in the past is that
you
> now suddenly have aliased handles. This should not be a problem for const
> handles, but we should watch out if we start making mistakes as they will be
> hard to find.

Done.

[Ivan Posva, 2012-06-28 20:19:16]

Even better...

-Ivan