diff --git a/include/hermes/VM/JSObject.h b/include/hermes/VM/JSObject.h index bf619ccf51a..a2ff888ccea 100644 --- a/include/hermes/VM/JSObject.h +++ b/include/hermes/VM/JSObject.h @@ -599,6 +599,18 @@ class JSObject : public GCCell { !onlyEnumerable)); } + /// Same as getOwnPropertyNames, however, *only* include non-enumerable + /// properties, and skip enumerable properties. + /// \returns a JSArray containing the names. + static CallResult<Handle<JSArray>> getOwnNonEnumerablePropertyNames( + Handle<JSObject> selfHandle, + Runtime &runtime) { + return getOwnPropertyKeys( + selfHandle, + runtime, + OwnKeys::Flags().plusIncludeNonSymbols().plusIncludeNonEnumerable()); + } + /// Return a list of property symbols keys belonging to this object. /// The order of properties follows ES2015 - insertion order. /// \returns a JSArray containing the symbols. diff --git a/lib/VM/JSObject.cpp b/lib/VM/JSObject.cpp index 8678a95289c..c61a556984b 100644 --- a/lib/VM/JSObject.cpp +++ b/lib/VM/JSObject.cpp @@ -3042,6 +3042,15 @@ CallResult<uint32_t> appendAllPropertyNames( // strings (SymbolIDs) or index names. llvh::SmallSet<SymbolID::RawType, 4> dedupNames; llvh::SmallSet<double, 4> dedupIdxNames; + // For the SymbolIDs we collect for the non enumerable properties, they are + // not stored in the resulting BigStorage. Therefore it's not safe to store + // the raw types in the SmallSet. We put them in this ArrayStorage to make the + // gc aware we are still holding reference to them. + CallResult<HermesValue> keepaliveRes = ArrayStorage::create(runtime, 0, 0); + // We can use keepaliveRes without checking its ExecutionResult since it will + // never fail. + MutableHandle<ArrayStorage> keepalive = + runtime.makeMutableHandle(vmcast<ArrayStorage>(*keepaliveRes)); // Add the current value of prop and/or propIdHandle to correct set(s). auto addToDedup = [&dedupIdxNames, &dedupNames, &runtime]( Handle<> prop, Handle<SymbolID> propIdHandle) { @@ -3133,6 +3142,44 @@ CallResult<uint32_t> appendAllPropertyNames( ++size; } } + // Now we must handle non-enumerable property shadowing. Get all the + // non-enumerable properties in this object, and add them to the sets we are + // maintaining. Therefore, if a parent object later on has a property with + // the same name as an non-enumerable property defined in the child object, + // the child object will shadow the parent's enumerable property. + // We don't support property shadowing with proxy objects. + if (LLVM_LIKELY(!head->isProxyObject())) { + auto nonEnumerablePropsRes = + JSObject::getOwnNonEnumerablePropertyNames(head, runtime); + if (LLVM_UNLIKELY(cr == ExecutionStatus::EXCEPTION)) { + return ExecutionStatus::EXCEPTION; + } + auto nonEnumerableProps = *nonEnumerablePropsRes; + auto flushTo = gcScope.createMarker(); + if (ArrayStorage::ensureCapacity( + keepalive, + runtime, + keepalive->size() + nonEnumerableProps->getEndIndex()) == + ExecutionStatus::EXCEPTION) { + return ExecutionStatus::EXCEPTION; + } + for (unsigned i = 0, e = nonEnumerableProps->getEndIndex(); i < e; ++i) { + gcScope.flushToMarker(flushTo); + prop = nonEnumerableProps->at(runtime, i).unboxToHV(runtime); + if (prop->isString()) { + CallResult<Handle<SymbolID>> symRes = + runtime.getIdentifierTable().getSymbolHandleFromPrimitive( + runtime, + runtime.makeHandle<StringPrimitive>(prop->getString())); + if (LLVM_UNLIKELY(symRes == ExecutionStatus::EXCEPTION)) { + return ExecutionStatus::EXCEPTION; + } + propIdHandle = *symRes; + ArrayStorage::push_back(keepalive, runtime, propIdHandle); + } + addToDedup(prop, propIdHandle); + } + } // Continue to follow the prototype chain. CallResult<PseudoHandle<JSObject>> parentRes = JSObject::getPrototypeOf(head, runtime); diff --git a/test/hermes/for_in.js b/test/hermes/for_in.js index 9ee23435213..d9656963400 100644 --- a/test/hermes/for_in.js +++ b/test/hermes/for_in.js @@ -127,3 +127,23 @@ for (var prop in child) { //CHECK: dup //CHECK: b //CHECK: a + +// Check that non-enumerable child properties shadow parent +// enumerable properties of the same name. +var parent = {hide: "value", propA: 5}; +var child = Object.create(parent); +Object.defineProperty(child, 'hide', {enumerable: false}); +child.propB = 5; +for (var prop in child) { + print(prop); +} +//CHECK: propB +//CHECK: propA +var grandchild = Object.create(child); +grandchild.propC = 10; +for (var prop in grandchild) { + print(prop); +} +//CHECK: propC +//CHECK: propB +//CHECK: propA