Bug 914314, part 1 - Move Shape::get into NativeObject.cpp and rename it to CallGetter. Eliminate unused parameters and consolidate. Add some comments; delete some dead code. No change in behavior. r=efaust.

--HG--
extra : rebase_source : fd3beea0159df960d056b378e9ceee26ef4da9a3

js/src/vm/NativeObject.cpp

@@ -1518,22 +1518,38 @@ js::NativeDefineElement(ExclusiveContext *cx, HandleNativeObject obj, uint32_t i
     return NativeDefineProperty(cx, obj, id, value, getter, setter, attrs);
 }
 
+
+/*** [[Get]] *************************************************************************************/
+
+static inline bool
+CallGetter(JSContext* cx, HandleObject receiver, HandleShape shape, MutableHandleValue vp)
+{
+    MOZ_ASSERT(!shape->hasDefaultGetter());
+
+    if (shape->hasGetterValue()) {
+        Value fval = shape->getterValue();
+        return InvokeGetterOrSetter(cx, receiver, fval, 0, 0, vp);
+    }
+
+    RootedId id(cx, shape->propid());
+    return CallJSPropertyOp(cx, shape->getterOp(), receiver, id, vp);
+}
+
 template <AllowGC allowGC>
 static MOZ_ALWAYS_INLINE bool
-NativeGetExistingPropertyInline(JSContext *cx,
+GetExistingProperty(JSContext *cx,
-                                typename MaybeRooted<JSObject*, allowGC>::HandleType obj,
+                    typename MaybeRooted<JSObject*, allowGC>::HandleType receiver,
-                                typename MaybeRooted<JSObject*, allowGC>::HandleType receiver,
+                    typename MaybeRooted<NativeObject*, allowGC>::HandleType obj,
-                                typename MaybeRooted<NativeObject*, allowGC>::HandleType pobj,
+                    typename MaybeRooted<Shape*, allowGC>::HandleType shape,
-                                typename MaybeRooted<Shape*, allowGC>::HandleType shape,
+                    typename MaybeRooted<Value, allowGC>::MutableHandleType vp)
-                                typename MaybeRooted<Value, allowGC>::MutableHandleType vp)
 {
     if (shape->hasSlot()) {
-        vp.set(pobj->getSlot(shape->slot()));
+        vp.set(obj->getSlot(shape->slot()));
         MOZ_ASSERT_IF(!vp.isMagic(JS_UNINITIALIZED_LEXICAL) &&
-                      !pobj->hasSingletonType() &&
+                      !obj->hasSingletonType() &&
-                      !pobj->template is<ScopeObject>() &&
+                      !obj->template is<ScopeObject>() &&
                       shape->hasDefaultGetter(),
-                      js::types::TypeHasProperty(cx, pobj->type(), shape->propid(), vp));
+                      js::types::TypeHasProperty(cx, obj->type(), shape->propid(), vp));
     } else {
         vp.setUndefined();
     }
