Bug 1139683 - Rewrite SetExistingProperty with comments and references to the standard. r=efaust.
authorJason Orendorff <jorendorff@mozilla.com>
Mon, 16 Feb 2015 10:48:07 -0600
changeset 264001 a25cc961aaf0c1400ff49514ca10aa7869798742
parent 264000 3e8cc3750b9c64641afc78d374455be8cdc4a79d
child 264002 c85aa8f8883c648c3ec65e6c734560f718aa8e4b
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1139683
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1139683 - Rewrite SetExistingProperty with comments and references to the standard. r=efaust.
js/src/vm/NativeObject.cpp
js/src/vm/Shape.h
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -1395,18 +1395,18 @@ js::NativeDefineProperty(ExclusiveContex
                 !CheckAccessorRedefinition(cx, obj, shape, getter, setter, id, attrs))
             {
                 return false;
             }
 
             attrs = ApplyOrDefaultAttributes(attrs, shape);
 
             if (shape->isAccessorDescriptor() && !(attrs & JSPROP_IGNORE_READONLY)) {
-                // ES6 draft 2014-10-14 9.1.6.3 step 7.c: Since [[Writable]] 
-                // is present, change the existing accessor property to a data 
+                // ES6 draft 2014-10-14 9.1.6.3 step 7.c: Since [[Writable]]
+                // is present, change the existing accessor property to a data
                 // property.
                 updateValue = UndefinedValue();
             } else {
                 // We are at most changing some attributes, and cannot convert
                 // from data descriptor to accessor, or vice versa. Take
                 // everything from the shape that we aren't changing.
                 uint32_t propMask = JSPROP_ENUMERATE | JSPROP_READONLY | JSPROP_PERMANENT;
                 attrs = (shape->attributes() & ~propMask) | (attrs & propMask);
@@ -2012,16 +2012,18 @@ js::SetPropertyByDefining(JSContext *cx,
         : JSPROP_ENUMERATE;
     JSGetterOp getter = clasp->getProperty;
     JSSetterOp setter = clasp->setProperty;
     MOZ_ASSERT(getter != JS_PropertyStub);
     MOZ_ASSERT(setter != JS_StrictPropertyStub);
     if (!receiver->is<NativeObject>())
         return DefineProperty(cx, receiver, id, v, getter, setter, attrs, result);
 
+    // If the receiver is native, there is one more legacy wrinkle: the class
+    // JSSetterOp is called after defining the new property.
     Rooted<NativeObject*> nativeReceiver(cx, &receiver->as<NativeObject>());
     return DefinePropertyOrElement(cx, nativeReceiver, id, getter, setter, attrs, v, true, result);
 }
 
 // When setting |id| for |receiver| and |obj| has no property for id, continue
 // the search up the prototype chain.
 bool
 js::SetPropertyOnProto(JSContext *cx, HandleObject obj, HandleObject receiver,
@@ -2144,64 +2146,91 @@ NativeSet(JSContext *cx, HandleNativeObj
     {
         obj->setSlot(shape->slot(), vp);
     }
 
     return true;  // result is populated by shape->set() above.
 }
 
 /*
- * Implement "the rest of" assignment to receiver[id] when an existing property
- * (shape) has been found on a native object (pobj).
+ * Finish the assignment `receiver[id] = vp` when an existing property (shape)
+ * has been found on a native object (pobj). This implements ES6 draft rev 32
+ * (2015 Feb 2) 9.1.9 steps 5 and 6.
  *
  * It is necessary to pass both id and shape because shape could be an implicit
  * dense or typed array element (i.e. not actually a pointer to a Shape).
  */
 static bool
 SetExistingProperty(JSContext *cx, HandleNativeObject obj, HandleObject receiver, HandleId id,
                     HandleNativeObject pobj, HandleShape shape, MutableHandleValue vp,
                     ObjectOpResult &result)
 {
+    // Step 5 for dense elements.
     if (IsImplicitDenseOrTypedArrayElement(shape)) {
-        /* ES5 8.12.4 [[Put]] step 2, for a dense data property on pobj. */
+        // Step 5.a is a no-op: all dense elements are writable.
+        // Step 5.b has to do with non-object receivers, which we don't support yet.
+
+        // Pure optimization for the common case:
         if (pobj == receiver)
             return SetDenseOrTypedArrayElement(cx, pobj, JSID_TO_INT(id), vp, result);
-    } else {
-        /* ES5 8.12.4 [[Put]] step 2. */
-        if (shape->isAccessorDescriptor()) {
-            if (shape->hasDefaultSetter())
-                return result.fail(JSMSG_GETTER_ONLY);
-        } else {
-            MOZ_ASSERT(shape->isDataDescriptor());
-            if (!shape->writable())
-                return result.fail(JSMSG_READ_ONLY);
-        }
+
+        // Steps 5.c-f.
+        return SetPropertyByDefining(cx, obj, receiver, id, vp, obj == pobj, result);
+    }
 
+    // Step 5 for all other properties.
+    if (shape->isDataDescriptor()) {
+        // Step 5.a.
+        if (!shape->writable())
+            return result.fail(JSMSG_READ_ONLY);
+
+        // steps 5.c-f.
         if (pobj == receiver) {
+            // Pure optimization for the common case. There's no point performing
+            // the lookup in step 5.c again, as our caller just did it for us. The
+            // result is |shape|.
+
+            // Steps 5.e.i-ii.
             if (pobj->is<ArrayObject>() && id == NameToId(cx->names().length)) {
                 Rooted<ArrayObject*> arr(cx, &pobj->as<ArrayObject>());
                 return ArraySetLength(cx, arr, id, shape->attributes(), vp, result);
             }
             return NativeSet(cx, obj, receiver, shape, vp, result);
         }
 
-        // pobj[id] is not an own property of receiver. Call the setter or shadow it.
-        if (!shape->shadowable() &&
+        // SpiderMonkey special case: assigning to an inherited slotless
+        // property causes the setter to be called, instead of shadowing,
+        // unless the existing property is JSPROP_SHADOWABLE (see bug 552432)
+        // or it's the array length property.
+        if (!shape->hasSlot() &&
+            !shape->hasShadowable() &&
             !(pobj->is<ArrayObject>() && id == NameToId(cx->names().length)))
         {
-            // Weird special case: slotless property with default setter.
-            if (shape->hasDefaultSetter() && !shape->hasGetterValue())
+            // Even weirder sub-special-case: inherited slotless data property
+            // with default setter. Wut.
+            if (shape->hasDefaultSetter())
                 return result.succeed();
 
             return shape->set(cx, obj, receiver, vp, result);
         }
+
+        // Shadow pobj[id] by defining a new data property receiver[id].
+        // Delegate everything to SetPropertyByDefining.
+        return SetPropertyByDefining(cx, obj, receiver, id, vp, obj == pobj, result);
     }
 
-    // Shadow pobj[id] by defining a new data property receiver[id].
-    return SetPropertyByDefining(cx, obj, receiver, id, vp, obj == pobj, result);
+    // Steps 6-11.
+    MOZ_ASSERT(shape->isAccessorDescriptor());
+    MOZ_ASSERT_IF(!shape->hasSetterObject(), shape->hasDefaultSetter());
+    if (shape->hasDefaultSetter())
+        return result.fail(JSMSG_GETTER_ONLY);
+    Value setter = ObjectValue(*shape->setterObject());
+    if (!InvokeGetterOrSetter(cx, receiver, setter, 1, vp.address(), vp))
+        return false;
+    return result.succeed();
 }
 
 bool
 js::NativeSetProperty(JSContext *cx, HandleNativeObject obj, HandleObject receiver, HandleId id,
                       QualifiedBool qualified, MutableHandleValue vp, ObjectOpResult &result)
 {
     // Fire watchpoints, if any.
     if (MOZ_UNLIKELY(obj->watched())) {
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -1014,26 +1014,17 @@ class Shape : public gc::TenuredCell
 
     bool isDataDescriptor() const {
         return (attrs & (JSPROP_SETTER | JSPROP_GETTER)) == 0;
     }
     bool isAccessorDescriptor() const {
         return (attrs & (JSPROP_SETTER | JSPROP_GETTER)) != 0;
     }
 
-    /*
-     * For ES5 compatibility, we allow properties with SetterOp-flavored
-     * setters to be shadowed when set. The "own" property thereby created in
-     * the directly referenced object will have the same getter and setter as
-     * the prototype property. See bug 552432.
-     */
-    bool shadowable() const {
-        MOZ_ASSERT_IF(isDataDescriptor(), writable());
-        return hasSlot() || (attrs & JSPROP_SHADOWABLE);
-    }
+    bool hasShadowable() const { return attrs & JSPROP_SHADOWABLE; }
 
     uint32_t entryCount() {
         if (hasTable())
             return table().entryCount();
         uint32_t count = 0;
         for (Shape::Range<NoGC> r(this); !r.empty(); r.popFront())
             ++count;
         return count;