Reviewers: Kevin Millikin,

Description:
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.


Please review this at http://codereview.chromium.org/8491016/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/ic.cc
  M src/objects-inl.h
  M src/objects-printer.cc
  M src/objects.h
  M src/objects.cc
  M src/property.h
  M src/runtime.cc
  M src/v8globals.h


Index: src/ic.cc
diff --git a/src/ic.cc b/src/ic.cc
index fbe77b09d23fa7daccabb0a842ae49221adf9b77..7b0a815aa646fd983b4a5341bd595911ac5a44f7 100644
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -1354,7 +1354,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 +1389,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;
   }

@@ -1719,6 +1724,13 @@ void KeyedStoreIC::UpdateCaches(LookupResult* lookup,
   Handle<Code> code;

   switch (type) {
+    case CONSTANT_FUNCTION:
+    case HANDLER:
+    case INTERCEPTOR:
+    case ELEMENTS_TRANSITION:
+    case NULL_DESCRIPTOR:
+      UNREACHABLE();
+      return;
     case FIELD:
       code = isolate()->stub_cache()->ComputeKeyedStoreField(
           name, receiver, lookup->GetFieldIndex(),
@@ -1726,7 +1738,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,7 +1745,9 @@ void KeyedStoreIC::UpdateCaches(LookupResult* lookup,
         break;
       }
       // fall through.
-    default:
+    case NORMAL:
+    case CALLBACKS:
+    case CONSTANT_TRANSITION:
       // Always rewrite to the generic case so that we do not
       // repeatedly try to rewrite.
       code = (strict_mode == kStrictMode)
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 55a3b2ff40d247e73d1c7fdf1d1c8d7c9751d708..3b5f58ab2ecf2d61a07ea2b5fdcd0b64402a435a 100644
--- 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));
 }


Index: src/objects-printer.cc
diff --git a/src/objects-printer.cc b/src/objects-printer.cc
index 1ca97de8675e7ebf8972b43ef2c1b3848f4defc5..c9f3f842dff455516bdf53a09da306dd876e0dca 100644
--- 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;
       }
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 3a18184673eef0a52f5f25c4e285356678373152..bddf1a8566d64b47f397c91ab8b9da861b0fa5fe 100644
--- 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;
 }

Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index b423cd427994ec919f328da4b379fdf7b3a7c052..c54df59d97ececfd3e8ef05a6388624004350395 100644
--- 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() {
Index: src/property.h
diff --git a/src/property.h b/src/property.h
index ffea41e66688ae46af057dc51736f0bad54384c1..133abc1b42ef0bd50e307485f5e0f7cc3fb7a300 100644
--- 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());
   }

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index 29807da9f296e3e5e4bf33a89fa329793fffac0d..c1a87dbf824f4721f0e6a64031fcc4ca922b68d3 100644
--- 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();
 }

Index: src/v8globals.h
diff --git a/src/v8globals.h b/src/v8globals.h
index 560e36874c19102bb8a8be9343efe28d561820f1..3a29a64453aa70baf3df85f68babce4cdf51b8f3 100644
--- 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 {


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to