From e89de18cfdfc4cda558336910a0218c97a2d4139 Mon Sep 17 00:00:00 2001 From: Andrea Bergia Date: Wed, 7 Aug 2024 11:17:54 +0200 Subject: [PATCH 1/5] Implement `Symbol.prototype.description` Refactored `NativeSymbol` to use `LambdaConstructor` instead of `IdScriptableObject`. --- .../mozilla/javascript/LambdaConstructor.java | 19 +- .../org/mozilla/javascript/NativeSymbol.java | 210 +++++++----------- .../org/mozilla/javascript/SymbolKey.java | 17 +- tests/testsrc/jstests/es6/symbols.js | 15 ++ 4 files changed, 129 insertions(+), 132 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java index c36cebc86d..39ad85bbee 100644 --- a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java +++ b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java @@ -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; /** @@ -130,6 +130,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(); diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java b/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java index 8add15dae2..d506253685 100644 --- a/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java +++ b/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java @@ -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"; @@ -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 { @@ -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); } @@ -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); @@ -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) { @@ -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 table = getGlobalMap(); + Map table = getGlobalMap(scope); NativeSymbol ret = table.get(name); if (ret == null) { @@ -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 table = getGlobalMap(); + Map table = getGlobalMap(scope); for (Map.Entry e : table.entrySet()) { if (e.getValue().key == sym.key) { return e.getKey(); @@ -341,8 +293,8 @@ SymbolKey getKey() { } @SuppressWarnings("unchecked") - private Map getGlobalMap() { - ScriptableObject top = (ScriptableObject) getTopLevelScope(this); + private static Map getGlobalMap(Scriptable scope) { + ScriptableObject top = (ScriptableObject) getTopLevelScope(scope); Map map = (Map) top.getAssociatedValue(GLOBAL_TABLE_KEY); if (map == null) { diff --git a/rhino/src/main/java/org/mozilla/javascript/SymbolKey.java b/rhino/src/main/java/org/mozilla/javascript/SymbolKey.java index 4649777cf2..a922b760cf 100644 --- a/rhino/src/main/java/org/mozilla/javascript/SymbolKey.java +++ b/rhino/src/main/java/org/mozilla/javascript/SymbolKey.java @@ -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 Symbol()). + */ 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 Symbol()). + */ + public Object getDescription() { + return name != null ? name : Undefined.instance; } @Override diff --git a/tests/testsrc/jstests/es6/symbols.js b/tests/testsrc/jstests/es6/symbols.js index 534fbe22fd..021d36bbd8 100644 --- a/tests/testsrc/jstests/es6/symbols.js +++ b/tests/testsrc/jstests/es6/symbols.js @@ -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"; From 778c5aaf034b33f100b77652c4dd4d84b7dfb6a3 Mon Sep 17 00:00:00 2001 From: Greg Brail Date: Tue, 24 Sep 2024 16:49:40 -0700 Subject: [PATCH 2/5] Fix test262 tests that pass --- tests/testsrc/test262.properties | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/testsrc/test262.properties b/tests/testsrc/test262.properties index 6c228bd74c..19d3db91b4 100644 --- a/tests/testsrc/test262.properties +++ b/tests/testsrc/test262.properties @@ -2312,25 +2312,15 @@ built-ins/Symbol 36/94 (38.3%) asyncDispose/prop-desc.js asyncIterator/prop-desc.js dispose/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]} From 319b675f1f6020153728502f1e47311c2d3e9f3c Mon Sep 17 00:00:00 2001 From: Greg Brail Date: Tue, 24 Sep 2024 18:22:11 -0700 Subject: [PATCH 3/5] Clean up NativeSymbol to use built-in features of LambdaConstructor. Fix how LambdaConstructor handles constructor functions. --- .../mozilla/javascript/LambdaConstructor.java | 6 +- .../org/mozilla/javascript/NativeSymbol.java | 126 +++++++----------- .../javascript/tests/LambdaFunctionTest.java | 16 +++ 3 files changed, 66 insertions(+), 82 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java index 39ad85bbee..0cf83baf79 100644 --- a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java +++ b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java @@ -87,7 +87,7 @@ public Object call(Context cx, Scriptable scope, Scriptable thisObj, Object[] ar throw ScriptRuntime.typeErrorById("msg.constructor.no.function", getFunctionName()); } if (target == null) { - return targetConstructor.construct(cx, scope, args); + return fireConstructor(cx, scope, args); } return target.call(cx, scope, thisObj, args); } @@ -97,6 +97,10 @@ public Scriptable construct(Context cx, Scriptable scope, Object[] args) { if ((flags & CONSTRUCTOR_NEW) == 0) { throw ScriptRuntime.typeErrorById("msg.no.new", getFunctionName()); } + return fireConstructor(cx, scope, args); + } + + private Scriptable fireConstructor(Context cx, Scriptable scope, Object[] args) { Scriptable obj = targetConstructor.construct(cx, scope, args); obj.setPrototype(getClassPrototype()); obj.setParentScope(scope); diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java b/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java index d506253685..2def897f7b 100644 --- a/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java +++ b/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java @@ -21,7 +21,6 @@ public class NativeSymbol extends ScriptableObject implements Symbol { public static final String TYPE_NAME = "symbol"; private static final Object GLOBAL_TABLE_KEY = new Object(); - private static final Object CONSTRUCTOR_SLOT = new Object(); private final SymbolKey key; private final NativeSymbol symbolData; @@ -32,36 +31,18 @@ public static void init(Context cx, Scriptable scope, boolean sealed) { 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; - } - }; + LambdaConstructor.CONSTRUCTOR_FUNCTION, + NativeSymbol::js_constructor); ctor.setPrototypePropertyAttributes(DONTENUM | READONLY | PERMANENT); ctor.defineConstructorMethod( - scope, "for", 1, NativeSymbol::js_for, DONTENUM, DONTENUM | READONLY); + scope, + "for", + 1, + (lcx, lscope, thisObj, args) -> NativeSymbol.js_for(lscope, args, ctor), + DONTENUM, + DONTENUM | READONLY); ctor.defineConstructorMethod( scope, "keyFor", 1, NativeSymbol::js_keyFor, DONTENUM, DONTENUM | READONLY); @@ -82,24 +63,19 @@ public Object call( ScriptableObject.defineProperty(scope, CLASS_NAME, ctor, DONTENUM); - cx.putThreadLocal(CONSTRUCTOR_SLOT, Boolean.TRUE); - try { - createStandardSymbol(cx, scope, ctor, "iterator", SymbolKey.ITERATOR); - createStandardSymbol(cx, scope, ctor, "species", SymbolKey.SPECIES); - createStandardSymbol(cx, scope, ctor, "toStringTag", SymbolKey.TO_STRING_TAG); - createStandardSymbol(cx, scope, ctor, "hasInstance", SymbolKey.HAS_INSTANCE); - createStandardSymbol( - cx, scope, ctor, "isConcatSpreadable", SymbolKey.IS_CONCAT_SPREADABLE); - createStandardSymbol(cx, scope, ctor, "isRegExp", SymbolKey.IS_REGEXP); - createStandardSymbol(cx, scope, ctor, "toPrimitive", SymbolKey.TO_PRIMITIVE); - createStandardSymbol(cx, scope, ctor, "match", SymbolKey.MATCH); - createStandardSymbol(cx, scope, ctor, "replace", SymbolKey.REPLACE); - 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); - } + createStandardSymbol(scope, ctor, "iterator", SymbolKey.ITERATOR); + createStandardSymbol(scope, ctor, "species", SymbolKey.SPECIES); + createStandardSymbol(scope, ctor, "toStringTag", SymbolKey.TO_STRING_TAG); + createStandardSymbol(scope, ctor, "hasInstance", SymbolKey.HAS_INSTANCE); + createStandardSymbol( + scope, ctor, "isConcatSpreadable", SymbolKey.IS_CONCAT_SPREADABLE); + createStandardSymbol(scope, ctor, "isRegExp", SymbolKey.IS_REGEXP); + createStandardSymbol(scope, ctor, "toPrimitive", SymbolKey.TO_PRIMITIVE); + createStandardSymbol(scope, ctor, "match", SymbolKey.MATCH); + createStandardSymbol(scope, ctor, "replace", SymbolKey.REPLACE); + createStandardSymbol(scope, ctor, "search", SymbolKey.SEARCH); + createStandardSymbol(scope, ctor, "split", SymbolKey.SPLIT); + createStandardSymbol(scope, ctor, "unscopables", SymbolKey.UNSCOPABLES); if (sealed) { // Can't seal until we have created all the stuff above! @@ -107,17 +83,6 @@ public Object call( } } - /** - * This has to be used only for constructing the prototype instance. This sets symbolData to - * null (see isSymbol() for more). - * - * @param desc the description - */ - private NativeSymbol(String desc) { - this.key = new SymbolKey(desc); - this.symbolData = null; - } - NativeSymbol(SymbolKey key) { this.key = key; this.symbolData = this; @@ -128,36 +93,35 @@ public NativeSymbol(NativeSymbol s) { this.symbolData = s.symbolData; } - /** - * Use this when we need to create symbols internally because of the convoluted way we have to - * construct them. - */ - public static NativeSymbol construct(Context cx, Scriptable scope, Object[] args) { - cx.putThreadLocal(CONSTRUCTOR_SLOT, Boolean.TRUE); - try { - return (NativeSymbol) cx.newObject(scope, CLASS_NAME, args); - } finally { - cx.removeThreadLocal(CONSTRUCTOR_SLOT); - } - } - @Override public String getClassName() { return CLASS_NAME; } + private static NativeSymbol constructSymbol( + Scriptable scope, LambdaConstructor ctor, SymbolKey key) { + NativeSymbol sym = new NativeSymbol(key); + sym.setPrototype(ctor.getClassPrototype()); + sym.setParentScope(scope); + return sym; + } + + private static NativeSymbol constructSymbol( + Scriptable scope, LambdaConstructor ctor, String name) { + NativeSymbol sym = new NativeSymbol(new SymbolKey(name)); + sym.setPrototype(ctor.getClassPrototype()); + sym.setParentScope(scope); + return sym; + } + private static void createStandardSymbol( - Context cx, Scriptable scope, LambdaConstructor ctor, String name, SymbolKey key) { - Scriptable sym = (Scriptable) ctor.call(cx, scope, scope, new Object[] {name, key}); + Scriptable scope, LambdaConstructor ctor, String name, SymbolKey key) { + NativeSymbol sym = constructSymbol(scope, ctor, key); ctor.defineProperty(name, sym, DONTENUM | READONLY | PERMANENT); } - private static NativeSymbol getSelf(Context cx, Scriptable scope, Object thisObj) { - try { - return (NativeSymbol) ScriptRuntime.toObject(cx, scope, thisObj); - } catch (ClassCastException cce) { - throw ScriptRuntime.typeErrorById("msg.invalid.type", thisObj.getClass().getName()); - } + private static NativeSymbol getSelf(Scriptable thisObj) { + return LambdaConstructor.convertThisObject(thisObj, NativeSymbol.class); } private static NativeSymbol js_constructor(Context cx, Scriptable scope, Object[] args) { @@ -175,13 +139,13 @@ private static NativeSymbol js_constructor(Context cx, Scriptable scope, Object[ private static String js_toString( Context cx, Scriptable scope, Scriptable thisObj, Object[] args) { - return getSelf(cx, scope, thisObj).toString(); + return getSelf(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 getSelf(cx, scope, thisObj).symbolData; + return getSelf(thisObj).symbolData; } private static Object js_description(Scriptable thisObj) { @@ -189,7 +153,7 @@ private static Object js_description(Scriptable thisObj) { return self.getKey().getDescription(); } - private static Object js_for(Context cx, Scriptable scope, Scriptable thisObj, Object[] args) { + private static Object js_for(Scriptable scope, Object[] args, LambdaConstructor constructor) { String name = (args.length > 0 ? ScriptRuntime.toString(args[0]) @@ -199,7 +163,7 @@ private static Object js_for(Context cx, Scriptable scope, Scriptable thisObj, O NativeSymbol ret = table.get(name); if (ret == null) { - ret = construct(cx, scope, new Object[] {name}); + ret = constructSymbol(scope, constructor, name); table.put(name, ret); } return ret; diff --git a/tests/src/test/java/org/mozilla/javascript/tests/LambdaFunctionTest.java b/tests/src/test/java/org/mozilla/javascript/tests/LambdaFunctionTest.java index ba2d7bdc5d..0554f87823 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/LambdaFunctionTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/LambdaFunctionTest.java @@ -90,6 +90,22 @@ public void constructLambdaClass() { + "assertTrue(tc instanceof TestClass);\n"); } + @Test + public void constructLambdaClassWithFunction() { + TestClass.init(root); + eval( + "let tc = TestClass('foo');\n" + + "assertEquals(tc.value, 'foo');\n" + + "tc.value = 'bar';\n" + + "assertEquals(tc.value, 'bar');\n" + + "tc.anotherValue = 123;\n" + + "assertEquals(tc.anotherValue, 123);\n" + + "assertEquals(TestClass.name, 'TestClass');\n" + + "assertEquals(TestClass.length, 1);\n" + + "assertEquals(typeof TestClass, 'function');\n" + + "assertTrue(tc instanceof TestClass);\n"); + } + @Test public void nativePrototypeFunctions() { eval( From f06af412ab558925812534c08059596121c36c2f Mon Sep 17 00:00:00 2001 From: Greg Brail Date: Tue, 24 Sep 2024 21:37:43 -0700 Subject: [PATCH 4/5] Clean up Map usage and comments --- .../mozilla/javascript/LambdaConstructor.java | 48 ++++++++++--- .../org/mozilla/javascript/NativeSymbol.java | 70 +++++++++---------- 2 files changed, 72 insertions(+), 46 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java index 0cf83baf79..c479beccef 100644 --- a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java +++ b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java @@ -41,7 +41,9 @@ public class LambdaConstructor extends LambdaFunction { /** * Create a new function that may be used as a constructor. The new object will have the * Function prototype and no parent. The caller is responsible for binding this object to the - * appropriate scope. + * appropriate scope. The new constructor function can be invoked using "new" or by calling it + * directly, and in either case will result in a new object being returned and wired to the + * correct prototype and scope. * * @param scope scope of the calling context * @param name name of the function @@ -56,8 +58,18 @@ public LambdaConstructor(Scriptable scope, String name, int length, Constructabl } /** - * Create a new function and control whether it may be invoked using new, as a function, or - * both. + * Create a new function that may be used as a constructor. The new object will have the + * Function prototype and no parent. The caller is responsible for binding this object to the + * appropriate scope. The "flags" argument controls whether the function may be invoked using + * "new," via a direct call, or both. If allowed by the flags, then the constructor will have + * the same effect either way. If not allowed by the flags, then a TypeError will be thrown. + * + * @param scope scope of the calling context + * @param name name of the function + * @param length the arity of the function + * @param flags which may be a combination of CONSTRUCTOR_NEW and CONSTRUCTOR_FUNCTION + * @param target an object that implements the function in Java. Since Constructable is a + * single-function interface this will typically be implemented as a lambda. */ public LambdaConstructor( Scriptable scope, String name, int length, int flags, Constructable target) { @@ -67,8 +79,19 @@ public LambdaConstructor( } /** - * Create a new constructor that may be called using new or as a function, and exhibits - * different behavior for each. + * Create a new function that may be used as a constructor. The new object will have the + * Function prototype and no parent. The caller is responsible for binding this object to the + * appropriate scope. The new constructor function will have different behavior depending on + * whether it is invoked via "new" or via a direct call. In the case of "new", a new object with + * a prototype and scope chain be returned, but in the case of a direct call, the user must + * implement whatever they need. This is typically used in the case of functions like the native + * Date constructor, which has totally different behavior depending on how it's invoked. + * + * @param scope scope of the calling context + * @param name name of the function + * @param length the arity of the function + * @param target an object that implements the function in Java. Since Constructable is a + * single-function interface this will typically be implemented as a lambda. */ public LambdaConstructor( Scriptable scope, @@ -162,15 +185,22 @@ public void definePrototypeProperty(Symbol key, Object value, int attributes) { proto.defineProperty(key, value, attributes); } + /** + * Define a property on the prototype using a function. The function will be wired to a + * JavaScript function, so the resulting property will look just like one that was defined using + * "Object.defineOwnProperty" with a property descriptor. + */ public void definePrototypeProperty( - Context cx, - String name, - java.util.function.Function getter, - int attributes) { + Context cx, String name, Function getter, int attributes) { ScriptableObject proto = getPrototypeScriptable(); proto.defineProperty(cx, name, getter, null, attributes); } + /** + * Define a property on the prototype using functions for getter and setter. The function will + * be wired to a JavaScript function, so the resulting property will look just like one that was + * defined using "Object.defineOwnProperty" with a property descriptor. + */ public void definePrototypeProperty( Context cx, String name, diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java b/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java index 2def897f7b..b47b814f23 100644 --- a/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java +++ b/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java @@ -40,7 +40,7 @@ public static void init(Context cx, Scriptable scope, boolean sealed) { scope, "for", 1, - (lcx, lscope, thisObj, args) -> NativeSymbol.js_for(lscope, args, ctor), + (lcx, lscope, thisObj, args) -> NativeSymbol.js_for(lcx, lscope, args, ctor), DONTENUM, DONTENUM | READONLY); ctor.defineConstructorMethod( @@ -63,19 +63,19 @@ public static void init(Context cx, Scriptable scope, boolean sealed) { ScriptableObject.defineProperty(scope, CLASS_NAME, ctor, DONTENUM); - createStandardSymbol(scope, ctor, "iterator", SymbolKey.ITERATOR); - createStandardSymbol(scope, ctor, "species", SymbolKey.SPECIES); - createStandardSymbol(scope, ctor, "toStringTag", SymbolKey.TO_STRING_TAG); - createStandardSymbol(scope, ctor, "hasInstance", SymbolKey.HAS_INSTANCE); - createStandardSymbol( - scope, ctor, "isConcatSpreadable", SymbolKey.IS_CONCAT_SPREADABLE); - createStandardSymbol(scope, ctor, "isRegExp", SymbolKey.IS_REGEXP); - createStandardSymbol(scope, ctor, "toPrimitive", SymbolKey.TO_PRIMITIVE); - createStandardSymbol(scope, ctor, "match", SymbolKey.MATCH); - createStandardSymbol(scope, ctor, "replace", SymbolKey.REPLACE); - createStandardSymbol(scope, ctor, "search", SymbolKey.SEARCH); - createStandardSymbol(scope, ctor, "split", SymbolKey.SPLIT); - createStandardSymbol(scope, ctor, "unscopables", SymbolKey.UNSCOPABLES); + // Create all the predefined symbols and bind them to the scope. + createStandardSymbol(cx, scope, ctor, "iterator", SymbolKey.ITERATOR); + createStandardSymbol(cx, scope, ctor, "species", SymbolKey.SPECIES); + createStandardSymbol(cx, scope, ctor, "toStringTag", SymbolKey.TO_STRING_TAG); + createStandardSymbol(cx, scope, ctor, "hasInstance", SymbolKey.HAS_INSTANCE); + createStandardSymbol(cx, scope, ctor, "isConcatSpreadable", SymbolKey.IS_CONCAT_SPREADABLE); + createStandardSymbol(cx, scope, ctor, "isRegExp", SymbolKey.IS_REGEXP); + createStandardSymbol(cx, scope, ctor, "toPrimitive", SymbolKey.TO_PRIMITIVE); + createStandardSymbol(cx, scope, ctor, "match", SymbolKey.MATCH); + createStandardSymbol(cx, scope, ctor, "replace", SymbolKey.REPLACE); + createStandardSymbol(cx, scope, ctor, "search", SymbolKey.SEARCH); + createStandardSymbol(cx, scope, ctor, "split", SymbolKey.SPLIT); + createStandardSymbol(cx, scope, ctor, "unscopables", SymbolKey.UNSCOPABLES); if (sealed) { // Can't seal until we have created all the stuff above! @@ -98,25 +98,23 @@ public String getClassName() { return CLASS_NAME; } + /** + * Create a symbol directly. We use this internally to construct new symbols as if the + * constructor was called directly. + */ private static NativeSymbol constructSymbol( - Scriptable scope, LambdaConstructor ctor, SymbolKey key) { - NativeSymbol sym = new NativeSymbol(key); - sym.setPrototype(ctor.getClassPrototype()); - sym.setParentScope(scope); - return sym; + Context cx, Scriptable scope, LambdaConstructor ctor, SymbolKey key) { + return (NativeSymbol) ctor.call(cx, scope, null, new Object[] {Undefined.instance, key}); } private static NativeSymbol constructSymbol( - Scriptable scope, LambdaConstructor ctor, String name) { - NativeSymbol sym = new NativeSymbol(new SymbolKey(name)); - sym.setPrototype(ctor.getClassPrototype()); - sym.setParentScope(scope); - return sym; + Context cx, Scriptable scope, LambdaConstructor ctor, String name) { + return (NativeSymbol) ctor.call(cx, scope, null, new Object[] {name}); } private static void createStandardSymbol( - Scriptable scope, LambdaConstructor ctor, String name, SymbolKey key) { - NativeSymbol sym = constructSymbol(scope, ctor, key); + Context cx, Scriptable scope, LambdaConstructor ctor, String name, SymbolKey key) { + NativeSymbol sym = constructSymbol(cx, scope, ctor, key); ctor.defineProperty(name, sym, DONTENUM | READONLY | PERMANENT); } @@ -144,29 +142,22 @@ private static String js_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 getSelf(thisObj).symbolData; } private static Object js_description(Scriptable thisObj) { - NativeSymbol self = LambdaConstructor.convertThisObject(thisObj, NativeSymbol.class); - return self.getKey().getDescription(); + return getSelf(thisObj).getKey().getDescription(); } - private static Object js_for(Scriptable scope, Object[] args, LambdaConstructor constructor) { + private static Object js_for( + Context cx, Scriptable scope, Object[] args, LambdaConstructor constructor) { String name = (args.length > 0 ? ScriptRuntime.toString(args[0]) : ScriptRuntime.toString(Undefined.instance)); Map table = getGlobalMap(scope); - NativeSymbol ret = table.get(name); - - if (ret == null) { - ret = constructSymbol(scope, constructor, name); - table.put(name, ret); - } - return ret; + return table.computeIfAbsent(name, (k) -> constructSymbol(cx, scope, constructor, name)); } @SuppressWarnings("ReferenceEquality") @@ -256,6 +247,11 @@ SymbolKey getKey() { return key; } + /** + * Return the Map that stores global symbols for the 'for' and 'keyFor' operations. It must work + * across "realms" in the same top-level Rhino scope, so we store it there as an associated + * property. + */ @SuppressWarnings("unchecked") private static Map getGlobalMap(Scriptable scope) { ScriptableObject top = (ScriptableObject) getTopLevelScope(scope); From 102ee6503546ceaab779ae5f4fc50aa6fae8e302 Mon Sep 17 00:00:00 2001 From: Greg Brail Date: Wed, 25 Sep 2024 21:34:54 -0700 Subject: [PATCH 5/5] Fix test262.properties statistics --- tests/testsrc/test262.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testsrc/test262.properties b/tests/testsrc/test262.properties index 19d3db91b4..bdb3006c1b 100644 --- a/tests/testsrc/test262.properties +++ b/tests/testsrc/test262.properties @@ -2308,7 +2308,7 @@ built-ins/String 140/1182 (11.84%) built-ins/StringIteratorPrototype 0/7 (0.0%) -built-ins/Symbol 36/94 (38.3%) +built-ins/Symbol 26/94 (27.66%) asyncDispose/prop-desc.js asyncIterator/prop-desc.js dispose/prop-desc.js