Save a handle allocation in two accessor functions of class Type.

runtime/vm/object.cc

Index: runtime/vm/object.cc
===================================================================
--- runtime/vm/object.cc	(revision 15727)
+++ runtime/vm/object.cc	(working copy)
@@ -8712,18 +8712,14 @@
 
 RawClass* Type::type_class() const {
   ASSERT(HasResolvedTypeClass());
-  Class& type_class = Class::Handle();
-  type_class ^= raw_ptr()->type_class_;
-  return type_class.raw();
+  return reinterpret_cast<RawClass*>(raw_ptr()->type_class_);

[Ivan Posva, 2012/12/07 17:17:31]

I do not like the approach taken here. It should be safe in at least debug mode,
giving us the same protection which using the handle does. This pattern here
does not qualify.

[Florian Schneider, 2012/12/10 16:11:27]

What protection do you want here? The caller has to wrap the raw pointer in a
handle anyway, so all asserts in handle-initialization will be done there. I
don't see much benefit of creating a handle here, just for asserting.

[Ivan Posva, 2012/12/10 17:13:42]

Please note that since you are returning a RawClass* here, that there is no
further protection (or checking) if the caller associates this with a Class
handle. This has been a deliberate decision, as we need to be able to trust the
return types of calls.
I did not say that you needed to create a handle here in the non-debug case.

 }
 
 
 RawUnresolvedClass* Type::unresolved_class() const {
-  ASSERT(!HasResolvedTypeClass());
-  UnresolvedClass& unresolved_class = UnresolvedClass::Handle();
-  unresolved_class ^= raw_ptr()->type_class_;
-  ASSERT(!unresolved_class.IsNull());
-  return unresolved_class.raw();
+  ASSERT(!Object::Handle(raw_ptr()->type_class_).IsNull());
+  ASSERT(Object::Handle(raw_ptr()->type_class_).IsUnresolvedClass());
+  return reinterpret_cast<RawUnresolvedClass*>(raw_ptr()->type_class_);

[Ivan Posva, 2012/12/07 17:17:31]

ditto + the check for HasResolvedTypeClass() has disappeared.

[Florian Schneider, 2012/12/10 16:11:27]

Sure, I can re-insert the assert, but it is redundant wrt the asserts I added.

[Ivan Posva, 2012/12/10 17:13:42]

Type::HasResolvedType() conveys a completely different meaning from the
type_class_ field containing an object of type UnresolvedClass. It might be that
this is currently the case, but relying on that fact here makes me nervous.

 }