@@ -1559,40 +1575,41 @@ NativeGetExistingPropertyInline(JSContext *cx,
     if (!allowGC)
         return false;
 
-    if (!shape->get(cx,
+    if (!CallGetter(cx,
                     MaybeRooted<JSObject*, allowGC>::toHandle(receiver),
-                    MaybeRooted<JSObject*, allowGC>::toHandle(obj),
+                    MaybeRooted<Shape*, allowGC>::toHandle(shape),
-                    MaybeRooted<JSObject*, allowGC>::toHandle(pobj),
                     MaybeRooted<Value, allowGC>::toMutableHandle(vp)))
     {
         return false;
     }
 
-    /* Update slotful shapes according to the value produced by the getter. */
+    // Ancient nonstandard extension: via the JSAPI it's possible to create a
-    if (shape->hasSlot() && pobj->contains(cx, shape))
+    // data property that has both a slot and a getter. In that case, copy the
-        pobj->setSlot(shape->slot(), vp);
+    // value returned by the getter back into the slot.
+    if (shape->hasSlot() && obj->contains(cx, shape))
+        obj->setSlot(shape->slot(), vp);
 
     return true;
 }
 
 bool
-js::NativeGetExistingProperty(JSContext *cx, HandleObject obj, HandleNativeObject pobj,
+js::NativeGetExistingProperty(JSContext *cx, HandleObject receiver, HandleNativeObject obj,
                               HandleShape shape, MutableHandleValue vp)
 {
-    return NativeGetExistingPropertyInline<CanGC>(cx, obj, obj, pobj, shape, vp);
+    return GetExistingProperty<CanGC>(cx, receiver, obj, shape, vp);
 }
 
 /*
  * Given pc pointing after a property accessing bytecode, return true if the
- * access is "object-detecting" in the sense used by web scripts, e.g., when
+ * access is "property-detecting" -- that is, if we shouldn't warn about it
- * checking whether document.all is defined.
+ * even if no such property is found and strict warnings are enabled.
  */
 static bool
 Detecting(JSContext *cx, JSScript *script, jsbytecode *pc)
 {
     MOZ_ASSERT(script->containsPC(pc));
 
-    /* General case: a branch or equality op follows the access. */
+    // General case: a branch or equality op follows the access.
     JSOp op = JSOp(*pc);
     if (js_CodeSpec[op].format & JOF_DETECTING)
         return true;
@@ -1600,10 +1617,7 @@ Detecting(JSContext *cx, JSScript *script, jsbytecode *pc)
     jsbytecode *endpc = script->codeEnd();
 
     if (op == JSOP_NULL) {
-        /*
+        // Special case #1: don't warn about (obj.prop == null).
-         * Special case #1: handle (document.all == null).  Don't sweat
-         * about JS1.2's revision of the equality operators here.
-         */
         if (++pc < endpc) {
             op = JSOp(*pc);
             return op == JSOP_EQ || op == JSOP_NE;
@@ -1612,11 +1626,7 @@ Detecting(JSContext *cx, JSScript *script, jsbytecode *pc)
     }
 
     if (op == JSOP_GETGNAME || op == JSOP_GETNAME) {
-        /*
+        // Special case #2: don't warn about (obj.prop == undefined).
-         * Special case #2: handle (document.all == undefined).  Don't worry
-         * about a local variable named |undefined| shadowing the immutable
-         * global binding...because, really?
-         */
         JSAtom *atom = script->getAtom(GET_UINT32_INDEX(pc));
         if (atom == cx->names().undefined &&
             (pc += js_CodeSpec[op].length) < endpc) {
@@ -1630,13 +1640,13 @@ Detecting(JSContext *cx, JSScript *script, jsbytecode *pc)
 
 template <AllowGC allowGC>
 static MOZ_ALWAYS_INLINE bool
-GetPropertyHelperInline(JSContext *cx,
+NativeGetPropertyInline(JSContext *cx,
                         typename MaybeRooted<NativeObject*, allowGC>::HandleType obj,
                         typename MaybeRooted<JSObject*, allowGC>::HandleType receiver,
                         typename MaybeRooted<jsid, allowGC>::HandleType id,
                         typename MaybeRooted<Value, allowGC>::MutableHandleType vp)
 {
-    /* This call site is hot -- use the always-inlined variant of LookupNativeProperty(). */
+    /* This call site is hot -- use the always-inlined LookupPropertyInline(). */
     typename MaybeRooted<JSObject*, allowGC>::RootType obj2(cx);
     typename MaybeRooted<Shape*, allowGC>::RootType shape(cx);
     if (!LookupPropertyInline<allowGC>(cx, obj, id, &obj2, &shape))
@@ -1737,40 +1747,26 @@ GetPropertyHelperInline(JSContext *cx,
         return true;
     }
 
-    // This call site is hot -- use the always-inlined variant of
+    // This call site is hot -- use the always-inlined GetExistingProperty().
-    // NativeGetExistingProperty().
+    return GetExistingProperty<allowGC>(cx, receiver, nobj2, shape, vp);
-    if (!NativeGetExistingPropertyInline<allowGC>(cx, obj, receiver, nobj2, shape, vp))
-        return false;
-
-    return true;
 }
 
 bool
 js::NativeGetProperty(JSContext *cx, HandleNativeObject obj, HandleObject receiver, HandleId id,
                       MutableHandleValue vp)
 {
-    /* This call site is hot -- use the always-inlined variant of GetPropertyHelper(). */
+    return NativeGetPropertyInline<CanGC>(cx, obj, receiver, id, vp);
-    return GetPropertyHelperInline<CanGC>(cx, obj, receiver, id, vp);
 }
 
 bool
 js::NativeGetPropertyNoGC(JSContext *cx, NativeObject *obj, JSObject *receiver, jsid id, Value *vp)
 {
-    AutoAssertNoException nogc(cx);
+    AutoAssertNoException noexc(cx);
-    return GetPropertyHelperInline<NoGC>(cx, obj, receiver, id, vp);
+    return NativeGetPropertyInline<NoGC>(cx, obj, receiver, id, vp);
 }
 
-bool
-js::NativeGetElement(JSContext *cx, HandleNativeObject obj, HandleObject receiver, uint32_t index,
-                     MutableHandleValue vp)
-{
-    RootedId id(cx);
-    if (!IndexToId(cx, index, &id))
-        return false;
 
-    /* This call site is hot -- use the always-inlined variant of js_GetPropertyHelper(). */
+/*** [[Set]] *************************************************************************************/
-    return GetPropertyHelperInline<CanGC>(cx, obj, receiver, id, vp);
-}
 
 static bool
 MaybeReportUndeclaredVarAssignment(JSContext *cx, JSString *propname)
@@ -1796,9 +1792,6 @@ MaybeReportUndeclaredVarAssignment(JSContext *cx, JSString *propname)
                                         JSMSG_UNDECLARED_VAR, bytes.ptr());
 }
 
-
-/*** [[Set]] *************************************************************************************/
-
 /*
  * When a [[Set]] operation finds no existing property with the given id
  * or finds a writable data property on the prototype chain, we end up here.
@@ -2128,6 +2121,9 @@ js::NativeSetElement(JSContext *cx, HandleNativeObject obj, HandleObject receive
     return NativeSetProperty(cx, obj, receiver, id, Qualified, vp, strict);
 }
 
+
+/* * */
+
 bool
 js::NativeSetPropertyAttributes(JSContext *cx, HandleNativeObject obj, HandleId id,
                                 unsigned *attrsp)

js/src/vm/NativeObject.h

@@ -1322,8 +1322,15 @@ extern bool
 NativeLookupElement(JSContext *cx, HandleNativeObject obj, uint32_t index,
                     MutableHandleObject objp, MutableHandleShape propp);
 
+/*
+ * Get a property from `receiver`, after having already done a lookup and found
+ * the property on a native object `obj`.
+ *
+ * `shape` must not be null and must not be an implicit dense property. It must
+ * be present in obj's shape chain.
+ */
 extern bool
-NativeGetExistingProperty(JSContext *cx, HandleObject obj, HandleNativeObject pobj,
+NativeGetExistingProperty(JSContext *cx, HandleObject receiver, HandleNativeObject obj,
                           HandleShape shape, MutableHandle<Value> vp);
 
 extern bool

js/src/vm/Shape-inl.h

@@ -33,21 +33,6 @@ StackBaseShape::StackBaseShape(ExclusiveContext *cx, const Class *clasp,
     compartment(cx->compartment_)
 {}
 
-inline bool
-Shape::get(JSContext* cx, HandleObject receiver, JSObject* obj, JSObject *pobj,
-           MutableHandleValue vp)
-{
-    MOZ_ASSERT(!hasDefaultGetter());
-
-    if (hasGetterValue()) {
-        Value fval = getterValue();
-        return InvokeGetterOrSetter(cx, receiver, fval, 0, 0, vp);
-    }
-
-    RootedId id(cx, propid());
-    return CallJSPropertyOp(cx, getterOp(), receiver, id, vp);
-}
-
 inline Shape *
 Shape::search(ExclusiveContext *cx, jsid id)
 {

js/src/vm/Shape.h

@@ -917,7 +917,6 @@ class Shape : public gc::TenuredCell
                setter() == rawSetter;
     }
 
-    bool get(JSContext* cx, HandleObject receiver, JSObject *obj, JSObject *pobj, MutableHandleValue vp);
     bool set(JSContext* cx, HandleObject obj, HandleObject receiver, bool strict, MutableHandleValue vp);
 
     BaseShape *base() const { return base_.get(); }