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 732d044
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 73 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
10 changes: 1 addition & 9 deletions rhino/src/test/java/org/mozilla/javascript/FunctionPrototypeSymbolHasInstanceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,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 Down

0 comments on commit 732d044

Please sign in to comment.