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.

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

Implement Symbol.prototype.description and refactor Symbol to use LambdaConstructor #1579

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class LambdaConstructor extends LambdaFunction {
public static final int CONSTRUCTOR_DEFAULT = CONSTRUCTOR_FUNCTION | CONSTRUCTOR_NEW;

// Lambdas should not be serialized.
private final transient Constructable targetConstructor;
protected final transient Constructable targetConstructor;
private final int flags;

/**
Expand Down Expand Up @@ -112,6 +112,23 @@ public void definePrototypeMethod(
proto.defineProperty(name, f, attributes);
}

/**
* Define a function property on the prototype of the constructor using a LambdaFunction under
* the covers.
*/
public void definePrototypeMethod(
Scriptable scope,
SymbolKey name,
int length,
Callable target,
int attributes,
int propertyAttributes) {
LambdaFunction f = new LambdaFunction(scope, "[" + name.getName() + "]", length, target);
f.setStandardPropertyAttributes(propertyAttributes);
ScriptableObject proto = getPrototypeScriptable();
proto.defineProperty(name, f, attributes);
}

/** Define a property that may be of any type on the prototype of this constructor. */
public void definePrototypeProperty(String name, Object value, int attributes) {
ScriptableObject proto = getPrototypeScriptable();
Expand Down
210 changes: 81 additions & 129 deletions rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* properties. One of them is that some objects can have an "internal data slot" that makes them a
* Symbol and others cannot.
*/
public class NativeSymbol extends IdScriptableObject implements Symbol {
public class NativeSymbol extends ScriptableObject implements Symbol {
private static final long serialVersionUID = -589539749749830003L;

public static final String CLASS_NAME = "Symbol";
Expand All @@ -27,8 +27,60 @@ public class NativeSymbol extends IdScriptableObject implements Symbol {
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);
LambdaConstructor ctor =
new LambdaConstructor(
scope,
CLASS_NAME,
0,
0, // Unused
NativeSymbol::js_constructor) {
// Calling new Symbol('foo') is not allowed. However, we need to be able to
// create
// a new symbol instance to register built-in ones and for the implementation of
// "for",
// so we have this trick of a thread-local variable to allow it.
@Override
public Scriptable construct(Context cx, Scriptable scope, Object[] args) {
if (cx.getThreadLocal(CONSTRUCTOR_SLOT) == null) {
throw ScriptRuntime.typeErrorById("msg.no.new", getFunctionName());
}
return (Scriptable) call(cx, scope, null, args);
}

// Calling Symbol('foo') should act like a constructor call
@Override
public Object call(
Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
Scriptable obj = targetConstructor.construct(cx, scope, args);
obj.setPrototype(getClassPrototype());
obj.setParentScope(scope);
return obj;
}
};

ctor.setPrototypePropertyAttributes(DONTENUM | READONLY | PERMANENT);

ctor.defineConstructorMethod(
scope, "for", 1, NativeSymbol::js_for, DONTENUM, DONTENUM | READONLY);
ctor.defineConstructorMethod(
scope, "keyFor", 1, NativeSymbol::js_keyFor, DONTENUM, DONTENUM | READONLY);

ctor.definePrototypeMethod(
scope, "toString", 0, NativeSymbol::js_toString, DONTENUM, DONTENUM | READONLY);
ctor.definePrototypeMethod(
scope, "valueOf", 0, NativeSymbol::js_valueOf, DONTENUM, DONTENUM | READONLY);
ctor.definePrototypeMethod(
scope,
SymbolKey.TO_PRIMITIVE,
1,
NativeSymbol::js_valueOf,
DONTENUM | READONLY,
DONTENUM | READONLY);
ctor.definePrototypeProperty(SymbolKey.TO_STRING_TAG, CLASS_NAME, DONTENUM | READONLY);
ctor.definePrototypeProperty(
cx, "description", NativeSymbol::js_description, DONTENUM | READONLY);

ScriptableObject.defineProperty(scope, CLASS_NAME, ctor, DONTENUM);

cx.putThreadLocal(CONSTRUCTOR_SLOT, Boolean.TRUE);
try {
Expand All @@ -45,7 +97,6 @@ public static void init(Context cx, Scriptable scope, boolean sealed) {
createStandardSymbol(cx, scope, ctor, "search", SymbolKey.SEARCH);
createStandardSymbol(cx, scope, ctor, "split", SymbolKey.SPLIT);
createStandardSymbol(cx, scope, ctor, "unscopables", SymbolKey.UNSCOPABLES);

} finally {
cx.removeThreadLocal(CONSTRUCTOR_SLOT);
}
Expand Down Expand Up @@ -95,117 +146,12 @@ public String getClassName() {
return CLASS_NAME;
}

@Override
protected void fillConstructorProperties(IdFunctionObject ctor) {
super.fillConstructorProperties(ctor);
addIdFunctionProperty(ctor, CLASS_NAME, ConstructorId_for, "for", 1);
addIdFunctionProperty(ctor, CLASS_NAME, ConstructorId_keyFor, "keyFor", 1);
}

private static void createStandardSymbol(
Context cx, Scriptable scope, ScriptableObject ctor, String name, SymbolKey key) {
Scriptable sym = cx.newObject(scope, CLASS_NAME, new Object[] {name, key});
Context cx, Scriptable scope, LambdaConstructor ctor, String name, SymbolKey key) {
Scriptable sym = (Scriptable) ctor.call(cx, scope, scope, new Object[] {name, key});
ctor.defineProperty(name, sym, DONTENUM | READONLY | PERMANENT);
}

@Override
protected int findPrototypeId(String s) {
int id = 0;
switch (s) {
case "constructor":
id = Id_constructor;
break;
case "toString":
id = Id_toString;
break;
case "valueOf":
id = Id_valueOf;
break;
default:
id = 0;
break;
}
return id;
}

@Override
protected int findPrototypeId(Symbol key) {
if (SymbolKey.TO_STRING_TAG.equals(key)) {
return SymbolId_toStringTag;
} else if (SymbolKey.TO_PRIMITIVE.equals(key)) {
return SymbolId_toPrimitive;
}
return 0;
}

private static final int ConstructorId_keyFor = -2,
ConstructorId_for = -1,
Id_constructor = 1,
Id_toString = 2,
Id_valueOf = 4,
SymbolId_toStringTag = 3,
SymbolId_toPrimitive = 5,
MAX_PROTOTYPE_ID = SymbolId_toPrimitive;

@Override
protected void initPrototypeId(int id) {
switch (id) {
case Id_constructor:
initPrototypeMethod(CLASS_NAME, id, "constructor", 0);
break;
case Id_toString:
initPrototypeMethod(CLASS_NAME, id, "toString", 0);
break;
case Id_valueOf:
initPrototypeMethod(CLASS_NAME, id, "valueOf", 0);
break;
case SymbolId_toStringTag:
initPrototypeValue(id, SymbolKey.TO_STRING_TAG, CLASS_NAME, DONTENUM | READONLY);
break;
case SymbolId_toPrimitive:
initPrototypeMethod(
CLASS_NAME, id, SymbolKey.TO_PRIMITIVE, "Symbol.toPrimitive", 1);
break;
default:
super.initPrototypeId(id);
break;
}
}

@Override
public Object execIdCall(
IdFunctionObject f, Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
if (!f.hasTag(CLASS_NAME)) {
return super.execIdCall(f, cx, scope, thisObj, args);
}
int id = f.methodId();
switch (id) {
case ConstructorId_for:
return js_for(cx, scope, args);
case ConstructorId_keyFor:
return js_keyFor(cx, scope, args);

case Id_constructor:
if (thisObj == null) {
if (cx.getThreadLocal(CONSTRUCTOR_SLOT) == null) {
// We should never get to this via "new".
throw ScriptRuntime.typeErrorById("msg.no.symbol.new");
}
// Unless we are being called by our own internal "new"
return js_constructor(args);
}
return construct(cx, scope, args);

case Id_toString:
return getSelf(cx, scope, thisObj).toString();
case Id_valueOf:
case SymbolId_toPrimitive:
return getSelf(cx, scope, thisObj).js_valueOf();
default:
return super.execIdCall(f, cx, scope, thisObj, args);
}
}

private static NativeSymbol getSelf(Context cx, Scriptable scope, Object thisObj) {
try {
return (NativeSymbol) ScriptRuntime.toObject(cx, scope, thisObj);
Expand All @@ -214,16 +160,10 @@ private static NativeSymbol getSelf(Context cx, Scriptable scope, Object thisObj
}
}

private static NativeSymbol js_constructor(Object[] args) {
String desc;
if (args.length > 0) {
if (Undefined.instance.equals(args[0])) {
desc = "";
} else {
desc = ScriptRuntime.toString(args[0]);
}
} else {
desc = "";
private static NativeSymbol js_constructor(Context cx, Scriptable scope, Object[] args) {
String desc = null;
if (args.length > 0 && !Undefined.isUndefined(args[0])) {
desc = ScriptRuntime.toString(args[0]);
}

if (args.length > 1) {
Expand All @@ -233,18 +173,29 @@ private static NativeSymbol js_constructor(Object[] args) {
return new NativeSymbol(new SymbolKey(desc));
}

private Object js_valueOf() {
private static String js_toString(
Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
return getSelf(cx, scope, thisObj).toString();
}

private static Object js_valueOf(
Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
// In the case that "Object()" was called we actually have a different "internal slot"
return symbolData;
return getSelf(cx, scope, thisObj).symbolData;
}

private static Object js_description(Scriptable thisObj) {
NativeSymbol self = LambdaConstructor.convertThisObject(thisObj, NativeSymbol.class);
return self.getKey().getDescription();
}

private Object js_for(Context cx, Scriptable scope, Object[] args) {
private static Object js_for(Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
String name =
(args.length > 0
? ScriptRuntime.toString(args[0])
: ScriptRuntime.toString(Undefined.instance));

Map<String, NativeSymbol> table = getGlobalMap();
Map<String, NativeSymbol> table = getGlobalMap(scope);
NativeSymbol ret = table.get(name);

if (ret == null) {
Expand All @@ -255,14 +206,15 @@ private Object js_for(Context cx, Scriptable scope, Object[] args) {
}

@SuppressWarnings("ReferenceEquality")
private Object js_keyFor(Context cx, Scriptable scope, Object[] args) {
private static Object js_keyFor(
Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
Object s = (args.length > 0 ? args[0] : Undefined.instance);
if (!(s instanceof NativeSymbol)) {
throw ScriptRuntime.throwCustomError(cx, scope, "TypeError", "Not a Symbol");
}
NativeSymbol sym = (NativeSymbol) s;

Map<String, NativeSymbol> table = getGlobalMap();
Map<String, NativeSymbol> table = getGlobalMap(scope);
for (Map.Entry<String, NativeSymbol> e : table.entrySet()) {
if (e.getValue().key == sym.key) {
return e.getKey();
Expand Down Expand Up @@ -341,8 +293,8 @@ SymbolKey getKey() {
}

@SuppressWarnings("unchecked")
private Map<String, NativeSymbol> getGlobalMap() {
ScriptableObject top = (ScriptableObject) getTopLevelScope(this);
private static Map<String, NativeSymbol> getGlobalMap(Scriptable scope) {
ScriptableObject top = (ScriptableObject) getTopLevelScope(scope);
Map<String, NativeSymbol> map =
(Map<String, NativeSymbol>) top.getAssociatedValue(GLOBAL_TABLE_KEY);
if (map == null) {
Expand Down
17 changes: 15 additions & 2 deletions rhino/src/main/java/org/mozilla/javascript/SymbolKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,27 @@ public class SymbolKey implements Symbol, Serializable {
public static final SymbolKey SPLIT = new SymbolKey("Symbol.split");
public static final SymbolKey UNSCOPABLES = new SymbolKey("Symbol.unscopables");

private String name;
// If passed a javascript undefined, this will be a (java) null
private final String name;

public SymbolKey(String name) {
this.name = name;
}

/**
* Returns the symbol's name. Returns empty string for anonymous symbol (i.e. something created
* with <code>Symbol()</code>).
*/
public String getName() {
return name;
return name != null ? name : "";
}

/**
* Returns the symbol's description - will return {@link Undefined#instance} if we have an
* anonymous symbol (i.e. something created with <code>Symbol()</code>).
*/
public Object getDescription() {
return name != null ? name : Undefined.instance;
}

@Override
Expand Down
15 changes: 15 additions & 0 deletions tests/testsrc/jstests/es6/symbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -571,4 +571,19 @@ TestStringify('{}', symbol_wrapper);
symbol_wrapper.a = 1;
TestStringify('{"a":1}', symbol_wrapper);


function TestDescription() {
assertEquals(Symbol("abc").description, "abc");
assertEquals(Symbol.iterator.description, "Symbol.iterator");
assertEquals(Symbol().description, undefined);

assertFalse(Symbol("abc").hasOwnProperty("description"));
assertTrue(Symbol.prototype.hasOwnProperty("description"));
assertTrue(Object.getOwnPropertyDescriptor(Symbol.prototype, "description").configurable);
assertFalse(Object.getOwnPropertyDescriptor(Symbol.prototype, "description").enumerable);
assertTrue(Object.getOwnPropertyDescriptor(Symbol.prototype, "description").get !== null);
}
TestDescription();


"success";
12 changes: 1 addition & 11 deletions tests/testsrc/test262.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2299,27 +2299,17 @@ built-ins/String 140/1182 (11.84%)

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

built-ins/Symbol 34/92 (36.96%)
built-ins/Symbol 24/92 (26.09%)
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
iterator/cross-realm.js
keyFor/arg-non-symbol.js
keyFor/cross-realm.js
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/Symbol.toPrimitive/name.js
prototype/Symbol.toPrimitive/prop-desc.js
prototype/Symbol.toPrimitive/redefined-symbol-wrapper-ordinary-toprimitive.js
prototype/Symbol.toPrimitive/removed-symbol-wrapper-ordinary-toprimitive.js
prototype/toString/not-a-constructor.js {unsupported: [Reflect.construct]}
Expand Down
Loading