Refactoring only: Make the handling of PropertyType more explicit.

Do not rely on 'default' clauses or 'if's when analysing a PropertyType, because
this makes it hard to find the relevant places when a new type is added. Note
that the detection of "phantom property types" is left untouched, because this
might have a performance impact, especially for the GC (to be investigated).

This is a preliminary step for introducing a new kind of map transition.

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

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

diff --git a/src/ic.cc b/src/ic.cc
index fbe77b0..b6b0614 100644 (file)
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -1330,10 +1330,12 @@ void StoreIC::UpdateCaches(LookupResult* lookup,
                            Handle<JSObject> receiver,
                            Handle<String> name,
                            Handle<Object> value) {
-  // Skip JSGlobalProxy.
   ASSERT(!receiver->IsJSGlobalProxy());
-
   ASSERT(StoreICableLookup(lookup));
+  // These are not cacheable, so we never see such LookupResults here.
+  ASSERT(lookup->type() != HANDLER);
+  // We get only called for properties or transitions, see StoreICableLookup.
+  ASSERT(lookup->type() != NULL_DESCRIPTOR);
 
   // If the property has a non-field type allowing map transitions
   // where there is extra room in the object, we leave the IC in its
@@ -1354,7 +1356,6 @@ void StoreIC::UpdateCaches(LookupResult* lookup,
       break;
     case MAP_TRANSITION: {
       if (lookup->GetAttributes() != NONE) return;
-      ASSERT(type == MAP_TRANSITION);
       Handle<Map> transition(lookup->GetTransitionMap());
       int index = transition->PropertyIndexFor(*name);
       code = isolate()->stub_cache()->ComputeStoreField(
@@ -1390,7 +1391,13 @@ void StoreIC::UpdateCaches(LookupResult* lookup,
       code = isolate()->stub_cache()->ComputeStoreInterceptor(
           name, receiver, strict_mode);
       break;
-    default:
+    case CONSTANT_FUNCTION:
+    case CONSTANT_TRANSITION:
+    case ELEMENTS_TRANSITION:
+      return;
+    case HANDLER:
+    case NULL_DESCRIPTOR:
+      UNREACHABLE();
       return;
   }
 
@@ -1636,13 +1643,12 @@ MaybeObject* KeyedStoreIC::Store(State state,
       return *value;
     }
 
-    // Lookup the property locally in the receiver.
-    LookupResult lookup(isolate());
-    receiver->LocalLookup(*name, &lookup);
-
     // Update inline cache and stub cache.
-    if (FLAG_use_ic) {
-      UpdateCaches(&lookup, state, strict_mode, receiver, name, value);
+    if (FLAG_use_ic && !receiver->IsJSGlobalProxy()) {
+      LookupResult lookup(isolate());
+      if (LookupForWrite(receiver, name, &lookup)) {
+        UpdateCaches(&lookup, state, strict_mode, receiver, name, value);
+      }
     }
 
     // Set the property.
@@ -1698,15 +1704,12 @@ void KeyedStoreIC::UpdateCaches(LookupResult* lookup,
                                 Handle<JSObject> receiver,
                                 Handle<String> name,
                                 Handle<Object> value) {
-  // Skip JSGlobalProxy.
-  if (receiver->IsJSGlobalProxy()) return;
-
-  // Bail out if we didn't find a result.
-  if (!lookup->IsPropertyOrTransition() || !lookup->IsCacheable()) return;
-
-  // If the property is read-only, we leave the IC in its current
-  // state.
-  if (lookup->IsReadOnly()) return;
+  ASSERT(!receiver->IsJSGlobalProxy());
+  ASSERT(StoreICableLookup(lookup));
+  // These are not cacheable, so we never see such LookupResults here.
+  ASSERT(lookup->type() != HANDLER);
+  // We get only called for properties or transitions, see StoreICableLookup.
+  ASSERT(lookup->type() != NULL_DESCRIPTOR);
 
   // If the property has a non-field type allowing map transitions
   // where there is extra room in the object, we leave the IC in its
@@ -1726,7 +1729,6 @@ void KeyedStoreIC::UpdateCaches(LookupResult* lookup,
       break;
     case MAP_TRANSITION:
       if (lookup->GetAttributes() == NONE) {
-        ASSERT(type == MAP_TRANSITION);
         Handle<Map> transition(lookup->GetTransitionMap());
         int index = transition->PropertyIndexFor(*name);
         code = isolate()->stub_cache()->ComputeKeyedStoreField(
@@ -1734,13 +1736,22 @@ void KeyedStoreIC::UpdateCaches(LookupResult* lookup,
         break;
       }
       // fall through.
-    default:
+    case NORMAL:
+    case CONSTANT_FUNCTION:
+    case CALLBACKS:
+    case INTERCEPTOR:
+    case CONSTANT_TRANSITION:
+    case ELEMENTS_TRANSITION:
       // Always rewrite to the generic case so that we do not
       // repeatedly try to rewrite.
       code = (strict_mode == kStrictMode)
           ? generic_stub_strict()
           : generic_stub();
       break;
+    case HANDLER:
+    case NULL_DESCRIPTOR:
+      UNREACHABLE();
+      return;
   }
 
   ASSERT(!code.is_null());

diff --git a/src/objects-inl.h b/src/objects-inl.h
index 55a3b2f..3b5f58a 100644 (file)
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -1865,9 +1865,7 @@ bool DescriptorArray::IsProperty(int descriptor_number) {
 
 
 bool DescriptorArray::IsTransition(int descriptor_number) {
-  PropertyType t = GetType(descriptor_number);
-  return t == MAP_TRANSITION || t == CONSTANT_TRANSITION ||
-      t == ELEMENTS_TRANSITION;
+  return IsTransitionType(GetType(descriptor_number));
 }
 
 

diff --git a/src/objects-printer.cc b/src/objects-printer.cc
index 1ca97de..c9f3f84 100644 (file)
--- a/src/objects-printer.cc
+++ b/src/objects-printer.cc
@@ -295,7 +295,9 @@ void JSObject::PrintProperties(FILE* out) {
         case NULL_DESCRIPTOR:
           PrintF(out, "(null descriptor)\n");
           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
           UNREACHABLE();
           break;
       }

diff --git a/src/objects.cc b/src/objects.cc
index 3a18184..bddf1a8 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -3023,10 +3023,11 @@ MaybeObject* JSObject::SetPropertyForResult(LookupResult* result,
     case NULL_DESCRIPTOR:
     case ELEMENTS_TRANSITION:
       return ConvertDescriptorToFieldAndMapTransition(name, value, attributes);
-    default:
+    case HANDLER:
       UNREACHABLE();
+      return value;
   }
-  UNREACHABLE();
+  UNREACHABLE();  // keep the compiler happy
   return value;
 }
 
@@ -3111,10 +3112,11 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes(
     case NULL_DESCRIPTOR:
     case ELEMENTS_TRANSITION:
       return ConvertDescriptorToFieldAndMapTransition(name, value, attributes);
-    default:
+    case HANDLER:
       UNREACHABLE();
+      return value;
   }
-  UNREACHABLE();
+  UNREACHABLE();  // keep the compiler happy
   return value;
 }
 
@@ -3400,8 +3402,10 @@ MaybeObject* JSObject::NormalizeProperties(PropertyNormalizationMode mode,
       case INTERCEPTOR:
       case ELEMENTS_TRANSITION:
         break;
-      default:
+      case HANDLER:
+      case NORMAL:
         UNREACHABLE();
+        break;
     }
   }
 
@@ -7007,9 +7011,7 @@ void Map::CreateOneBackPointer(Map* target) {
 void Map::CreateBackPointers() {
   DescriptorArray* descriptors = instance_descriptors();
   for (int i = 0; i < descriptors->number_of_descriptors(); i++) {
-    if (descriptors->GetType(i) == MAP_TRANSITION ||
-        descriptors->GetType(i) == ELEMENTS_TRANSITION ||
-        descriptors->GetType(i) == CONSTANT_TRANSITION) {
+    if (descriptors->IsTransition(i)) {
       Object* object = reinterpret_cast<Object*>(descriptors->GetValue(i));
       if (object->IsMap()) {
         CreateOneBackPointer(reinterpret_cast<Map*>(object));
@@ -7047,9 +7049,7 @@ void Map::ClearNonLiveTransitions(Heap* heap, Object* real_prototype) {
     // map is not reached again by following a back pointer from a
     // non-live object.
     PropertyDetails details(Smi::cast(contents->get(i + 1)));
-    if (details.type() == MAP_TRANSITION ||
-        details.type() == ELEMENTS_TRANSITION ||
-        details.type() == CONSTANT_TRANSITION) {
+    if (IsTransitionType(details.type())) {
       Object* object = reinterpret_cast<Object*>(contents->get(i));
       if (object->IsMap()) {
         Map* target = reinterpret_cast<Map*>(object);
@@ -8034,7 +8034,7 @@ const char* Code::PropertyType2String(PropertyType type) {
     case CONSTANT_TRANSITION: return "CONSTANT_TRANSITION";
     case NULL_DESCRIPTOR: return "NULL_DESCRIPTOR";
   }
-  UNREACHABLE();
+  UNREACHABLE();  // keep the compiler happy
   return NULL;
 }
 

diff --git a/src/objects.h b/src/objects.h
index b423cd4..c54df59 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -207,8 +207,7 @@ class PropertyDetails BASE_EMBEDDED {
   bool IsTransition() {
     PropertyType t = type();
     ASSERT(t != INTERCEPTOR);
-    return t == MAP_TRANSITION || t == CONSTANT_TRANSITION ||
-        t == ELEMENTS_TRANSITION;
+    return IsTransitionType(t);
   }
 
   bool IsProperty() {

diff --git a/src/property.h b/src/property.h
index ffea41e..133abc1 100644 (file)
--- a/src/property.h
+++ b/src/property.h
@@ -292,10 +292,10 @@ class LookupResult BASE_EMBEDDED {
     }
   }
 
+
   Map* GetTransitionMap() {
     ASSERT(lookup_type_ == DESCRIPTOR_TYPE);
-    ASSERT(type() == MAP_TRANSITION || type() == CONSTANT_TRANSITION ||
-           type() == ELEMENTS_TRANSITION);
+    ASSERT(IsTransitionType(type()));
     return Map::cast(GetValue());
   }
 

diff --git a/src/runtime.cc b/src/runtime.cc
index 29807da..c1a87db 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -10406,10 +10406,11 @@ static MaybeObject* DebugLookupResultValue(Heap* heap,
     case CONSTANT_TRANSITION:
     case NULL_DESCRIPTOR:
       return heap->undefined_value();
-    default:
+    case HANDLER:
       UNREACHABLE();
+      return heap->undefined_value();
   }
-  UNREACHABLE();
+  UNREACHABLE();  // keep the compiler happy
   return heap->undefined_value();
 }
 

diff --git a/src/v8globals.h b/src/v8globals.h
index 560e368..3a29a64 100644 (file)
--- a/src/v8globals.h
+++ b/src/v8globals.h
@@ -29,6 +29,7 @@
 #define V8_V8GLOBALS_H_
 
 #include "globals.h"
+#include "checks.h"
 
 namespace v8 {
 namespace internal {
@@ -347,6 +348,25 @@ enum PropertyType {
 };
 
 
+inline bool IsTransitionType(PropertyType type) {
+  switch (type) {
+    case MAP_TRANSITION:
+    case CONSTANT_TRANSITION:
+    case ELEMENTS_TRANSITION:
+      return true;
+    case NORMAL:
+    case FIELD:
+    case CONSTANT_FUNCTION:
+    case CALLBACKS:
+    case HANDLER:
+    case INTERCEPTOR:
+    case NULL_DESCRIPTOR:
+      return false;
+  }
+  UNREACHABLE();  // keep the compiler happy
+  return false;
+}
+
 // Whether to remove map transitions and constant transitions from a
 // DescriptorArray.
 enum TransitionFlag {