Skip to content

Commit a223de1

Browse files
woessgilles-duboscq
woess
authored and
gilles-duboscq
committed
Implement [[DefineOwnProperty]] for existing properties in dictionary objects.
1 parent 9dff105 commit a223de1

File tree

3 files changed

+120
-19
lines changed

3 files changed

+120
-19
lines changed

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/builtins/JSDictionary.java‎

Lines changed: 94 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@
4040
*/
4141
package com.oracle.truffle.js.runtime.builtins;
4242

43+
import static com.oracle.truffle.js.runtime.util.DefinePropertyUtil.nonConfigurableMessage;
44+
import static com.oracle.truffle.js.runtime.util.DefinePropertyUtil.nonWritableMessage;
45+
import static com.oracle.truffle.js.runtime.util.DefinePropertyUtil.notExtensibleMessage;
46+
import static com.oracle.truffle.js.runtime.util.DefinePropertyUtil.reject;
47+
4348
import java.util.AbstractMap;
4449
import java.util.ArrayList;
4550
import java.util.Collections;
@@ -260,15 +265,27 @@ public PropertyDescriptor getOwnProperty(JSDynamicObject thisObj, Object key) {
260265
@Override
261266
public boolean defineOwnProperty(JSDynamicObject thisObj, Object key, PropertyDescriptor desc, boolean doThrow) {
262267
assert JSRuntime.isPropertyKey(key);
263-
if (!hasOwnProperty(thisObj, key) && JSObject.isExtensible(thisObj)) {
264-
getHashMap(thisObj).put(key, makeFullyPopulatedPropertyDescriptor(desc));
265-
return true;
268+
PropertyDescriptor current = getHashMap(thisObj).get(key);
269+
if (current == null) {
270+
current = super.getOwnProperty(thisObj, key);
271+
boolean extensible = JSObject.isExtensible(thisObj);
272+
if (current == null) {
273+
if (!extensible) {
274+
return reject(doThrow, notExtensibleMessage(key, doThrow));
275+
}
276+
validateAndPutDesc(thisObj, key, makeFullyPopulatedPropertyDescriptor(desc));
277+
return true;
278+
} else {
279+
return DefinePropertyUtil.validateAndApplyPropertyDescriptor(thisObj, key, extensible, desc, current, doThrow);
280+
}
281+
} else {
282+
return validateAndApplyPropertyDescriptorExisting(thisObj, key, desc, current, doThrow);
266283
}
284+
}
267285

268-
// defineProperty is currently not supported on dictionary objects,
269-
// so we need to convert back to a normal shape-based object.
270-
makeOrdinaryObject(thisObj, "defineOwnProperty");
271-
return super.defineOwnProperty(thisObj, key, desc, doThrow);
286+
private static PropertyDescriptor validateAndPutDesc(JSDynamicObject thisObj, Object key, PropertyDescriptor newDesc) {
287+
assert newDesc.isFullyPopulatedPropertyDescriptor();
288+
return getHashMap(thisObj).put(key, newDesc);
272289
}
273290

274291
/**
@@ -296,6 +313,76 @@ private static PropertyDescriptor makeFullyPopulatedPropertyDescriptor(PropertyD
296313
}
297314
}
298315

316+
private static boolean validateAndApplyPropertyDescriptorExisting(JSDynamicObject thisObj, Object key, PropertyDescriptor descriptor, PropertyDescriptor currentDesc, boolean doThrow) {
317+
CompilerAsserts.neverPartOfCompilation();
318+
assert currentDesc.isFullyPopulatedPropertyDescriptor();
319+
if (descriptor.hasNoFields()) {
320+
return true;
321+
}
322+
323+
if (!currentDesc.getConfigurable()) {
324+
if ((descriptor.hasConfigurable() && descriptor.getConfigurable()) ||
325+
(descriptor.hasEnumerable() && (descriptor.getEnumerable() != currentDesc.getEnumerable()))) {
326+
return reject(doThrow, nonConfigurableMessage(key, doThrow));
327+
}
328+
if (!descriptor.isGenericDescriptor() && descriptor.isAccessorDescriptor() != currentDesc.isAccessorDescriptor()) {
329+
return reject(doThrow, nonConfigurableMessage(key, doThrow));
330+
}
331+
if (currentDesc.isAccessorDescriptor()) {
332+
if ((descriptor.hasGet() && !JSRuntime.isSameValue(descriptor.getGet(), currentDesc.getGet())) ||
333+
(descriptor.hasSet() && !JSRuntime.isSameValue(descriptor.getSet(), currentDesc.getSet()))) {
334+
return reject(doThrow, nonConfigurableMessage(key, doThrow));
335+
}
336+
} else {
337+
assert currentDesc.isDataDescriptor();
338+
if (!currentDesc.getWritable()) {
339+
if (descriptor.hasWritable() && descriptor.getWritable()) {
340+
return reject(doThrow, nonConfigurableMessage(key, doThrow));
341+
}
342+
if (descriptor.hasValue() && !JSRuntime.isSameValue(descriptor.getValue(), currentDesc.getValue())) {
343+
return reject(doThrow, nonWritableMessage(key, doThrow));
344+
}
345+
}
346+
}
347+
}
348+
349+
if (currentDesc.isDataDescriptor() && descriptor.isAccessorDescriptor()) {
350+
PropertyDescriptor newDesc = PropertyDescriptor.createAccessor(descriptor.getGet(), descriptor.getSet(),
351+
descriptor.getIfHasEnumerable(currentDesc.getEnumerable()),
352+
descriptor.getIfHasConfigurable(currentDesc.getConfigurable()));
353+
validateAndPutDesc(thisObj, key, newDesc);
354+
return true;
355+
} else if (currentDesc.isAccessorDescriptor() && descriptor.isDataDescriptor()) {
356+
Object value = descriptor.hasValue() ? descriptor.getValue() : Undefined.instance;
357+
PropertyDescriptor newDesc = PropertyDescriptor.createData(value,
358+
descriptor.getIfHasEnumerable(currentDesc.getEnumerable()),
359+
descriptor.getIfHasConfigurable(currentDesc.getConfigurable()),
360+
descriptor.getWritable());
361+
validateAndPutDesc(thisObj, key, newDesc);
362+
return true;
363+
} else {
364+
if (descriptor.hasConfigurable()) {
365+
currentDesc.setConfigurable(descriptor.getConfigurable());
366+
}
367+
if (descriptor.hasEnumerable()) {
368+
currentDesc.setEnumerable(descriptor.getEnumerable());
369+
}
370+
if (descriptor.hasWritable()) {
371+
currentDesc.setWritable(descriptor.getWritable());
372+
}
373+
if (descriptor.hasValue()) {
374+
currentDesc.setValue(descriptor.getValue());
375+
}
376+
if (descriptor.hasGet()) {
377+
currentDesc.setGet(descriptor.getGet());
378+
}
379+
if (descriptor.hasSet()) {
380+
currentDesc.setSet(descriptor.getSet());
381+
}
382+
return true;
383+
}
384+
}
385+
299386
@SuppressWarnings("unchecked")
300387
static EconomicMap<Object, PropertyDescriptor> getHashMap(JSDynamicObject obj) {
301388
assert JSDictionary.isJSDictionaryObject(obj);

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/objects/PropertyDescriptor.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,20 @@ public boolean isGenericDescriptor() {
308308
return !(isAccessorDescriptor() || isDataDescriptor());
309309
}
310310

311+
/**
312+
* Returns true if this property descriptor does not have any fields.
313+
*/
314+
public boolean hasNoFields() {
315+
return !hasValue() && !hasGet() && !hasSet() && !hasConfigurable() && !hasEnumerable() && !hasWritable();
316+
}
317+
318+
/**
319+
* Returns true if this is a fully populated data or accessor property descriptor.
320+
*/
321+
public boolean isFullyPopulatedPropertyDescriptor() {
322+
return hasConfigurable() && hasEnumerable() && ((hasValue() && hasWritable()) || (hasGet() && hasSet()));
323+
}
324+
311325
public int getFlags() {
312326
return JSAttributes.fromConfigurableEnumerableWritable(getIfHasConfigurable(false), getIfHasEnumerable(false), getIfHasWritable(false));
313327
}

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/util/DefinePropertyUtil.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,12 @@ public static boolean isCompatiblePropertyDescriptor(boolean extensible, Propert
9191
/**
9292
* Implementation of ValidateAndApplyPropertyDescriptor as defined in ECMAScript 2015, 9.1.6.3.
9393
*/
94-
private static boolean validateAndApplyPropertyDescriptor(JSDynamicObject thisObj, Object propertyKey, boolean extensible, PropertyDescriptor descriptor, PropertyDescriptor current,
94+
public static boolean validateAndApplyPropertyDescriptor(JSDynamicObject thisObj, Object propertyKey, boolean extensible, PropertyDescriptor descriptor, PropertyDescriptor current,
9595
boolean doThrow) {
9696
CompilerAsserts.neverPartOfCompilation();
9797
if (current == null) {
9898
if (!extensible) {
99-
return reject(doThrow, "object is not extensible");
99+
return reject(doThrow, notExtensibleMessage(propertyKey, doThrow));
100100
}
101101
if (thisObj == Undefined.instance) {
102102
return true;
@@ -123,7 +123,7 @@ private static boolean definePropertyExisting(JSDynamicObject thisObj, Object ke
123123
boolean currentWritable = currentDesc.getWritable();
124124

125125
// 5. Return true, if every field in Desc is absent.
126-
if (everyFieldAbsent(descriptor)) {
126+
if (descriptor.hasNoFields()) {
127127
return true;
128128
}
129129

@@ -276,13 +276,6 @@ private static boolean definePropertyExisting(JSDynamicObject thisObj, Object ke
276276
}
277277
}
278278

279-
/**
280-
* Implements "return true, if every field in Desc is absent, as defined by 8.12.9 step 5.
281-
*/
282-
private static boolean everyFieldAbsent(PropertyDescriptor descriptor) {
283-
return !descriptor.hasValue() && !descriptor.hasGet() && !descriptor.hasSet() && !descriptor.hasConfigurable() && !descriptor.hasEnumerable() && !descriptor.hasWritable();
284-
}
285-
286279
/**
287280
* Implementing 8.12.9 [[DefineOwnProperty]], section "4" (a new property is defined).
288281
*
@@ -350,14 +343,21 @@ public static boolean reject(boolean doThrow, String message) {
350343
return false;
351344
}
352345

353-
private static String nonConfigurableMessage(Object key, boolean reject) {
346+
public static String notExtensibleMessage(Object key, boolean reject) {
347+
if (reject) {
348+
return "Cannot define property " + key + ", object is not extensible";
349+
}
350+
return "";
351+
}
352+
353+
public static String nonConfigurableMessage(Object key, boolean reject) {
354354
if (reject) {
355355
return isNashornMode() ? "property is not configurable" : cannotRedefineMessage(key);
356356
}
357357
return "";
358358
}
359359

360-
private static String nonWritableMessage(Object key, boolean reject) {
360+
public static String nonWritableMessage(Object key, boolean reject) {
361361
if (reject) {
362362
return isNashornMode() ? "property is not writable" : cannotRedefineMessage(key);
363363
}

0 commit comments

Comments
 (0)