[ES6] for...in iteration doesn't comply with the specification
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Feb 2016 00:15:58 +0000 (00:15 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Feb 2016 00:15:58 +0000 (00:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154665

Reviewed by Michael Saboff.

If you read ForIn/OfHeadEvaluation inside the spec:
https://tc39.github.io/ecma262/#sec-runtime-semantics-forin-div-ofheadevaluation-tdznames-expr-iterationkind
It calls EnumerateObjectProperties(obj) to get a set of properties
to enumerate over (it models this "set" as en ES6 generator function).
EnumerateObjectProperties is defined in section 13.7.5.15:
https://tc39.github.io/ecma262/#sec-enumerate-object-properties
The implementation calls Reflect.getOwnPropertyDescriptor(.) on the
properties it sees. We must do the same by modeling the operation as
a [[GetOwnProperty]] instead of a [[HasProperty]] internal method call.

* jit/JITOperations.cpp:
* jit/JITOperations.h:
* runtime/CommonSlowPaths.cpp:
(JSC::SLOW_PATH_DECL):
* runtime/JSObject.cpp:
(JSC::JSObject::hasProperty):
(JSC::JSObject::hasPropertyGeneric):
* runtime/JSObject.h:
* tests/stress/proxy-get-own-property.js:
(assert):
(let.handler.getOwnPropertyDescriptor):
(i.set assert):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@197144 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/jit/JITOperations.cpp
Source/JavaScriptCore/jit/JITOperations.h
Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/JSObject.h
Source/JavaScriptCore/tests/stress/proxy-get-own-property.js

index d5ad797..69c044d 100644 (file)
@@ -1,5 +1,35 @@
 2016-02-25  Saam barati  <sbarati@apple.com>
 
+        [ES6] for...in iteration doesn't comply with the specification
+        https://bugs.webkit.org/show_bug.cgi?id=154665
+
+        Reviewed by Michael Saboff.
+
+        If you read ForIn/OfHeadEvaluation inside the spec:
+        https://tc39.github.io/ecma262/#sec-runtime-semantics-forin-div-ofheadevaluation-tdznames-expr-iterationkind
+        It calls EnumerateObjectProperties(obj) to get a set of properties
+        to enumerate over (it models this "set" as en ES6 generator function).
+        EnumerateObjectProperties is defined in section 13.7.5.15:
+        https://tc39.github.io/ecma262/#sec-enumerate-object-properties
+        The implementation calls Reflect.getOwnPropertyDescriptor(.) on the
+        properties it sees. We must do the same by modeling the operation as
+        a [[GetOwnProperty]] instead of a [[HasProperty]] internal method call.
+
+        * jit/JITOperations.cpp:
+        * jit/JITOperations.h:
+        * runtime/CommonSlowPaths.cpp:
+        (JSC::SLOW_PATH_DECL):
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::hasProperty):
+        (JSC::JSObject::hasPropertyGeneric):
+        * runtime/JSObject.h:
+        * tests/stress/proxy-get-own-property.js:
+        (assert):
+        (let.handler.getOwnPropertyDescriptor):
+        (i.set assert):
+
+2016-02-25  Saam barati  <sbarati@apple.com>
+
         [ES6] Implement Proxy.[[Set]]
         https://bugs.webkit.org/show_bug.cgi?id=154511
 
index 3a386ae..5b74565 100644 (file)
@@ -958,13 +958,6 @@ size_t JIT_OPERATION operationCompareStringEq(ExecState* exec, JSCell* left, JSC
 #endif
 }
 
-size_t JIT_OPERATION operationHasProperty(ExecState* exec, JSObject* base, JSString* property)
-{
-    int result = base->hasProperty(exec, property->toIdentifier(exec));
-    return result;
-}
-    
-
 EncodedJSValue JIT_OPERATION operationNewArrayWithProfile(ExecState* exec, ArrayAllocationProfile* profile, const JSValue* values, int size)
 {
     VM* vm = &exec->vm();
@@ -1705,7 +1698,7 @@ EncodedJSValue JIT_OPERATION operationHasIndexedPropertyDefault(ExecState* exec,
         // https://bugs.webkit.org/show_bug.cgi?id=149886
         byValInfo->arrayProfile->setOutOfBounds();
     }
-    return JSValue::encode(jsBoolean(object->hasProperty(exec, index)));
+    return JSValue::encode(jsBoolean(object->hasPropertyGeneric(exec, index, PropertySlot::InternalMethodType::GetOwnProperty)));
 }
     
 EncodedJSValue JIT_OPERATION operationHasIndexedPropertyGeneric(ExecState* exec, EncodedJSValue encodedBase, EncodedJSValue encodedSubscript, ByValInfo* byValInfo)
@@ -1729,7 +1722,7 @@ EncodedJSValue JIT_OPERATION operationHasIndexedPropertyGeneric(ExecState* exec,
         // https://bugs.webkit.org/show_bug.cgi?id=149886
         byValInfo->arrayProfile->setOutOfBounds();
     }
-    return JSValue::encode(jsBoolean(object->hasProperty(exec, subscript.asUInt32())));
+    return JSValue::encode(jsBoolean(object->hasPropertyGeneric(exec, subscript.asUInt32(), PropertySlot::InternalMethodType::GetOwnProperty)));
 }
     
 EncodedJSValue JIT_OPERATION operationGetByValString(ExecState* exec, EncodedJSValue encodedBase, EncodedJSValue encodedSubscript, ByValInfo* byValInfo)
