Refactoring only: Make the handling of PropertyType more explicit.

src/ic.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 1336 matching lines @@
   switch (type) {
     case FIELD:
       code = isolate()->stub_cache()->ComputeStoreField(name,
                                                         receiver,
                                                         lookup->GetFieldIndex(),
                                                         Handle<Map>::null(),
                                                         strict_mode);
       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(
           name, receiver, index, transition, strict_mode);
       break;
     }
     case NORMAL:
       if (receiver->IsGlobalObject()) {
         // The stub generated for the global object picks the value directly
         // from the property cell. So the property must be directly on the
@@ skipping 15 matching lines @@
       if (v8::ToCData<Address>(callback->setter()) == 0) return;
       code = isolate()->stub_cache()->ComputeStoreCallback(
           name, receiver, callback, strict_mode);
       break;
     }
     case INTERCEPTOR:
       ASSERT(!receiver->GetNamedInterceptor()->setter()->IsUndefined());
       code = isolate()->stub_cache()->ComputeStoreInterceptor(
           name, receiver, strict_mode);
       break;
-    default:
+    case CONSTANT_FUNCTION:
+    case CONSTANT_TRANSITION:
+    case ELEMENTS_TRANSITION:
+      return;
+    case HANDLER:

[Kevin Millikin (Chromium), 2011/11/07 14:13:39]

Is HANDLER unreachable because the caller takes care of proxies?  In that case,
then !receiver->IsJSProxy() is also one of the preconditions and should be
asserted on entry to this function.

I find it much more clear to get a precondition assertion violation on entry
rather than as a switch match failure in the middle of the function.

[Sven Panne, 2011/11/08 07:36:10]

Done.

+    case NULL_DESCRIPTOR:
+      UNREACHABLE();
       return;
   }
 
   // Patch the call site depending on the state of the cache.
   if (state == UNINITIALIZED || state == MONOMORPHIC_PROTOTYPE_FAILURE) {
     set_target(*code);
   } else if (state == MONOMORPHIC) {
     // Only move to megamorphic if the target changes.
     if (target() != *code) {
       set_target((strict_mode == kStrictMode)
@@ skipping 308 matching lines @@
   // where there is extra room in the object, we leave the IC in its
   // current state.
   PropertyType type = lookup->type();
 
   // Compute the code stub for this store; used for rewriting to
   // monomorphic state and making sure that the code stub is in the
   // stub cache.
   Handle<Code> code;
 
   switch (type) {
+    case CONSTANT_FUNCTION:

[Kevin Millikin (Chromium), 2011/11/07 14:13:39]

I guess we should not get NULL_DESCRIPTOR here, and HANDLER is covered because
the caller ensures it (?).

I don't immediately see why CONSTANT_FUNCTION, INTERCEPTOR, and
ELEMENTS_TRANSITION is impossible.  It definitely needs a short comment.

[Sven Panne, 2011/11/08 07:36:10]

Done.

+    case HANDLER:
+    case INTERCEPTOR:
+    case ELEMENTS_TRANSITION:
+    case NULL_DESCRIPTOR:
+      UNREACHABLE();
+      return;
     case FIELD:
       code = isolate()->stub_cache()->ComputeKeyedStoreField(
           name, receiver, lookup->GetFieldIndex(),
           Handle<Map>::null(), strict_mode);
       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(
             name, receiver, index, transition, strict_mode);
         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)
           ? generic_stub_strict()
           : generic_stub();
       break;
   }
 
   ASSERT(!code.is_null());
 
@@ skipping 629 matching lines @@
 #undef ADDR
 };
 
 
 Address IC::AddressFromUtilityId(IC::UtilityId id) {
   return IC_utilities[id];
 }
 
 
 } }  // namespace v8::internal