Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xe
0xe committed Dec 12, 2024
1 parent d13a4e4 commit 013b02b
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 168 deletions.
31 changes: 6 additions & 25 deletions rhino/src/main/java/org/mozilla/javascript/BaseFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ static void init(Context cx, Scriptable scope, boolean sealed) {
obj.setStandardPropertyAttributes(READONLY | DONTENUM);
}
IdFunctionObject constructor = obj.exportAsJSClass(MAX_PROTOTYPE_ID, scope, sealed);
ScriptRuntimeES6.addSymbolHasInstance(cx, scope, constructor);
if (cx.getLanguageVersion() >= Context.VERSION_ES6) {
ScriptRuntimeES6.addSymbolHasInstance(cx, scope, constructor);
}
}

/**
Expand Down Expand Up @@ -269,7 +271,7 @@ protected void fillConstructorProperties(IdFunctionObject ctor) {
@Override
protected void initPrototypeId(int id) {
if (id == SymbolId_hasInstance) {
initPrototypeValue(id, SymbolKey.HAS_INSTANCE, makeHasInstance(), 0x0F);
initPrototypeValue(id, SymbolKey.HAS_INSTANCE, makeHasInstance(), CONST | DONTENUM);
return;
}

Expand Down Expand Up @@ -359,9 +361,8 @@ public Object call(
}
throw ScriptRuntime.typeErrorById(
"msg.instanceof.bad.prototype", getFunctionName());
} else {
return false; // NOT_FOUND, null etc.
}
return false; // NOT_FOUND, null etc.
}
});
}
Expand Down Expand Up @@ -564,26 +565,6 @@ protected boolean hasPrototypeProperty() {
return prototypeProperty != null || this instanceof NativeFunction;
}

@Override
ScriptableObject buildDataDescriptorHelper(
int instanceIdInfo, Scriptable scope, Object value, int attr) {
if (instanceIdInfo == SymbolId_hasInstance) {
return buildDataDescriptor(scope, value, attr, SymbolKey.HAS_INSTANCE.toString(), 1);
} else {
return super.buildDataDescriptorHelper(instanceIdInfo, scope, value, attr);
}
}

@Override
ScriptableObject buildDataDescriptorHelper(
Symbol key, Scriptable scope, Object value, int attr) {
if (key == SymbolKey.HAS_INSTANCE) {
return buildDataDescriptor(scope, value, attr, key.toString(), 1);
} else {
return super.buildDataDescriptorHelper(key, scope, value, attr);
}
}

public Object getPrototypeProperty() {
Object result = prototypeProperty;
if (result == null) {
Expand Down Expand Up @@ -705,7 +686,7 @@ private Object jsConstructor(Context cx, Scriptable scope, Object[] args) {
@Override
protected int findPrototypeId(Symbol k) {
if (SymbolKey.HAS_INSTANCE.equals(k)) return SymbolId_hasInstance;
else return 0;
return 0;
}

@Override
Expand Down
23 changes: 4 additions & 19 deletions rhino/src/main/java/org/mozilla/javascript/IdScriptableObject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ final void delete(int id) {
String name = null;
if (valueArray[nameSlot] instanceof String)
name = (String) valueArray[nameSlot];
else if (valueArray[nameSlot] instanceof SymbolKey) {
else if (valueArray[nameSlot] instanceof Symbol) {
name = valueArray[nameSlot].toString();
}
throw ScriptRuntime.typeErrorById(
Expand Down Expand Up @@ -942,21 +942,6 @@ protected ScriptableObject getOwnPropertyDescriptor(Context cx, Object id) {
return desc;
}

/**
* Overridden in the base class for different descriptors
*
* @return ScriptableObject
*/
ScriptableObject buildDataDescriptorHelper(
Symbol key, Scriptable scope, Object value, int attr) {
return buildDataDescriptor(scope, value, attr);
}

