Skip to content
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

Add ES2019 Symbol.prototype.description #1561

Closed
wants to merge 3 commits into from
Closed

Add ES2019 Symbol.prototype.description #1561

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
9e31c94
Add ES2019 Symbol.prototype.description
camnwalter Aug 11, 2024
50f7426
Make Symbols with undefined not be an empty string description
camnwalter Aug 12, 2024
ab8343d
Update test262.properties
camnwalter Aug 12, 2024
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
31 changes: 28 additions & 3 deletions rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,21 @@ public class NativeSymbol extends IdScriptableObject implements Symbol {
private static final Object GLOBAL_TABLE_KEY = new Object();
private static final Object CONSTRUCTOR_SLOT = new Object();

static final SymbolKey GETDESCRIPTION = new SymbolKey("[Symbol.getDescription]");

private final SymbolKey key;
private final NativeSymbol symbolData;

public static void init(Context cx, Scriptable scope, boolean sealed) {
NativeSymbol obj = new NativeSymbol("");
ScriptableObject ctor = obj.exportAsJSClass(MAX_PROTOTYPE_ID, scope, false);

ScriptableObject desc = (ScriptableObject) cx.newObject(scope);
desc.put("enumerable", desc, Boolean.FALSE);
desc.put("configurable", desc, Boolean.TRUE);
desc.put("get", desc, obj.get(GETDESCRIPTION, obj));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
desc.put("get", desc, obj.get(GETDESCRIPTION, obj));
desc.put("get", desc, new LambdaFunction(scope, 0, NativeSymbol::js_getDescription));

I was able to change the code to look like this to not have to deal with the new SymbolKey and without having to deal with converting everything else to using lambda. Is this good?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that'll do, but would like @gbrail to have a look as well

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, the implementation of "length" in NativeArray is implemented mostly here

@gbrail the NativeArray.length property is not defined as an accessor property, but as a value property (and correctly so)

Symbol.description is, together with a few other properties (see #963) to be implemented as an accessor property, so with a get property defined in the PropertyDescriptor.

To my knowledge the only place where we do this currently is for the size property of Set and Map, which is where the (convoluted) SymbolKey approach came from.

But I think the LambdaFunction approach @camnwalter came up with now is cleaner and gets the job done, agree?

obj.defineOwnProperty(cx, "description", desc);

cx.putThreadLocal(CONSTRUCTOR_SLOT, Boolean.TRUE);
try {
createStandardSymbol(cx, scope, ctor, "iterator", SymbolKey.ITERATOR);
Expand Down Expand Up @@ -134,6 +142,8 @@ protected int findPrototypeId(Symbol key) {
return SymbolId_toStringTag;
} else if (SymbolKey.TO_PRIMITIVE.equals(key)) {
return SymbolId_toPrimitive;
} else if (GETDESCRIPTION.equals(key)) {
return SymbolId_description;
}
return 0;
}
Expand All @@ -143,9 +153,10 @@ protected int findPrototypeId(Symbol key) {
Id_constructor = 1,
Id_toString = 2,
Id_valueOf = 4,
SymbolId_description = 6,
SymbolId_toStringTag = 3,
SymbolId_toPrimitive = 5,
MAX_PROTOTYPE_ID = SymbolId_toPrimitive;
MAX_PROTOTYPE_ID = SymbolId_description;

@Override
protected void initPrototypeId(int id) {
Expand All @@ -159,6 +170,9 @@ protected void initPrototypeId(int id) {
case Id_valueOf:
initPrototypeMethod(CLASS_NAME, id, "valueOf", 0);
break;
case SymbolId_description:
initPrototypeMethod(CLASS_NAME, id, GETDESCRIPTION, "get description", 0);
break;
case SymbolId_toStringTag:
initPrototypeValue(id, SymbolKey.TO_STRING_TAG, CLASS_NAME, DONTENUM | READONLY);
break;
Expand Down Expand Up @@ -201,6 +215,9 @@ public Object execIdCall(
case Id_valueOf:
case SymbolId_toPrimitive:
return getSelf(cx, scope, thisObj).js_valueOf();

case SymbolId_description:
return getSelf(cx, scope, thisObj).js_getDescription();
default:
return super.execIdCall(f, cx, scope, thisObj, args);
}
Expand All @@ -218,12 +235,12 @@ private static NativeSymbol js_constructor(Object[] args) {
String desc;
if (args.length > 0) {
if (Undefined.instance.equals(args[0])) {
desc = "";
desc = null;
} else {
desc = ScriptRuntime.toString(args[0]);
}
} else {
desc = "";
desc = null;
}

if (args.length > 1) {
Expand Down Expand Up @@ -271,6 +288,14 @@ private Object js_keyFor(Context cx, Scriptable scope, Object[] args) {
return Undefined.instance;
}

private Object js_getDescription() {
String name = key.getName();
if (name == null) {
return Undefined.instance;
}
return name;
}

@Override
public String toString() {
return key.toString();
Expand Down
10 changes: 2 additions & 8 deletions tests/testsrc/test262.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2426,10 +2426,9 @@ built-ins/String 140/1182 (11.84%)

built-ins/StringIteratorPrototype 0/7 (0.0%)

built-ins/Symbol 34/92 (36.96%)
built-ins/Symbol 28/92 (30.43%)
asyncIterator/prop-desc.js
for/cross-realm.js
for/description.js
for/not-a-constructor.js {unsupported: [Reflect.construct]}
hasInstance/cross-realm.js
isConcatSpreadable/cross-realm.js
Expand All @@ -2439,12 +2438,7 @@ built-ins/Symbol 34/92 (36.96%)
keyFor/not-a-constructor.js {unsupported: [Reflect.construct]}
matchAll 2/2 (100.0%)
match/cross-realm.js
prototype/description/description-symboldescriptivestring.js
prototype/description/descriptor.js
prototype/description/get.js
prototype/description/this-val-non-symbol.js
prototype/description/this-val-symbol.js
prototype/description/wrapper.js
prototype/description/this-val-non-symbol.js {unsupported: [Proxy]}
prototype/Symbol.toPrimitive/name.js
prototype/Symbol.toPrimitive/prop-desc.js
prototype/Symbol.toPrimitive/redefined-symbol-wrapper-ordinary-toprimitive.js
Expand Down
Loading