Skip to content

Commit 3dd4035

Browse files
aardvark179aardvark179
authored
Refactor internal property descriptors to not be ScriptableObjects
* Move from `ScriptableObject`s to `DescriptorInfo` for descriptors. * Update test 262 properties. * Better API for descriptor fields. This helps prepare the way for more refactoring by separating these descriptors, which in JavaScript are not truly objects, but merely internal records.
1 parent f7341b4 commit 3dd4035

20 files changed

Lines changed: 396 additions & 359 deletions

‎rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java‎

Lines changed: 37 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import java.util.Map;
88
import java.util.Objects;
99
import java.util.function.Predicate;
10+
import org.mozilla.javascript.ScriptableObject.DescriptorInfo;
1011

1112
/**
1213
* Abstract Object Operations as defined by EcmaScript
@@ -83,12 +84,11 @@ static boolean testIntegrityLevel(Context cx, Object o, INTEGRITY_LEVEL level) {
8384
ids = obj.getIds(map, true, true);
8485
}
8586
for (Object name : ids) {
86-
ScriptableObject desc = obj.getOwnPropertyDescriptor(cx, name);
87-
if (Boolean.TRUE.equals(desc.get("configurable"))) return false;
87+
DescriptorInfo desc = obj.getOwnPropertyDescriptor(cx, name);
88+
if (desc.isConfigurable()) return false;
8889

89-
if (level == INTEGRITY_LEVEL.FROZEN
90-
&& ScriptableObject.isDataDescriptor(desc)
91-
&& Boolean.TRUE.equals(desc.get("writable"))) return false;
90+
if (level == INTEGRITY_LEVEL.FROZEN && desc.isDataDescriptor() && desc.isWritable())
91+
return false;
9292
}
9393

9494
return true;
@@ -150,21 +150,20 @@ static boolean setIntegrityLevel(Context cx, Object o, INTEGRITY_LEVEL level) {
150150
ids = obj.getIds(map, true, true);
151151
}
152152
for (Object key : ids) {
153-
ScriptableObject desc = obj.getOwnPropertyDescriptor(cx, key);
153+
DescriptorInfo desc = obj.getOwnPropertyDescriptor(cx, key);
154154

155155
if (level == INTEGRITY_LEVEL.SEALED) {
156-
if (Boolean.TRUE.equals(desc.get("configurable"))) {
157-
desc.put("configurable", desc, Boolean.FALSE);
156+
if (desc.isConfigurable()) {
157+
desc.configurable = false;
158158

159159
obj.defineOwnProperty(cx, key, desc, false);
160160
}
161161
} else {
162-
if (ScriptableObject.isDataDescriptor(desc)
163-
&& Boolean.TRUE.equals(desc.get("writable"))) {
164-
desc.put("writable", desc, Boolean.FALSE);
162+
if (desc.isDataDescriptor() && desc.isWritable()) {
163+
desc.writable = false;
165164
}
166-
if (Boolean.TRUE.equals(desc.get("configurable"))) {
167-
desc.put("configurable", desc, Boolean.FALSE);
165+
if (desc.isConfigurable()) {
166+
desc.configurable = false;
168167
}
169168
obj.defineOwnProperty(cx, key, desc, false);
170169
}
@@ -402,7 +401,7 @@ public static long lengthOfArrayLike(Context cx, Scriptable o) {
402401
* IsCompatiblePropertyDescriptor (Extensible, Desc, Current)</a>
403402
*/
404403
static boolean isCompatiblePropertyDescriptor(
405-
Context cx, boolean extensible, ScriptableObject desc, ScriptableObject current) {
404+
Context cx, boolean extensible, DescriptorInfo desc, DescriptorInfo current) {
406405
return validateAndApplyPropertyDescriptor(
407406
cx,
408407
Undefined.SCRIPTABLE_UNDEFINED,
@@ -424,15 +423,14 @@ static boolean validateAndApplyPropertyDescriptor(
424423
Scriptable o,
425424
Scriptable p,
426425
boolean extensible,
427-
ScriptableObject desc,
428-
ScriptableObject current) {
426+
DescriptorInfo desc,
427+
DescriptorInfo current) {
429428
if (Undefined.isUndefined(current)) {
430429
if (!extensible) {
431430
return false;
432431
}
433432

434-
if (ScriptableObject.isGenericDescriptor(desc)
435-
|| ScriptableObject.isDataDescriptor(desc)) {
433+
if (desc.isGenericDescriptor() || desc.isDataDescriptor()) {
436434
/*
437435
i. i. If O is not undefined, create an own data property named P of object O whose [[Value]], [[Writable]], [[Enumerable]], and [[Configurable]] attribute values are described by Desc.
438436
If the value of an attribute field of Desc is absent, the attribute of the newly created property is set to its default value.
@@ -445,32 +443,35 @@ static boolean validateAndApplyPropertyDescriptor(
445443
return true;
446444
}
447445

448-
if (desc.getIds().length == 0) {
446+
if (!desc.hasEnumerable()
447+
&& !desc.hasConfigurable()
448+
&& !desc.hasWritable()
449+
&& !desc.hasGetter()
450+
&& !desc.hasSetter()
451+
&& !desc.hasValue()) {
449452
return true;
450453
}
451454

452-
if (Boolean.FALSE.equals(current.get("configurable"))) {
453-
if (Boolean.TRUE.equals(ScriptableObject.hasProperty(desc, "configurable"))
454-
&& Boolean.TRUE.equals(desc.get("configurable"))) {
455+
if (current.isConfigurable(false)) {
456+
if (desc.isConfigurable()) {
455457
return false;
456458
}
457459

458-
if (Boolean.TRUE.equals(ScriptableObject.hasProperty(desc, "enumerable"))
459-
&& !Objects.equals(desc.get("enumerable"), current.get("enumerable"))) {
460+
if (desc.hasEnumerable() && !Objects.equals(desc.enumerable, current.enumerable)) {
460461
return false;
461462
}
462463
}
463464

464-
if (ScriptableObject.isGenericDescriptor(desc)) {
465+
if (desc.isGenericDescriptor()) {
465466
return true;
466467
}
467468

468-
if (ScriptableObject.isDataDescriptor(current) != ScriptableObject.isDataDescriptor(desc)) {
469-
if (Boolean.FALSE.equals(current.get("configurable"))) {
469+
if (current.isDataDescriptor() != desc.isDataDescriptor()) {
470+
if (current.isConfigurable(false)) {
470471
return false;
471472
}
472-
if (ScriptableObject.isDataDescriptor(current)) {
473-
if (Boolean.FALSE.equals(current.get("configurable"))) {
473+
if (current.isDataDescriptor()) {
474+
if (current.isConfigurable(false)) {
474475
// i. i. If O is not undefined, convert the property named P of object O from a
475476
// data property to an accessor property. Preserve the existing values of the
476477
// converted property's [[Configurable]] and [[Enumerable]] attributes and set
@@ -482,28 +483,22 @@ static boolean validateAndApplyPropertyDescriptor(
482483
// the rest of the property's attributes to their default values.
483484
}
484485
}
485-
} else if (ScriptableObject.isDataDescriptor(current)
486-
&& ScriptableObject.isDataDescriptor(desc)) {
487-
if (Boolean.FALSE.equals(current.get("configurable"))
488-
&& Boolean.FALSE.equals(current.get("writable"))) {
489-
if (Boolean.TRUE.equals(ScriptableObject.hasProperty(desc, "writable"))
490-
&& Boolean.TRUE.equals(desc.get("writable"))) {
486+
} else if (current.isDataDescriptor() && desc.isDataDescriptor()) {
487+
if (current.isConfigurable(false) && current.isWritable(false)) {
488+
if (desc.isWritable()) {
491489
return false;
492490
}
493-
if (Boolean.TRUE.equals(ScriptableObject.hasProperty(desc, "value"))
494-
&& !Objects.equals(desc.get("value"), current.get("value"))) {
491+
if (desc.hasValue() && !Objects.equals(desc.value, current.value)) {
495492
return false;
496493
}
497494
return true;
498495
}
499496
} else {
500-
if (Boolean.FALSE.equals(current.get("configurable"))) {
501-
if (Boolean.TRUE.equals(ScriptableObject.hasProperty(desc, "set"))
502-
&& !Objects.equals(desc.get("set"), current.get("set"))) {
497+
if (current.isConfigurable(false)) {
498+
if (desc.hasSetter() && !Objects.equals(desc.setter, current.setter)) {
503499
return false;
504500
}
505-
if (Boolean.TRUE.equals(ScriptableObject.hasProperty(desc, "get"))
506-
&& !Objects.equals(desc.get("get"), current.get("get"))) {
501+
if (desc.hasGetter() && !Objects.equals(desc.getter, current.getter)) {
507502
return false;
508503
}
509504
return true;

‎rhino/src/main/java/org/mozilla/javascript/AccessorSlot.java‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package org.mozilla.javascript;
22

3+
import org.mozilla.javascript.ScriptableObject.DescriptorInfo;
4+
35
/**
46
* This is a specialization of Slot to store various types of values that are retrieved dynamically
57
* using Java and JavaScript functions. Unlike LambdaSlot, the fact that these values are accessed
@@ -43,45 +45,40 @@ boolean isSetterSlot() {
4345
}
4446

4547
@Override
46-
ScriptableObject getPropertyDescriptor(Context cx, Scriptable scope) {
48+
DescriptorInfo getPropertyDescriptor(Context cx, Scriptable scope) {
4749
// It sounds logical that this would be the same as the logic for a normal Slot,
4850
// but the spec is super pedantic about things like the order of properties here,
4951
// so we need special support here.
5052

51-
ScriptableObject desc = (ScriptableObject) cx.newObject(scope);
5253
int attr = getAttributes();
53-
54+
DescriptorInfo desc;
5455
boolean es6 = cx.getLanguageVersion() >= Context.VERSION_ES6;
5556
if (es6) {
57+
desc = new DescriptorInfo(ScriptableObject.NOT_FOUND, attr, false);
5658
if (getter == null && setter == null) {
57-
desc.defineProperty(
58-
"writable",
59-
(attr & ScriptableObject.READONLY) == 0,
60-
ScriptableObject.EMPTY);
59+
desc.writable = (attr & ScriptableObject.READONLY) == 0;
6160
}
6261
} else {
63-
desc.setCommonDescriptorProperties(attr, getter == null && setter == null);
62+
desc =
63+
new DescriptorInfo(
64+
ScriptableObject.NOT_FOUND, attr, getter == null && setter == null);
6465
}
6566

6667
String fName = name == null ? "f" : name.toString();
6768
if (getter != null) {
6869
Function f = getter.asGetterFunction(fName, scope);
69-
desc.defineProperty("get", f == null ? Undefined.instance : f, ScriptableObject.EMPTY);
70+
desc.getter = f == null ? Undefined.instance : f;
7071
}
7172
if (setter != null) {
7273
Function f = setter.asSetterFunction(fName, scope);
73-
desc.defineProperty("set", f == null ? Undefined.instance : f, ScriptableObject.EMPTY);
74+
desc.setter = f == null ? Undefined.instance : f;
7475
} else if (es6) {
75-
desc.defineProperty("set", Undefined.instance, ScriptableObject.EMPTY);
76+
desc.setter = Undefined.instance;
7677
}
7778

7879
if (es6) {
79-
desc.defineProperty(
80-
"enumerable", (attr & ScriptableObject.DONTENUM) == 0, ScriptableObject.EMPTY);
81-
desc.defineProperty(
82-
"configurable",
83-
(attr & ScriptableObject.PERMANENT) == 0,
84-
ScriptableObject.EMPTY);
80+
desc.enumerable = (attr & ScriptableObject.DONTENUM) == 0;
81+
desc.configurable = (attr & ScriptableObject.PERMANENT) == 0;
8582
}
8683

8784
return desc;

‎rhino/src/main/java/org/mozilla/javascript/Arguments.java‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ Object[] getIds(CompoundOperationMap map, boolean getNonEnumerable, boolean getS
254254
}
255255

256256
@Override
257-
protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) {
257+
protected DescriptorInfo getOwnPropertyDescriptor(Context cx, Object id) {
258258
if (ScriptRuntime.isSymbol(id) || id instanceof Scriptable) {
259259
return super.getOwnPropertyDescriptor(cx, id);
260260
}
@@ -272,18 +272,16 @@ protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) {
272272
value = getFromActivation(index);
273273
}
274274
if (super.has(index, this)) { // the descriptor has been redefined
275-
ScriptableObject desc = super.getOwnPropertyDescriptor(cx, id);
276-
desc.put("value", desc, value);
275+
DescriptorInfo desc = super.getOwnPropertyDescriptor(cx, id);
276+
desc.value = value;
277277
return desc;
278278
}
279-
Scriptable scope = getParentScope();
280-
if (scope == null) scope = this;
281-
return buildDataDescriptor(scope, value, EMPTY);
279+
return buildDataDescriptor(value, EMPTY);
282280
}
283281

284282
@Override
285283
protected boolean defineOwnProperty(
286-
Context cx, Object id, ScriptableObject desc, boolean checkValid) {
284+
Context cx, Object id, DescriptorInfo desc, boolean checkValid) {
287285
super.defineOwnProperty(cx, id, desc, checkValid);
288286
if (ScriptRuntime.isSymbol(id)) {
289287
return true;
@@ -296,17 +294,17 @@ protected boolean defineOwnProperty(
296294
Object value = arg(index);
297295
if (value == NOT_FOUND) return true;
298296

299-
if (isAccessorDescriptor(desc)) {
297+
if (desc.isAccessorDescriptor()) {
300298
removeArg(index);
301299
return true;
302300
}
303301

304-
Object newValue = getProperty(desc, "value");
302+
Object newValue = desc.value;
305303
if (newValue == NOT_FOUND) return true;
306304

307305
replaceArg(index, newValue);
308306

309-
if (isFalse(getProperty(desc, "writable"))) {
307+
if (isFalse(desc.writable)) {
310308
removeArg(index);
311309
}
312310
return true;

‎rhino/src/main/java/org/mozilla/javascript/ArrayLikeAbstractOperations.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import java.io.Serializable;
88
import java.util.Comparator;
9+
import org.mozilla.javascript.ScriptableObject.DescriptorInfo;
910

1011
/** Contains implementation of shared methods useful for arrays and typed arrays. */
1112
public class ArrayLikeAbstractOperations {
@@ -275,11 +276,7 @@ static void defineElem(Context cx, Scriptable target, long index, Object value)
275276
if (!(target instanceof NativeArray && ((NativeArray) target).getDenseOnly())
276277
&& target instanceof ScriptableObject) {
277278
var so = (ScriptableObject) target;
278-
ScriptableObject desc = new NativeObject();
279-
desc.defineProperty("value", value, 0);
280-
desc.defineProperty("writable", Boolean.TRUE, 0);
281-
desc.defineProperty("enumerable", Boolean.TRUE, 0);
282-
desc.defineProperty("configurable", Boolean.TRUE, 0);
279+
var desc = new DescriptorInfo(true, true, true, value);
283280
so.defineOwnProperty(cx, index, desc);
284281
return;
285282
}

‎rhino/src/main/java/org/mozilla/javascript/BoundFunction.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,7 @@ public BoundFunction(
3838
ScriptRuntime.setFunctionProtoAndParent(this, cx, scope, false);
3939

4040
Function thrower = ScriptRuntime.typeErrorThrower(cx);
41-
NativeObject throwing = new NativeObject();
42-
ScriptRuntime.setBuiltinProtoAndParent(throwing, scope, TopLevel.Builtins.Object);
43-
throwing.put("get", throwing, thrower);
44-
throwing.put("set", throwing, thrower);
45-
throwing.put("enumerable", throwing, Boolean.FALSE);
46-
throwing.put("configurable", throwing, Boolean.FALSE);
47-
throwing.preventExtensions();
41+
var throwing = new DescriptorInfo(false, NOT_FOUND, false, thrower, thrower, NOT_FOUND);
4842

4943
this.defineOwnProperty(cx, "caller", throwing, false);
5044
this.defineOwnProperty(cx, "arguments", throwing, false);

‎rhino/src/main/java/org/mozilla/javascript/BuiltInSlot.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,8 @@ void setAttributes(int value) {
166166

167167
@Override
168168
@SuppressWarnings("unchecked")
169-
ScriptableObject getPropertyDescriptor(Context cx, Scriptable scope) {
170-
return ScriptableObject.buildDataDescriptor(
171-
scope, getValue((T) this.value), getAttributes());
169+
DescriptorInfo getPropertyDescriptor(Context cx, Scriptable scope) {
170+
return ScriptableObject.buildDataDescriptor(getValue((T) this.value), getAttributes());
172171
}
173172

174173
@SuppressWarnings("unchecked")

0 commit comments

Comments
 (0)