Address review comments from a previous CL.

vm/object.cc

Index: vm/object.cc
===================================================================
--- vm/object.cc	(revision 9828)
+++ vm/object.cc	(working copy)
@@ -97,145 +97,53 @@
 RawClass* Object::unwind_error_class_ = reinterpret_cast<RawClass*>(RAW_NULL);
 #undef RAW_NULL
 
-int Object::GetSingletonClassIndex(const RawClass* raw_class) {
-  ASSERT(raw_class->IsHeapObject());
-  if (raw_class == class_class()) {
-    return kClassClass;
-  } else if (raw_class == null_class()) {
-    return kNullClass;
-  } else if (raw_class == dynamic_class()) {
-    return kDynamicClass;
-  } else if (raw_class == void_class()) {
-    return kVoidClass;
-  } else if (raw_class == unresolved_class_class()) {
-    return kUnresolvedClassClass;
-  } else if (raw_class == type_class()) {
-    return kTypeClass;
-  } else if (raw_class == type_parameter_class()) {
-    return kTypeParameterClass;
-  } else if (raw_class == type_arguments_class()) {
-    return kTypeArgumentsClass;
-  } else if (raw_class == instantiated_type_arguments_class()) {
-    return kInstantiatedTypeArgumentsClass;
-  } else if (raw_class == function_class()) {
-    return kFunctionClass;
-  } else if (raw_class == field_class()) {
-    return kFieldClass;
-  } else if (raw_class == literal_token_class()) {
-    return kLiteralTokenClass;
-  } else if (raw_class == token_stream_class()) {
-    return kTokenStreamClass;
-  } else if (raw_class == script_class()) {
-    return kScriptClass;
-  } else if (raw_class == library_class()) {
-    return kLibraryClass;
-  } else if (raw_class == library_prefix_class()) {
-    return kLibraryPrefixClass;
-  } else if (raw_class == code_class()) {
-    return kCodeClass;
-  } else if (raw_class == instructions_class()) {
-    return kInstructionsClass;
-  } else if (raw_class == pc_descriptors_class()) {
-    return kPcDescriptorsClass;
-  } else if (raw_class == stackmap_class()) {
-    return kStackmapClass;
-  } else if (raw_class == var_descriptors_class()) {
-    return kLocalVarDescriptorsClass;
-  } else if (raw_class == exception_handlers_class()) {
-    return kExceptionHandlersClass;
-  } else if (raw_class == context_class()) {
-    return kContextClass;
-  } else if (raw_class == context_scope_class()) {
-    return kContextScopeClass;
-  } else if (raw_class == icdata_class()) {
-    return kICDataClass;
-  } else if (raw_class == subtypetestcache_class()) {
-    return kSubtypeTestCacheClass;
-  } else if (raw_class == api_error_class()) {
-    return kApiErrorClass;
-  } else if (raw_class == language_error_class()) {
-    return kLanguageErrorClass;
-  } else if (raw_class == unhandled_exception_class()) {
-    return kUnhandledExceptionClass;
-  } else if (raw_class == unwind_error_class()) {
-    return kUnwindErrorClass;
+int Object::GetSingletonClassIndex(const RawObject* raw_obj) {
+  ASSERT(raw_obj->IsHeapObject());
+  intptr_t id = raw_obj->GetClassId();
+  if (id == kClass) {
+    const RawClass* raw_class = reinterpret_cast<const RawClass*>(raw_obj);
+    id = raw_class->ptr()->id_;
+    if ((id >= kClass && id <= kUnwindError) ||
+        (id >= kNullClassId && id <= kVoidClassId)) {

[regis, 2012/07/24 13:49:25]

We usually wrap all terms in parentheses.

[siva, 2012/08/13 20:38:25]

This function has been deleted in newer code.

On 2012/07/24 13:49:25, regis wrote:

+      return id;
+    }
   }
   return kInvalidIndex;
 }
 
 
-RawClass* Object::GetSingletonClass(int index) {
-  switch (index) {
-    case kClassClass: return class_class();
-    case kNullClass: return null_class();
-    case kDynamicClass: return dynamic_class();
-    case kVoidClass: return void_class();
-    case kUnresolvedClassClass: return unresolved_class_class();
-    case kTypeClass: return type_class();
-    case kTypeParameterClass: return type_parameter_class();
-    case kTypeArgumentsClass: return type_arguments_class();
-    case kInstantiatedTypeArgumentsClass:
-        return instantiated_type_arguments_class();
-    case kFunctionClass: return function_class();
-    case kFieldClass: return field_class();
-    case kLiteralTokenClass: return literal_token_class();
-    case kTokenStreamClass: return token_stream_class();
-    case kScriptClass: return script_class();
-    case kLibraryClass: return library_class();
-    case kLibraryPrefixClass: return library_prefix_class();
-    case kCodeClass: return code_class();
-    case kInstructionsClass: return instructions_class();
-    case kPcDescriptorsClass: return pc_descriptors_class();
-    case kStackmapClass: return stackmap_class();
-    case kLocalVarDescriptorsClass: return var_descriptors_class();
-    case kExceptionHandlersClass: return exception_handlers_class();
-    case kContextClass: return context_class();
-    case kContextScopeClass: return context_scope_class();
-    case kICDataClass: return icdata_class();
-    case kSubtypeTestCacheClass: return subtypetestcache_class();
-    case kApiErrorClass: return api_error_class();
-    case kLanguageErrorClass: return language_error_class();
-    case kUnhandledExceptionClass: return unhandled_exception_class();
-    case kUnwindErrorClass: return unwind_error_class();
-    default: break;
-  }
-  UNREACHABLE();
-  return reinterpret_cast<RawClass*>(kHeapObjectTag);  // return RAW_NULL.
-}
-
-
 const char* Object::GetSingletonClassName(int index) {
   switch (index) {
-    case kClassClass: return "Class";
-    case kNullClass: return "Null";
-    case kDynamicClass: return "Dynamic";
-    case kVoidClass: return "void";
-    case kUnresolvedClassClass: return "UnresolvedClass";
-    case kTypeClass: return "Type";
-    case kTypeParameterClass: return "TypeParameter";
-    case kTypeArgumentsClass: return "TypeArguments";
-    case kInstantiatedTypeArgumentsClass: return "InstantiatedTypeArguments";
-    case kFunctionClass: return "Function";
-    case kFieldClass: return "Field";
-    case kLiteralTokenClass: return "LiteralToken";
-    case kTokenStreamClass: return "TokenStream";
-    case kScriptClass: return "Script";
-    case kLibraryClass: return "Library";
-    case kLibraryPrefixClass: return "LibraryPrefix";
-    case kCodeClass: return "Code";
-    case kInstructionsClass: return "Instructions";
-    case kPcDescriptorsClass: return "PcDescriptors";
-    case kStackmapClass: return "Stackmap";
-    case kLocalVarDescriptorsClass: return "LocalVarDescriptors";
-    case kExceptionHandlersClass: return "ExceptionHandlers";
-    case kContextClass: return "Context";
-    case kContextScopeClass: return "ContextScope";
-    case kICDataClass: return "ICData";
-    case kSubtypeTestCacheClass: return "SubtypeTestCache";
-    case kApiErrorClass: return "ApiError";
-    case kLanguageErrorClass: return "LanguageError";
-    case kUnhandledExceptionClass: return "UnhandledException";
-    case kUnwindErrorClass: return "UnwindError";
+    case kClass: return "Class";
+    case kUnresolvedClass: return "UnresolvedClass";
+    case kType: return "Type";
+    case kTypeParameter: return "TypeParameter";
+    case kTypeArguments: return "TypeArguments";
+    case kInstantiatedTypeArguments: return "InstantiatedTypeArguments";
+    case kFunction: return "Function";
+    case kField: return "Field";
+    case kLiteralToken: return "LiteralToken";
+    case kTokenStream: return "TokenStream";
+    case kScript: return "Script";
+    case kLibrary: return "Library";
+    case kLibraryPrefix: return "LibraryPrefix";
+    case kCode: return "Code";
+    case kInstructions: return "Instructions";
+    case kPcDescriptors: return "PcDescriptors";
+    case kStackmap: return "Stackmap";
+    case kLocalVarDescriptors: return "LocalVarDescriptors";
+    case kExceptionHandlers: return "ExceptionHandlers";
+    case kContext: return "Context";
+    case kContextScope: return "ContextScope";
+    case kICData: return "ICData";
+    case kSubtypeTestCache: return "SubtypeTestCache";
+    case kApiError: return "ApiError";
+    case kLanguageError: return "LanguageError";
+    case kUnhandledException: return "UnhandledException";
+    case kUnwindError: return "UnwindError";
+    case kNullClassId: return "Null";
+    case kDynamicClassId: return "Dynamic";
+    case kVoidClassId: return "void";
     default: break;
   }
   UNREACHABLE();
@@ -1052,8 +960,7 @@
     return raw_ptr()->name_;
   }
   ASSERT(class_class() != Class::null());  // class_class_ should be set up.
-  intptr_t index = GetSingletonClassIndex(raw());
-  return String::NewSymbol(GetSingletonClassName(index));
+  return String::NewSymbol(GetSingletonClassName(raw_ptr()->id_));
 }
 
 
@@ -1610,127 +1517,19 @@
 
 
 RawClass* Class::GetClass(ObjectKind kind) {
-  ObjectStore* object_store = Isolate::Current()->object_store();
-  switch (kind) {
-    case kInteger:
-      ASSERT(object_store->integer_implementation_class() != Class::null());
-      return object_store->integer_implementation_class();
-    case kSmi:
-      ASSERT(object_store->smi_class() != Class::null());
-      return object_store->smi_class();
-    case kMint:
-      ASSERT(object_store->mint_class() != Class::null());
-      return object_store->mint_class();
-    case kBigint:
-      ASSERT(object_store->bigint_class() != Class::null());
-      return object_store->bigint_class();
-    case kDouble:
-      ASSERT(object_store->double_class() != Class::null());
-      return object_store->double_class();
-    case kOneByteString:
-      ASSERT(object_store->one_byte_string_class() != Class::null());
-      return object_store->one_byte_string_class();
-    case kTwoByteString:
-      ASSERT(object_store->two_byte_string_class() != Class::null());
-      return object_store->two_byte_string_class();
-    case kFourByteString:
-      ASSERT(object_store->four_byte_string_class() != Class::null());
-      return object_store->four_byte_string_class();
-    case kExternalOneByteString:
-      ASSERT(object_store->external_one_byte_string_class() != Class::null());
-      return object_store->external_one_byte_string_class();
-    case kExternalTwoByteString:
-      ASSERT(object_store->external_two_byte_string_class() != Class::null());
-      return object_store->external_two_byte_string_class();
-    case kExternalFourByteString:
-      ASSERT(object_store->external_four_byte_string_class() != Class::null());
-      return object_store->external_four_byte_string_class();
-    case kBool:
-      ASSERT(object_store->bool_class() != Class::null());
-      return object_store->bool_class();
-    case kArray:
-      ASSERT(object_store->array_class() != Class::null());
-      return object_store->array_class();
-    case kImmutableArray:
-      ASSERT(object_store->immutable_array_class() != Class::null());
-      return object_store->immutable_array_class();
-    case kGrowableObjectArray:
-      ASSERT(object_store->growable_object_array_class() != Class::null());
-      return object_store->growable_object_array_class();
-    case kInt8Array:
-      ASSERT(object_store->int8_array_class() != Class::null());
-      return object_store->int8_array_class();
-    case kUint8Array:
-      ASSERT(object_store->uint8_array_class() != Class::null());
-      return object_store->uint8_array_class();
-    case kInt16Array:
-      ASSERT(object_store->int16_array_class() != Class::null());
-      return object_store->int16_array_class();
-    case kUint16Array:
-      ASSERT(object_store->uint16_array_class() != Class::null());
-      return object_store->uint16_array_class();
-    case kInt32Array:
-      ASSERT(object_store->int32_array_class() != Class::null());
-      return object_store->int32_array_class();
-    case kUint32Array:
-      ASSERT(object_store->uint32_array_class() != Class::null());
-      return object_store->uint32_array_class();
-    case kInt64Array:
-      ASSERT(object_store->int64_array_class() != Class::null());
-      return object_store->int64_array_class();
-    case kUint64Array:
-      ASSERT(object_store->uint64_array_class() != Class::null());
-      return object_store->uint64_array_class();
-    case kFloat32Array:
-      ASSERT(object_store->float32_array_class() != Class::null());
-      return object_store->float32_array_class();
-    case kFloat64Array:
-      ASSERT(object_store->float64_array_class() != Class::null());
-      return object_store->float64_array_class();
-    case kExternalInt8Array:
-      ASSERT(object_store->external_int8_array_class() != Class::null());
-      return object_store->external_int8_array_class();
-    case kExternalUint8Array:
-      ASSERT(object_store->external_uint8_array_class() != Class::null());
-      return object_store->external_uint8_array_class();
-    case kExternalInt16Array:
-      ASSERT(object_store->external_int16_array_class() != Class::null());
-      return object_store->external_int16_array_class();
-    case kExternalUint16Array:
-      ASSERT(object_store->external_uint16_array_class() != Class::null());
-      return object_store->external_uint16_array_class();
-    case kExternalInt32Array:
-      ASSERT(object_store->external_int32_array_class() != Class::null());
-      return object_store->external_int32_array_class();
-    case kExternalUint32Array:
-      ASSERT(object_store->external_uint32_array_class() != Class::null());
-      return object_store->external_uint32_array_class();
-    case kExternalInt64Array:
-      ASSERT(object_store->external_int64_array_class() != Class::null());
-      return object_store->external_int64_array_class();
-    case kExternalUint64Array:
-      ASSERT(object_store->external_uint64_array_class() != Class::null());
-      return object_store->external_uint64_array_class();
-    case kExternalFloat32Array:
-      ASSERT(object_store->external_float32_array_class() != Class::null());
-      return object_store->external_float32_array_class();
-    case kExternalFloat64Array:
-      ASSERT(object_store->external_float64_array_class() != Class::null());
-      return object_store->external_float64_array_class();
-    case kStacktrace:
-      ASSERT(object_store->stacktrace_class() != Class::null());
-      return object_store->stacktrace_class();
-    case kJSRegExp:
-      ASSERT(object_store->jsregexp_class() != Class::null());
-      return object_store->jsregexp_class();
-    case kClosure:
-      return Class::New<Closure>();
-    case kInstance:
-      return Class::New<Instance>();
-    default:
-      OS::Print("Class::GetClass kind unknown: %d\n", kind);
-      UNREACHABLE();
+  if (kind == kClosure) {
+    return Class::New<Closure>();
   }
+  if (kind == kInstance) {
+    return Class::New<Instance>();
+  }
+  if (kind >= kInteger && kind <= kJSRegExp) {
+    Isolate* isolate = Isolate::Current();
+    ASSERT(isolate->class_table()->At(kind) != Class::null());
+    return isolate->class_table()->At(kind);
+  }
+  OS::Print("Class::GetClass kind unknown: %d\n", kind);
+  UNREACHABLE();
   return Class::null();
 }
 
@@ -7217,7 +7016,6 @@
   intptr_t data_pos = old_num * TestEntryLength();
   for (intptr_t i = 0; i < class_ids.length(); i++) {
     // Null is used as terminating value, do not add it.

[regis, 2012/07/24 13:49:25]

You need to sync: r9835 committed - Use kIllegalObjectKind instead of NULL
object to terminate ICData ...

[siva, 2012/08/13 20:38:25]

Done.

-    ASSERT(class_ids[i] != kNullClass);
     ASSERT(class_ids[i] != kIllegalObjectKind);
     data.SetAt(data_pos++, Smi::Handle(Smi::New(class_ids[i])));
   }
@@ -7230,7 +7028,7 @@
                               const Function& target) const {
   ASSERT(num_args_tested() == 1);  // Otherwise use 'AddCheck'.
   // Not supporting collection of null receivers.
-  ASSERT(receiver_class_id != kNullClass);
+  ASSERT(receiver_class_id != kNullClassId);
   ASSERT(receiver_class_id != kIllegalObjectKind);
   ASSERT(!target.IsNull());