@@ -2049,7 +2042,7 @@ EncodedJSValue JIT_OPERATION operationHasGenericProperty(ExecState* exec, Encode
         return JSValue::encode(jsBoolean(false));
 
     JSObject* base = baseValue.toObject(exec);
-    return JSValue::encode(jsBoolean(base->hasProperty(exec, asString(propertyName)->toIdentifier(exec))));
+    return JSValue::encode(jsBoolean(base->hasPropertyGeneric(exec, asString(propertyName)->toIdentifier(exec), PropertySlot::InternalMethodType::GetOwnProperty)));
 }
 
 EncodedJSValue JIT_OPERATION operationHasIndexedProperty(ExecState* exec, JSCell* baseCell, int32_t subscript)
@@ -2057,7 +2050,7 @@ EncodedJSValue JIT_OPERATION operationHasIndexedProperty(ExecState* exec, JSCell
     VM& vm = exec->vm();
     NativeCallFrameTracer tracer(&vm, exec);
     JSObject* object = baseCell->toObject(exec, exec->lexicalGlobalObject());
-    return JSValue::encode(jsBoolean(object->hasProperty(exec, subscript)));
+    return JSValue::encode(jsBoolean(object->hasPropertyGeneric(exec, subscript, PropertySlot::InternalMethodType::GetOwnProperty)));
 }
     
 JSCell* JIT_OPERATION operationGetPropertyEnumerator(ExecState* exec, JSCell* cell)
index defd5f7..81a73ea 100644 (file)
@@ -297,7 +297,6 @@ EncodedJSValue JIT_OPERATION operationCompareStringEq(ExecState*, JSCell* left,
 #else
 size_t JIT_OPERATION operationCompareStringEq(ExecState*, JSCell* left, JSCell* right) WTF_INTERNAL;
 #endif
-size_t JIT_OPERATION operationHasProperty(ExecState*, JSObject*, JSString*) WTF_INTERNAL;
 EncodedJSValue JIT_OPERATION operationNewArrayWithProfile(ExecState*, ArrayAllocationProfile*, const JSValue* values, int32_t size) WTF_INTERNAL;
 EncodedJSValue JIT_OPERATION operationNewArrayBufferWithProfile(ExecState*, ArrayAllocationProfile*, const JSValue* values, int32_t size) WTF_INTERNAL;
 EncodedJSValue JIT_OPERATION operationNewArrayWithSizeAndProfile(ExecState*, ArrayAllocationProfile*, EncodedJSValue size) WTF_INTERNAL;
index 415115f..167c287 100644 (file)
@@ -602,7 +602,7 @@ SLOW_PATH_DECL(slow_path_has_indexed_property)
     JSValue property = OP(3).jsValue();
     pc[4].u.arrayProfile->observeStructure(base->structure(vm));
     ASSERT(property.isUInt32());
-    RETURN(jsBoolean(base->hasProperty(exec, property.asUInt32())));
+    RETURN(jsBoolean(base->hasPropertyGeneric(exec, property.asUInt32(), PropertySlot::InternalMethodType::GetOwnProperty)));
 }
 
 SLOW_PATH_DECL(slow_path_has_structure_property)
@@ -614,7 +614,7 @@ SLOW_PATH_DECL(slow_path_has_structure_property)
     JSPropertyNameEnumerator* enumerator = jsCast<JSPropertyNameEnumerator*>(OP(4).jsValue().asCell());
     if (base->structure(vm)->id() == enumerator->cachedStructureID())
         RETURN(jsBoolean(true));
-    RETURN(jsBoolean(base->hasProperty(exec, asString(property.asCell())->toIdentifier(exec))));
+    RETURN(jsBoolean(base->hasPropertyGeneric(exec, asString(property.asCell())->toIdentifier(exec), PropertySlot::InternalMethodType::GetOwnProperty)));
 }
 
 SLOW_PATH_DECL(slow_path_has_generic_property)