ScriptableObject buildDataDescriptorHelper(
int instanceIdInfo, Scriptable scope, Object value, int attr) {
return buildDataDescriptor(scope, value, attr);
}

private ScriptableObject getBuiltInDescriptor(String name) {
Object value = null;
int attr = EMPTY;
Expand All @@ -971,14 +956,14 @@ private ScriptableObject getBuiltInDescriptor(String name) {
int id = (info & 0xFFFF);
value = getInstanceIdValue(id);
attr = (info >>> 16);
return buildDataDescriptorHelper(info, scope, value, attr);
return buildDataDescriptor(scope, value, attr);
}
if (prototypeValues != null) {
int id = prototypeValues.findId(name);
if (id != 0) {
value = prototypeValues.get(id);
attr = prototypeValues.getAttributes(id);
return buildDataDescriptorHelper(info, scope, value, attr);
return buildDataDescriptor(scope, value, attr);
}
}
return null;
Expand All @@ -998,7 +983,7 @@ private ScriptableObject getBuiltInDescriptor(Symbol key) {
if (id != 0) {
value = prototypeValues.get(id);
attr = prototypeValues.getAttributes(id);
return buildDataDescriptorHelper(key, scope, value, attr);
return buildDataDescriptor(scope, value, attr);
}
}
return null;
Expand Down
22 changes: 2 additions & 20 deletions rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,39 +135,21 @@ public abstract class ScriptableObject

protected static ScriptableObject buildDataDescriptor(
Scriptable scope, Object value, int attributes) {
return buildDataDescriptor(scope, value, attributes, null, -1);
}

protected static ScriptableObject buildDataDescriptor(
Scriptable scope, Object value, int attributes, String name, int length) {
ScriptableObject desc = new NativeObject();
ScriptRuntime.setBuiltinProtoAndParent(desc, scope, TopLevel.Builtins.Object);
desc.defineProperty("value", value, EMPTY);
desc.setCommonDescriptorProperties(attributes, true, name, length);
desc.setCommonDescriptorProperties(attributes, true);
return desc;
}

protected void setCommonDescriptorProperties(
int attributes, boolean defineWritable, String name, int length) {
if (name != null) {
defineProperty("name", "[Symbol.hasInstance]", attributes);
}

if (length != -1) {
defineProperty("length", length, attributes);
}

protected void setCommonDescriptorProperties(int attributes, boolean defineWritable) {
if (defineWritable) {
defineProperty("writable", (attributes & READONLY) == 0, EMPTY);
}
defineProperty("enumerable", (attributes & DONTENUM) == 0, EMPTY);
defineProperty("configurable", (attributes & PERMANENT) == 0, EMPTY);
}

protected void setCommonDescriptorProperties(int attributes, boolean defineWritable) {
setCommonDescriptorProperties(attributes, defineWritable, null, -1);
}

static void checkValidAttributes(int attributes) {
final int mask = READONLY | DONTENUM | PERMANENT | UNINITIALIZED_CONST;
if ((attributes & ~mask) != 0) {
Expand Down
117 changes: 13 additions & 104 deletions rhino/src/test/java/org/mozilla/javascript/FunctionPrototypeSymbolHasInstanceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,7 @@ public void testSymbolHasInstanceIsPresent() {
+ "};\n"
+ "var g = {};\n"
+ "`${f.hasOwnProperty(Symbol.hasInstance)}:${g.hasOwnProperty(Symbol.hasInstance)}`";
Utils.runWithAllOptimizationLevels(
(cx) -> {
cx.setLanguageVersion(Context.VERSION_ES6);
final Scriptable scope = cx.initStandardObjects();
String result =
(String)
cx.evaluateString(
scope, script, "testSymbolHasInstance", 0, null);
Assert.assertEquals("true:false", result);
return null;
});
Utils.assertWithAllOptimizationLevelsES6("true:false", script);
}

@Test
Expand All @@ -39,15 +29,7 @@ public void testSymbolHasInstanceCanBeCalledLikeAnotherMethod() {
+ " }"
+ "};\n"
+ "f[Symbol.hasInstance]() == 42";
Utils.runWithAllOptimizationLevels(
(cx) -> {
cx.setLanguageVersion(Context.VERSION_ES6);
final Scriptable scope = cx.initStandardObjects();
Object result =
cx.evaluateString(scope, script, "testSymbolHasInstance", 0, null);
Assert.assertEquals(true, result);
return null;
});
Utils.assertWithAllOptimizationLevelsES6(true, script);
}

// See: https://tc39.es/ecma262/#sec-function.prototype-%symbol.hasinstance%
Expand All @@ -56,15 +38,7 @@ public void testFunctionPrototypeSymbolHasInstanceHasAttributes() {
String script =
"var a = Object.getOwnPropertyDescriptor(Function.prototype, Symbol.hasInstance);\n"
+ "a.writable + ':' + a.configurable + ':' + a.enumerable";
Utils.runWithAllOptimizationLevels(
(cx) -> {
cx.setLanguageVersion(Context.VERSION_ES6);
final Scriptable scope = cx.initStandardObjects();
Object result =
cx.evaluateString(scope, script, "testSymbolHasInstance", 0, null);
Assert.assertEquals("false:false:false", result);
return null;
});
Utils.assertWithAllOptimizationLevelsES6("false:false:false", script);
}

// See: https://tc39.es/ecma262/#sec-function.prototype-%symbol.hasinstance%
Expand All @@ -81,15 +55,7 @@ public void testFunctionPrototypeSymbolHasInstanceHasAttributesStrictMode() {
+ " typeErrorThrown = true \n"
+ "}\n"
+ "Object.prototype.hasOwnProperty.call(Function.prototype, Symbol.hasInstance) + ':' + typeErrorThrown + ':' + t + ':' + a.writable + ':' + a.configurable + ':' + a.enumerable; \n";
Utils.runWithAllOptimizationLevels(
(cx) -> {
cx.setLanguageVersion(Context.VERSION_ES6);
final Scriptable scope = cx.initStandardObjects();
Object result =
cx.evaluateString(scope, script, "testSymbolHasInstance", 0, null);
Assert.assertEquals("true:true:function:false:false:false", result);
return null;
});
Utils.assertWithAllOptimizationLevelsES6("true:true:function:false:false:false", script);
}

@Test
Expand All @@ -102,34 +68,17 @@ public void testFunctionPrototypeSymbolHasInstanceHasProperties() {
String script2 =
"var a = Object.getOwnPropertyDescriptor(Function.prototype[Symbol.hasInstance], 'name');\n"
+ "a.value + ':' + a.writable + ':' + a.configurable + ':' + a.enumerable";
Utils.runWithAllOptimizationLevels(
(cx) -> {
cx.setLanguageVersion(Context.VERSION_ES6);
final Scriptable scope = cx.initStandardObjects();
Object result =
cx.evaluateString(scope, script, "testSymbolHasInstance", 0, null);
Object result2 =
cx.evaluateString(scope, script2, "testSymbolHasInstance", 0, null);
Assert.assertEquals("1:false:true:false", result);
Assert.assertEquals("Symbol(Symbol.hasInstance):false:true:false", result2);
return null;
});
Utils.assertWithAllOptimizationLevelsES6("1:false:true:false", script);
Utils.assertWithAllOptimizationLevelsES6(
"Symbol(Symbol.hasInstance):false:true:false", script2);
}

@Test
public void testFunctionPrototypeSymbolHasInstance() {
String script =
"(Function.prototype[Symbol.hasInstance] instanceof Function) + ':' + "
+ "Function.prototype[Symbol.hasInstance].call(Function, Object)\n";
Utils.runWithAllOptimizationLevels(
(cx) -> {
cx.setLanguageVersion(Context.VERSION_ES6);
final Scriptable scope = cx.initStandardObjects();
Object result =
cx.evaluateString(scope, script, "testSymbolHasInstance", 0, null);
Assert.assertEquals("true:true", result);
return null;
});
Utils.assertWithAllOptimizationLevelsES6("true:true", script);
}

@Test
Expand All @@ -140,15 +89,7 @@ public void testFunctionPrototypeSymbolHasInstanceOnObjectReturnsTrue() {
+ "var o2 = Object.create(o);\n"
+ "(f[Symbol.hasInstance](o)) + ':' + "
+ "(f[Symbol.hasInstance](o2));\n";
Utils.runWithAllOptimizationLevels(
(cx) -> {
cx.setLanguageVersion(Context.VERSION_ES6);
final Scriptable scope = cx.initStandardObjects();
Object result =
cx.evaluateString(scope, script, "testSymbolHasInstance", 0, null);
Assert.assertEquals("true:true", result);
return null;
});
Utils.assertWithAllOptimizationLevelsES6("true:true", script);
}

@Test
Expand All @@ -158,15 +99,7 @@ public void testFunctionPrototypeSymbolHasInstanceOnBoundTargetReturnsTrue() {
+ "var bc = new BC();\n"
+ "var bound = BC.bind();\n"
+ "bound[Symbol.hasInstance](bc);\n";
Utils.runWithAllOptimizationLevels(
(cx) -> {
cx.setLanguageVersion(Context.VERSION_ES6);
final Scriptable scope = cx.initStandardObjects();
Object result =
cx.evaluateString(scope, script, "testSymbolHasInstance", 0, null);
Assert.assertEquals(true, result);
return null;
});
Utils.assertWithAllOptimizationLevelsES6(true, script);
}

@Test
Expand All @@ -179,31 +112,15 @@ public void testFunctionInstanceNullVoidEtc() {
+ "(null instanceof f) + ':' +\n"
+ "(void 0 instanceof f)\n"
+ "a";
Utils.runWithAllOptimizationLevels(
(cx) -> {
cx.setLanguageVersion(Context.VERSION_ES6);
final Scriptable scope = cx.initStandardObjects();
Object result =
cx.evaluateString(scope, script, "testSymbolHasInstance", 0, null);
Assert.assertEquals("false:false:false:false", result);
return null;
});
Utils.assertWithAllOptimizationLevelsES6("false:false:false:false", script);
}

@Test
public void testFunctionPrototypeSymbolHasInstanceReturnsFalseOnUndefinedOrProtoypeNotFound() {
String script =
"Function.prototype[Symbol.hasInstance].call() + ':' +"
+ "Function.prototype[Symbol.hasInstance].call({});";
Utils.runWithAllOptimizationLevels(
(cx) -> {
cx.setLanguageVersion(Context.VERSION_ES6);
final Scriptable scope = cx.initStandardObjects();
Object result =
cx.evaluateString(scope, script, "testSymbolHasInstance", 0, null);
Assert.assertEquals("false:false", result);
return null;
});
Utils.assertWithAllOptimizationLevelsES6("false:false", script);
}

@Test
Expand All @@ -221,15 +138,7 @@ public void testSymbolHasInstanceIsInvokedInInstanceOf() {
+ "Object.setPrototypeOf(g, f);\n"
+ "g instanceof f;"
+ "globalSet == 1";
Utils.runWithAllOptimizationLevels(
(cx) -> {
cx.setLanguageVersion(Context.VERSION_ES6);
final Scriptable scope = cx.initStandardObjects();
Object result =
cx.evaluateString(scope, script, "testSymbolHasInstance", 0, null);
Assert.assertEquals(true, result);
return null;
});
Utils.assertWithAllOptimizationLevelsES6(true, script);
}

@Test
Expand Down

0 comments on commit 013b02b

Please sign in to comment.