Made PropertyType handling even more explicit.

Replaced FIRST_PHANTOM_PROPERTY_TYPE by a predicate. Removed the (hopefully)
last default cases for switches on PropertyType. Benchmarks show that both
changes are performace-neutral.

Now every value of PropertyType should either be handled by an explicit case in
a switch or by an equality operator. Therefore, the C++ compiler should finally
be able to tell us which places to touch when changing PropertyType.

Review URL: http://codereview.chromium.org/8506004

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9930 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/factory.cc

@@ -844,7 +844,7 @@ Handle<DescriptorArray> Factory::CopyAppendCallbackDescriptors(
 
   // Copy the descriptors from the array.
   for (int i = 0; i < array->number_of_descriptors(); i++) {
-    if (array->GetType(i) != NULL_DESCRIPTOR) {
+    if (!array->IsNullDescriptor(i)) {
       result->CopyFrom(descriptor_count++, *array, i, witness);
     }
   }

src/mark-compact.cc

@@ -1635,15 +1635,14 @@ void MarkCompactCollector::MarkDescriptorArray(
 
     RecordSlot(slot, slot, *slot);
 
-    PropertyType type = details.type();
-    if (type < FIRST_PHANTOM_PROPERTY_TYPE) {
+    if (details.IsProperty()) {
       HeapObject* object = HeapObject::cast(value);
       MarkBit mark = Marking::MarkBitFrom(HeapObject::cast(object));
       if (!mark.Get()) {
         SetMark(HeapObject::cast(object), mark);
         marking_deque_.PushBlack(object);
       }
-    } else if (type == ELEMENTS_TRANSITION && value->IsFixedArray()) {
+    } else if (details.type() == ELEMENTS_TRANSITION && value->IsFixedArray()) {
       // For maps with multiple elements transitions, the transition maps are
       // stored in a FixedArray. Keep the fixed array alive but not the maps
       // that it refers to.

src/objects-inl.h

@@ -1860,7 +1860,7 @@ AccessorDescriptor* DescriptorArray::GetCallbacks(int descriptor_number) {
 
 
 bool DescriptorArray::IsProperty(int descriptor_number) {
-  return GetType(descriptor_number) < FIRST_PHANTOM_PROPERTY_TYPE;
+  return IsRealProperty(GetType(descriptor_number));
 }
 
 

src/profile-generator.cc

@@ -2114,7 +2114,17 @@ void V8HeapExplorer::ExtractPropertyReferences(JSObject* js_obj,
               js_obj, entry,
               descs->GetKey(i), descs->GetConstantFunction(i));
           break;
-        default: ;
+        case NORMAL:  // only in slow mode
+        case HANDLER:  // only in lookup results, not in descriptors
+        case INTERCEPTOR:  // only in lookup results, not in descriptors
+        case MAP_TRANSITION:  // we do not care about transitions here...
+        case ELEMENTS_TRANSITION:
+        case CONSTANT_TRANSITION:
+        case NULL_DESCRIPTOR:  // ... and not about "holes"
+          break;
+          // TODO(svenpanne): Should we really ignore accessors here?
+        case CALLBACKS:
+          break;
       }
     }
   } else {

src/property-details.h

@@ -61,12 +61,11 @@ enum PropertyType {
   CALLBACKS                 = 3,
   HANDLER                   = 4,  // only in lookup results, not in descriptors
   INTERCEPTOR               = 5,  // only in lookup results, not in descriptors
+  // All properties before MAP_TRANSITION are real.
   MAP_TRANSITION            = 6,  // only in fast mode
   ELEMENTS_TRANSITION       = 7,
   CONSTANT_TRANSITION       = 8,  // only in fast mode
   NULL_DESCRIPTOR           = 9,  // only in fast mode
-  // All properties before MAP_TRANSITION are real.
-  FIRST_PHANTOM_PROPERTY_TYPE = MAP_TRANSITION,
   // There are no IC stubs for NULL_DESCRIPTORS. Therefore,
   // NULL_DESCRIPTOR can be used as the type flag for IC stubs for
   // nonexistent properties.
@@ -94,6 +93,26 @@ inline bool IsTransitionType(PropertyType type) {
 }
 
 
+inline bool IsRealProperty(PropertyType type) {
+  switch (type) {
+    case NORMAL:
+    case FIELD:
+    case CONSTANT_FUNCTION:
+    case CALLBACKS:
+    case HANDLER:
+    case INTERCEPTOR:
+      return true;
+    case MAP_TRANSITION:
+    case ELEMENTS_TRANSITION:
+    case CONSTANT_TRANSITION:
+    case NULL_DESCRIPTOR:
+      return false;
+  }
+  UNREACHABLE();  // keep the compiler happy
+  return false;
+}
+
+
 // PropertyDetails captures type and attributes for a property.
 // They are used both in property dictionaries and instance descriptors.
 class PropertyDetails BASE_EMBEDDED {
@@ -127,7 +146,7 @@ class PropertyDetails BASE_EMBEDDED {
   }
 
   bool IsProperty() {
-    return type() < FIRST_PHANTOM_PROPERTY_TYPE;
+    return IsRealProperty(type());
   }
 
   PropertyAttributes attributes() { return AttributesField::decode(value_); }

src/property.h

@@ -262,7 +262,7 @@ class LookupResult BASE_EMBEDDED {
   // Is the result is a property excluding transitions and the null
   // descriptor?
   bool IsProperty() {
-    return IsFound() && (type() < FIRST_PHANTOM_PROPERTY_TYPE);
+    return IsFound() && GetPropertyDetails().IsProperty();
   }
 
   // Is the result a property or a transition?

src/string-stream.cc

@@ -350,8 +350,7 @@ void StringStream::PrintUsingMap(JSObject* js_object) {
   }
   DescriptorArray* descs = map->instance_descriptors();
   for (int i = 0; i < descs->number_of_descriptors(); i++) {
-    switch (descs->GetType(i)) {
-      case FIELD: {
+    if (descs->GetType(i) == FIELD) {
       Object* key = descs->GetKey(i);
       if (key->IsString() || key->IsNumber()) {
         int len = 3;
@@ -370,10 +369,6 @@ void StringStream::PrintUsingMap(JSObject* js_object) {
         Add("%o\n", value);
       }
     }
-      break;
-      default:
-      break;
-    }
   }
 }