@@ -624,10 +624,10 @@ SLOW_PATH_DECL(slow_path_has_generic_property)
     JSValue property = OP(3).jsValue();
     bool result;
     if (property.isString())
-        result = base->hasProperty(exec, asString(property.asCell())->toIdentifier(exec));
+        result = base->hasPropertyGeneric(exec, asString(property.asCell())->toIdentifier(exec), PropertySlot::InternalMethodType::GetOwnProperty);
     else {
         ASSERT(property.isUInt32());
-        result = base->hasProperty(exec, property.asUInt32());
+        result = base->hasPropertyGeneric(exec, property.asUInt32(), PropertySlot::InternalMethodType::GetOwnProperty);
     }
     RETURN(jsBoolean(result));
 }
index 15e01e5..a885921 100644 (file)
@@ -1280,13 +1280,23 @@ void JSObject::putDirectNonIndexAccessor(VM& vm, PropertyName propertyName, JSVa
 // http://www.ecma-international.org/ecma-262/6.0/index.html#sec-hasproperty
 bool JSObject::hasProperty(ExecState* exec, PropertyName propertyName) const
 {
-    PropertySlot slot(this, PropertySlot::InternalMethodType::HasProperty);
-    return const_cast<JSObject*>(this)->getPropertySlot(exec, propertyName, slot);
+    return hasPropertyGeneric(exec, propertyName, PropertySlot::InternalMethodType::HasProperty);
 }
 
 bool JSObject::hasProperty(ExecState* exec, unsigned propertyName) const
 {
-    PropertySlot slot(this, PropertySlot::InternalMethodType::HasProperty);
+    return hasPropertyGeneric(exec, propertyName, PropertySlot::InternalMethodType::HasProperty);
+}
+
+bool JSObject::hasPropertyGeneric(ExecState* exec, PropertyName propertyName, PropertySlot::InternalMethodType internalMethodType) const
+{
+    PropertySlot slot(this, internalMethodType);
+    return const_cast<JSObject*>(this)->getPropertySlot(exec, propertyName, slot);
+}
+
+bool JSObject::hasPropertyGeneric(ExecState* exec, unsigned propertyName, PropertySlot::InternalMethodType internalMethodType) const
+{
+    PropertySlot slot(this, internalMethodType);
     return const_cast<JSObject*>(this)->getPropertySlot(exec, propertyName, slot);
 }
 
index 6a23511..38ff451 100644 (file)
@@ -478,6 +478,8 @@ public:
 
     JS_EXPORT_PRIVATE bool hasProperty(ExecState*, PropertyName) const;
     JS_EXPORT_PRIVATE bool hasProperty(ExecState*, unsigned propertyName) const;
+    bool hasPropertyGeneric(ExecState*, PropertyName, PropertySlot::InternalMethodType) const;
+    bool hasPropertyGeneric(ExecState*, unsigned propertyName, PropertySlot::InternalMethodType) const;
     bool hasOwnProperty(ExecState*, PropertyName) const;
     bool hasOwnProperty(ExecState*, unsigned) const;
 
index d7ba62c..1ea447c 100644 (file)
@@ -210,3 +210,56 @@ function assert(b) {
         assert(threw);
     }
 }
+
+
+
+Object.prototype.fooBarBaz = 20; // Make for-in go over the prototype chain to the top.
+
+{
+    let target = {};
+    let called = false;
+    let handler = {
+        getOwnPropertyDescriptor: function() {
+            called = true;
+            return undefined;
+        }
+    };
+    let proxy = new Proxy(target, handler);
+    for (let i = 0; i < 1000; i++) {
+        let set = new Set();
+        for (let p in proxy) {
+            set.add(p);
+        }
+        assert(set.has("fooBarBaz"));
+        assert(called);
+        called = false;
+    }
+}
+
+{
+    let target = {};
+    let called = false;
+    let handler = {
+        getOwnPropertyDescriptor: function() {
+            called = true;
+            return undefined;
+        }
+    };
+
+    let proxy = new Proxy(target, handler);
+    let proxyish = Object.create(proxy, {
+        x: {value: 20, enumerable: true},
+        y: {value: 20, enumerable: true}
+    });
+    for (let i = 0; i < 1000; i++) {
+        let set = new Set;
+        for (let p in proxyish) {
+            set.add(p);
+        }
+        assert(set.has("x"));
+        assert(set.has("y"));
+        assert(set.has("fooBarBaz"));
+        assert(called);
+        called = false;
+    }
+}