Skip to content

Refactor internal property descriptors to not be ScriptableObjects. #2143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+396 −359
Open

Refactor internal property descriptors to not be ScriptableObjects. #2143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dab5e40
Move from `ScriptableObject`s to `DescriptorInfo` for descriptors.
aardvark179 Sep 28, 2025
a71a3c2
Update test 262 properties.
aardvark179 Sep 30, 2025
9a7546f
Better API for descriptor fields.
aardvark179 Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 37 additions & 42 deletions rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.function.Predicate;
import org.mozilla.javascript.ScriptableObject.DescriptorInfo;

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

if (level == INTEGRITY_LEVEL.FROZEN
&& ScriptableObject.isDataDescriptor(desc)
&& Boolean.TRUE.equals(desc.get("writable"))) return false;
if (level == INTEGRITY_LEVEL.FROZEN && desc.isDataDescriptor() && desc.isWritable())
return false;
}

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

if (level == INTEGRITY_LEVEL.SEALED) {
if (Boolean.TRUE.equals(desc.get("configurable"))) {
desc.put("configurable", desc, Boolean.FALSE);
if (desc.isConfigurable()) {
desc.configurable = false;

obj.defineOwnProperty(cx, key, desc, false);
}
} else {
if (ScriptableObject.isDataDescriptor(desc)
&& Boolean.TRUE.equals(desc.get("writable"))) {
desc.put("writable", desc, Boolean.FALSE);
if (desc.isDataDescriptor() && desc.isWritable()) {
desc.writable = false;
}
if (Boolean.TRUE.equals(desc.get("configurable"))) {
desc.put("configurable", desc, Boolean.FALSE);
if (desc.isConfigurable()) {
desc.configurable = false;
}
obj.defineOwnProperty(cx, key, desc, false);
}
Expand Down Expand Up @@ -402,7 +401,7 @@ public static long lengthOfArrayLike(Context cx, Scriptable o) {
* IsCompatiblePropertyDescriptor (Extensible, Desc, Current)</a>
*/
static boolean isCompatiblePropertyDescriptor(
Context cx, boolean extensible, ScriptableObject desc, ScriptableObject current) {
Context cx, boolean extensible, DescriptorInfo desc, DescriptorInfo current) {
return validateAndApplyPropertyDescriptor(
cx,
Undefined.SCRIPTABLE_UNDEFINED,
Expand All @@ -424,15 +423,14 @@ static boolean validateAndApplyPropertyDescriptor(
Scriptable o,
Scriptable p,
boolean extensible,
ScriptableObject desc,
ScriptableObject current) {
DescriptorInfo desc,
DescriptorInfo current) {
if (Undefined.isUndefined(current)) {
if (!extensible) {
return false;
}

if (ScriptableObject.isGenericDescriptor(desc)
|| ScriptableObject.isDataDescriptor(desc)) {
if (desc.isGenericDescriptor() || desc.isDataDescriptor()) {
/*
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.
If the value of an attribute field of Desc is absent, the attribute of the newly created property is set to its default value.
Expand All @@ -445,32 +443,35 @@ static boolean validateAndApplyPropertyDescriptor(
return true;
}

if (desc.getIds().length == 0) {
if (!desc.hasEnumerable()
&& !desc.hasConfigurable()
&& !desc.hasWritable()
&& !desc.hasGetter()
&& !desc.hasSetter()
&& !desc.hasValue()) {
return true;
}

if (Boolean.FALSE.equals(current.get("configurable"))) {
if (Boolean.TRUE.equals(ScriptableObject.hasProperty(desc, "configurable"))
&& Boolean.TRUE.equals(desc.get("configurable"))) {
if (current.isConfigurable(false)) {
if (desc.isConfigurable()) {
return false;
}

if (Boolean.TRUE.equals(ScriptableObject.hasProperty(desc, "enumerable"))
&& !Objects.equals(desc.get("enumerable"), current.get("enumerable"))) {
if (desc.hasEnumerable() && !Objects.equals(desc.enumerable, current.enumerable)) {
return false;
}
}

if (ScriptableObject.isGenericDescriptor(desc)) {
if (desc.isGenericDescriptor()) {
return true;
}

if (ScriptableObject.isDataDescriptor(current) != ScriptableObject.isDataDescriptor(desc)) {
if (Boolean.FALSE.equals(current.get("configurable"))) {
if (current.isDataDescriptor() != desc.isDataDescriptor()) {
if (current.isConfigurable(false)) {
return false;
}
if (ScriptableObject.isDataDescriptor(current)) {
if (Boolean.FALSE.equals(current.get("configurable"))) {
if (current.isDataDescriptor()) {
if (current.isConfigurable(false)) {
// i. i. If O is not undefined, convert the property named P of object O from a
// data property to an accessor property. Preserve the existing values of the
// converted property's [[Configurable]] and [[Enumerable]] attributes and set
Expand All @@ -482,28 +483,22 @@ static boolean validateAndApplyPropertyDescriptor(
// the rest of the property's attributes to their default values.
}
}
} else if (ScriptableObject.isDataDescriptor(current)
&& ScriptableObject.isDataDescriptor(desc)) {
if (Boolean.FALSE.equals(current.get("configurable"))
&& Boolean.FALSE.equals(current.get("writable"))) {
if (Boolean.TRUE.equals(ScriptableObject.hasProperty(desc, "writable"))
&& Boolean.TRUE.equals(desc.get("writable"))) {
} else if (current.isDataDescriptor() && desc.isDataDescriptor()) {
if (current.isConfigurable(false) && current.isWritable(false)) {
if (desc.isWritable()) {
return false;
}
if (Boolean.TRUE.equals(ScriptableObject.hasProperty(desc, "value"))
&& !Objects.equals(desc.get("value"), current.get("value"))) {
if (desc.hasValue() && !Objects.equals(desc.value, current.value)) {
return false;
}
return true;
}
} else {
if (Boolean.FALSE.equals(current.get("configurable"))) {
if (Boolean.TRUE.equals(ScriptableObject.hasProperty(desc, "set"))
&& !Objects.equals(desc.get("set"), current.get("set"))) {
if (current.isConfigurable(false)) {
if (desc.hasSetter() && !Objects.equals(desc.setter, current.setter)) {
return false;
}
if (Boolean.TRUE.equals(ScriptableObject.hasProperty(desc, "get"))
&& !Objects.equals(desc.get("get"), current.get("get"))) {
if (desc.hasGetter() && !Objects.equals(desc.getter, current.getter)) {
return false;
}
return true;
Expand Down
31 changes: 14 additions & 17 deletions rhino/src/main/java/org/mozilla/javascript/AccessorSlot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.mozilla.javascript;

import org.mozilla.javascript.ScriptableObject.DescriptorInfo;

/**
* This is a specialization of Slot to store various types of values that are retrieved dynamically
* using Java and JavaScript functions. Unlike LambdaSlot, the fact that these values are accessed
Expand Down Expand Up @@ -43,45 +45,40 @@ boolean isSetterSlot() {
}

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

ScriptableObject desc = (ScriptableObject) cx.newObject(scope);
int attr = getAttributes();

DescriptorInfo desc;
boolean es6 = cx.getLanguageVersion() >= Context.VERSION_ES6;
if (es6) {
desc = new DescriptorInfo(ScriptableObject.NOT_FOUND, attr, false);
if (getter == null && setter == null) {
desc.defineProperty(
"writable",
(attr & ScriptableObject.READONLY) == 0,
ScriptableObject.EMPTY);
desc.writable = (attr & ScriptableObject.READONLY) == 0;
}
} else {
desc.setCommonDescriptorProperties(attr, getter == null && setter == null);
desc =
new DescriptorInfo(
ScriptableObject.NOT_FOUND, attr, getter == null && setter == null);
}

String fName = name == null ? "f" : name.toString();
if (getter != null) {
Function f = getter.asGetterFunction(fName, scope);
desc.defineProperty("get", f == null ? Undefined.instance : f, ScriptableObject.EMPTY);
desc.getter = f == null ? Undefined.instance : f;
}
if (setter != null) {
Function f = setter.asSetterFunction(fName, scope);
desc.defineProperty("set", f == null ? Undefined.instance : f, ScriptableObject.EMPTY);
desc.setter = f == null ? Undefined.instance : f;
} else if (es6) {
desc.defineProperty("set", Undefined.instance, ScriptableObject.EMPTY);
desc.setter = Undefined.instance;
}

if (es6) {
desc.defineProperty(
"enumerable", (attr & ScriptableObject.DONTENUM) == 0, ScriptableObject.EMPTY);
desc.defineProperty(
"configurable",
(attr & ScriptableObject.PERMANENT) == 0,
ScriptableObject.EMPTY);
desc.enumerable = (attr & ScriptableObject.DONTENUM) == 0;
desc.configurable = (attr & ScriptableObject.PERMANENT) == 0;
}

return desc;
Expand Down
18 changes: 8 additions & 10 deletions rhino/src/main/java/org/mozilla/javascript/Arguments.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ Object[] getIds(CompoundOperationMap map, boolean getNonEnumerable, boolean getS
}

@Override
protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) {
protected DescriptorInfo getOwnPropertyDescriptor(Context cx, Object id) {
if (ScriptRuntime.isSymbol(id) || id instanceof Scriptable) {
return super.getOwnPropertyDescriptor(cx, id);
}
Expand All @@ -272,18 +272,16 @@ protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) {
value = getFromActivation(index);
}
if (super.has(index, this)) { // the descriptor has been redefined
ScriptableObject desc = super.getOwnPropertyDescriptor(cx, id);
desc.put("value", desc, value);
DescriptorInfo desc = super.getOwnPropertyDescriptor(cx, id);
desc.value = value;
return desc;
}
Scriptable scope = getParentScope();
if (scope == null) scope = this;
return buildDataDescriptor(scope, value, EMPTY);
return buildDataDescriptor(value, EMPTY);
}

@Override
protected boolean defineOwnProperty(
Context cx, Object id, ScriptableObject desc, boolean checkValid) {
Context cx, Object id, DescriptorInfo desc, boolean checkValid) {
super.defineOwnProperty(cx, id, desc, checkValid);
if (ScriptRuntime.isSymbol(id)) {
return true;
Expand All @@ -296,17 +294,17 @@ protected boolean defineOwnProperty(
Object value = arg(index);
if (value == NOT_FOUND) return true;

if (isAccessorDescriptor(desc)) {
if (desc.isAccessorDescriptor()) {
removeArg(index);
return true;
}

Object newValue = getProperty(desc, "value");
Object newValue = desc.value;
if (newValue == NOT_FOUND) return true;

replaceArg(index, newValue);

if (isFalse(getProperty(desc, "writable"))) {
if (isFalse(desc.writable)) {
removeArg(index);
}
return true;
Expand Down
7 changes: 2 additions & 5 deletions rhino/src/main/java/org/mozilla/javascript/ArrayLikeAbstractOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import java.io.Serializable;
import java.util.Comparator;
import org.mozilla.javascript.ScriptableObject.DescriptorInfo;

/** Contains implementation of shared methods useful for arrays and typed arrays. */
public class ArrayLikeAbstractOperations {
Expand Down Expand Up @@ -275,11 +276,7 @@ static void defineElem(Context cx, Scriptable target, long index, Object value)
if (!(target instanceof NativeArray && ((NativeArray) target).getDenseOnly())
&& target instanceof ScriptableObject) {
var so = (ScriptableObject) target;
ScriptableObject desc = new NativeObject();
desc.defineProperty("value", value, 0);
desc.defineProperty("writable", Boolean.TRUE, 0);
desc.defineProperty("enumerable", Boolean.TRUE, 0);
desc.defineProperty("configurable", Boolean.TRUE, 0);
var desc = new DescriptorInfo(true, true, true, value);
so.defineOwnProperty(cx, index, desc);
return;
}
Expand Down
8 changes: 1 addition & 7 deletions rhino/src/main/java/org/mozilla/javascript/BoundFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,7 @@ public BoundFunction(
ScriptRuntime.setFunctionProtoAndParent(this, cx, scope, false);

Function thrower = ScriptRuntime.typeErrorThrower(cx);
NativeObject throwing = new NativeObject();
ScriptRuntime.setBuiltinProtoAndParent(throwing, scope, TopLevel.Builtins.Object);
throwing.put("get", throwing, thrower);
throwing.put("set", throwing, thrower);
throwing.put("enumerable", throwing, Boolean.FALSE);
throwing.put("configurable", throwing, Boolean.FALSE);
throwing.preventExtensions();
var throwing = new DescriptorInfo(false, NOT_FOUND, false, thrower, thrower, NOT_FOUND);

this.defineOwnProperty(cx, "caller", throwing, false);
this.defineOwnProperty(cx, "arguments", throwing, false);
Expand Down
5 changes: 2 additions & 3 deletions rhino/src/main/java/org/mozilla/javascript/BuiltInSlot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,8 @@ void setAttributes(int value) {

@Override
@SuppressWarnings("unchecked")
ScriptableObject getPropertyDescriptor(Context cx, Scriptable scope) {
return ScriptableObject.buildDataDescriptor(
scope, getValue((T) this.value), getAttributes());
DescriptorInfo getPropertyDescriptor(Context cx, Scriptable scope) {
return ScriptableObject.buildDataDescriptor(getValue((T) this.value), getAttributes());
}

@SuppressWarnings("unchecked")
Expand Down
Loading