Always check the prototype chain for a setter, when setting a property

that does not exist locally.  Previously, map transitions broke this
check. 


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

src/globals.h

@@ -297,14 +297,18 @@ enum InlineCacheState {
 
 
 // Type of properties.
+// Order of properties is significant.
+// Must fit in the BitField PropertyDetails::TypeField.
+// A copy of this is in mirror-delay.js.
 enum PropertyType {
   NORMAL              = 0,  // only in slow mode
-  MAP_TRANSITION      = 1,  // only in fast mode
+  FIELD               = 1,  // only in fast mode
   CONSTANT_FUNCTION   = 2,  // only in fast mode
-  FIELD               = 3,  // only in fast mode
-  CALLBACKS           = 4,
-  CONSTANT_TRANSITION = 5,  // only in fast mode
-  INTERCEPTOR         = 6,
+  CALLBACKS           = 3,
+  INTERCEPTOR         = 4,  // only in lookup results, not in descriptors.
+  FIRST_PHANTOM_PROPERTY_TYPE = 5,  // All properties before this are real.
+  MAP_TRANSITION      = 5,  // only in fast mode
+  CONSTANT_TRANSITION = 6,  // only in fast mode
   NULL_DESCRIPTOR     = 7   // only in fast mode
 };
 

src/mirror-delay.js

@@ -94,14 +94,17 @@ PropertyKind.Named   = 1;
 PropertyKind.Indexed = 2;
 
 
-// Different types of properties. NOTE value 1 is missing as it is used
-// internally for map transition.
+// A copy of the PropertyType enum from global.h
 PropertyType = {};
-PropertyType.Normal           = 0;
-PropertyType.ConstantFunction = 2;
-PropertyType.Field            = 3;
-PropertyType.Callbacks        = 4;
-PropertyType.Interceptor      = 5;
+PropertyType.Normal             = 0;
+PropertyType.Field              = 1;
+PropertyType.ConstantFunction   = 2;
+PropertyType.Callbacks          = 3;
+PropertyType.Interceptor        = 4;
+PropertyType.MapTransition      = 5;
+PropertyType.ConstantTransition = 6;
+PropertyType.NullDescriptor     = 7;
+
 
 
 // Different attributes for a property.

src/objects.cc

@@ -1507,64 +1507,66 @@ Object* JSObject::SetProperty(LookupResult* result,
     && !Top::MayNamedAccess(this, name, v8::ACCESS_SET)) {
     return SetPropertyWithFailedAccessCheck(result, name, value);
   }
-
-  if (result->IsValid()) {
-    if (!result->IsLoaded()) {
-      return SetLazyProperty(result, name, value, attributes);
+  if (result->IsNotFound() || !result->IsProperty()) {
+    // We could not find a local property so let's check whether there is an
+    // accessor that wants to handle the property.
+    LookupResult accessor_result;
+    LookupCallbackSetterInPrototypes(name, &accessor_result);
+    if (accessor_result.IsValid()) {
+      return SetPropertyWithCallback(accessor_result.GetCallbackObject(),
+                                     name,
+                                     value,
+                                     accessor_result.holder());
     }
-    if (result->IsReadOnly() && !result->IsTransitionType()) return value;
-    switch (result->type()) {
-      case NORMAL:
-        property_dictionary()->ValueAtPut(result->GetDictionaryEntry(), value);
-        return value;
-      case FIELD:
-        properties()->set(result->GetFieldIndex(), value);
-        return value;
-      case MAP_TRANSITION:
-        if (attributes == result->GetAttributes()) {
-          // Only use map transition if attributes matches.
-          return AddFastPropertyUsingMap(result->GetTransitionMap(),
-                                         name,
-                                         value);
-        } else {
-          return AddFastProperty(name, value, attributes);
-        }
-      case CONSTANT_FUNCTION:
-        if (value == result->GetConstantFunction()) return value;
-        // Only replace the function if necessary.
-        return ReplaceConstantFunctionProperty(name, value);
-      case CALLBACKS:
-        return SetPropertyWithCallback(result->GetCallbackObject(),
+  }
+  if (result->IsNotFound()) {
+    return AddProperty(name, value, attributes);
+  }
+  if (!result->IsLoaded()) {
+    return SetLazyProperty(result, name, value, attributes);
+  }
+  if (result->IsReadOnly() && result->IsProperty()) return value;
+  // This is a real property that is not read-only, or it is a
+  // transition or null descriptor and there are no setters in the prototypes.
+  switch (result->type()) {
+    case NORMAL:
+      property_dictionary()->ValueAtPut(result->GetDictionaryEntry(), value);
+      return value;
+    case FIELD:
+      properties()->set(result->GetFieldIndex(), value);
+      return value;
+    case MAP_TRANSITION:
+      if (attributes == result->GetAttributes()) {
+        // Only use map transition if the attributes match.
+        return AddFastPropertyUsingMap(result->GetTransitionMap(),
                                        name,
-                                       value,
-                                       result->holder());
-      case INTERCEPTOR:
-        return SetPropertyWithInterceptor(name, value, attributes);
-      case CONSTANT_TRANSITION:
-        // Replace with a MAP_TRANSITION to a new map with a FIELD, even
-        // if the value is a function.
-        // AddProperty has been extended to do this, in this case.
+                                       value);
+      } else {
         return AddFastProperty(name, value, attributes);
-      case NULL_DESCRIPTOR:
-        UNREACHABLE();
-      default:
-        UNREACHABLE();
-    }
-  }
-
-  // We could not find a local property so let's check whether there is an
-  // accessor that wants to handle the property.
-  LookupResult accessor_result;
-  LookupCallbackSetterInPrototypes(name, &accessor_result);
-  if (accessor_result.IsValid()) {
-    return SetPropertyWithCallback(accessor_result.GetCallbackObject(),
-                                   name,
-                                   value,
-                                   accessor_result.holder());
+      }
+    case CONSTANT_FUNCTION:
+      if (value == result->GetConstantFunction()) return value;
+      // Only replace the function if necessary.
+      return ReplaceConstantFunctionProperty(name, value);
+    case CALLBACKS:
+      return SetPropertyWithCallback(result->GetCallbackObject(),
+                                     name,
+                                     value,
+                                     result->holder());
+    case INTERCEPTOR:
+      return SetPropertyWithInterceptor(name, value, attributes);
+    case CONSTANT_TRANSITION:
+      // Replace with a MAP_TRANSITION to a new map with a FIELD, even
+      // if the value is a function.
+      // AddProperty has been extended to do this, in this case.
+      return AddFastProperty(name, value, attributes);
+    case NULL_DESCRIPTOR:
+      UNREACHABLE();
+    default:
+      UNREACHABLE();
   }
-
-  // The property was not found
-  return AddProperty(name, value, attributes);
+  UNREACHABLE();
+  return value;
 }
 
 

src/property.h

@@ -231,11 +231,12 @@ class LookupResult BASE_EMBEDDED {
   bool IsDontEnum() { return details_.IsDontEnum(); }
 
   bool IsValid() { return  lookup_type_ != NOT_FOUND; }
+  bool IsNotFound() { return lookup_type_ == NOT_FOUND; }
 
   // Tells whether the result is a property.
-  // Excluding transitions.
+  // Excluding transitions and the null descriptor.
   bool IsProperty() {
-    return IsValid() && !IsTransitionType();
+    return IsValid() && type() < FIRST_PHANTOM_PROPERTY_TYPE;
   }
 
   bool IsCacheable() { return cacheable_; }