Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
- Remove unnecessary code
- Simplify implementation to use execIdCall as BaseFunction inherits from IdScriptableObject
  • Loading branch information
0xe committed Jan 3, 2025
1 parent 74b1fd3 commit ce2c9af
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 70 deletions.
76 changes: 25 additions & 51 deletions rhino/src/main/java/org/mozilla/javascript/BaseFunction.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,7 @@ static void init(Context cx, Scriptable scope, boolean sealed) {
if (cx.getLanguageVersion() >= Context.VERSION_ES6) {
obj.setStandardPropertyAttributes(READONLY | DONTENUM);
}
IdFunctionObject constructor = obj.exportAsJSClass(MAX_PROTOTYPE_ID, scope, sealed);
if (cx.getLanguageVersion() >= Context.VERSION_ES6) {
ScriptRuntimeES6.addSymbolHasInstance(cx, scope, constructor);
}
obj.exportAsJSClass(MAX_PROTOTYPE_ID, scope, sealed);
}

/**
Expand Down Expand Up @@ -271,7 +268,13 @@ protected void fillConstructorProperties(IdFunctionObject ctor) {
@Override
protected void initPrototypeId(int id) {
if (id == SymbolId_hasInstance) {
initPrototypeValue(id, SymbolKey.HAS_INSTANCE, makeHasInstance(), CONST | DONTENUM);
initPrototypeMethod(
FUNCTION_TAG,
id,
SymbolKey.HAS_INSTANCE,
String.valueOf(SymbolKey.HAS_INSTANCE),
1,
CONST | DONTENUM);
return;
}

Expand Down Expand Up @@ -323,52 +326,6 @@ static boolean isApplyOrCall(IdFunctionObject f) {
return false;
}

private Object makeHasInstance() {
Context cx = Context.getCurrentContext();
ScriptableObject obj = null;

if (cx != null) {
Scriptable scope = this.getParentScope();
obj =
new LambdaFunction(
scope,
0,
new Callable() {
@Override
public Object call(
Context cx,
Scriptable scope,
Scriptable thisObj,
Object[] args) {
if (thisObj != null
&& args.length == 1
&& args[0] instanceof Scriptable) {
Scriptable obj = (Scriptable) args[0];
Object protoProp = null;
if (thisObj instanceof BoundFunction)
protoProp =
((NativeFunction)
((BoundFunction) thisObj)
.getTargetFunction())
.getPrototypeProperty();
else
protoProp =
ScriptableObject.getProperty(
thisObj, "prototype");
if (protoProp instanceof IdScriptableObject) {
return ScriptRuntime.jsDelegatesTo(
obj, (Scriptable) protoProp);
}
throw ScriptRuntime.typeErrorById(
"msg.instanceof.bad.prototype", getFunctionName());
}
return false; // NOT_FOUND, null etc.
}
});
}
return obj;
}

@Override
public Object execIdCall(
IdFunctionObject f, Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
Expand Down Expand Up @@ -424,6 +381,23 @@ public Object execIdCall(
boundArgs = ScriptRuntime.emptyArgs;
}
return new BoundFunction(cx, scope, targetFunction, boundThis, boundArgs);

case SymbolId_hasInstance:
if (thisObj != null && args.length == 1 && args[0] instanceof Scriptable) {
Scriptable obj = (Scriptable) args[0];
Object protoProp = null;
if (thisObj instanceof BoundFunction)
protoProp =
((NativeFunction) ((BoundFunction) thisObj).getTargetFunction())
.getPrototypeProperty();
else protoProp = ScriptableObject.getProperty(thisObj, "prototype");
if (protoProp instanceof IdScriptableObject) {
return ScriptRuntime.jsDelegatesTo(obj, (Scriptable) protoProp);
}
throw ScriptRuntime.typeErrorById(
"msg.instanceof.bad.prototype", getFunctionName());
}
return false; // NOT_FOUND, null etc.
}
throw new IllegalArgumentException(String.valueOf(id));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,14 @@ public final IdFunctionObject initPrototypeMethod(
return function;
}

public final IdFunctionObject initPrototypeMethod(
Object tag, int id, Symbol key, String functionName, int arity, int attributes) {
Scriptable scope = ScriptableObject.getTopLevelScope(this);
IdFunctionObject function = newIdFunction(tag, id, functionName, arity, scope);
prototypeValues.initValue(id, key, function, attributes);
return function;
}

public final void initPrototypeConstructor(IdFunctionObject f) {
int id = prototypeValues.constructorId;
if (id == 0) throw new IllegalStateException();
Expand Down
11 changes: 0 additions & 11 deletions rhino/src/main/java/org/mozilla/javascript/ScriptRuntimeES6.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,4 @@ public static void addSymbolUnscopables(
ScriptableObject.putProperty(unScopablesDescriptor, "writable", false);
constructor.defineOwnProperty(cx, SymbolKey.UNSCOPABLES, unScopablesDescriptor, false);
}

/** Registers the symbol <code>[Symbol.hasInstance]</code> on the given constructor function. */
public static void addSymbolHasInstance(
Context cx, Scriptable scope, IdScriptableObject constructor) {
ScriptableObject hasInstanceDescriptor = (ScriptableObject) cx.newObject(scope);
ScriptableObject.putProperty(hasInstanceDescriptor, "value", ScriptableObject.EMPTY);
ScriptableObject.putProperty(hasInstanceDescriptor, "enumerable", false);
ScriptableObject.putProperty(hasInstanceDescriptor, "configurable", false);
ScriptableObject.putProperty(hasInstanceDescriptor, "writable", false);
constructor.defineOwnProperty(cx, SymbolKey.HAS_INSTANCE, hasInstanceDescriptor, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -867,8 +867,8 @@ public boolean hasInstance(Scriptable instance) {
Context cx = Context.getCurrentContext();
Object hasInstance = ScriptRuntime.getObjectElem(this, SymbolKey.HAS_INSTANCE, cx);
if (hasInstance instanceof Callable) {
return (boolean)
((Callable) hasInstance).call(cx, getParentScope(), this, new Object[] {this});
return ScriptRuntime.toBoolean(
((Callable) hasInstance).call(cx, getParentScope(), this, new Object[] {this}));
}
return ScriptRuntime.jsDelegatesTo(instance, this);
}
Expand Down
10 changes: 4 additions & 6 deletions tests/testsrc/test262.properties
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ built-ins/Error 5/41 (12.2%)

~built-ins/FinalizationRegistry

built-ins/Function 184/508 (36.22%)
built-ins/Function 175/508 (34.45%)
internals/Call 2/2 (100.0%)
internals/Construct 6/6 (100.0%)
length/S15.3.5.1_A1_T3.js strict
Expand Down Expand Up @@ -719,9 +719,7 @@ built-ins/Function 184/508 (36.22%)
prototype/call/S15.3.4.4_A6_T2.js compiled
prototype/call/S15.3.4.4_A6_T5.js compiled
prototype/call/S15.3.4.4_A6_T7.js compiled
prototype/Symbol.hasInstance/length.js
prototype/Symbol.hasInstance/name.js
prototype/Symbol.hasInstance/value-get-prototype-of-err.js {unsupported: [Proxy]}
prototype/toString/async-arrow-function.js {unsupported: [async-functions]}
prototype/toString/async-function-declaration.js {unsupported: [async-functions]}
prototype/toString/async-function-expression.js {unsupported: [async-functions]}
Expand Down Expand Up @@ -950,7 +948,7 @@ built-ins/Number 23/335 (6.87%)
S9.3.1_A3_T1_U180E.js {unsupported: [u180e]}
S9.3.1_A3_T2_U180E.js {unsupported: [u180e]}

built-ins/Object 178/3408 (5.22%)
built-ins/Object 177/3408 (5.19%)
assign/assignment-to-readonly-property-of-target-must-throw-a-typeerror-exception.js
assign/not-a-constructor.js
assign/source-own-prop-error.js
Expand Down Expand Up @@ -1523,7 +1521,7 @@ built-ins/Promise 392/631 (62.12%)
resolve-thenable-deferred.js {unsupported: [async]}
resolve-thenable-immed.js {unsupported: [async]}

built-ins/Proxy 76/311 (24.44%)
built-ins/Proxy 73/311 (23.47%)
construct/arguments-realm.js
construct/call-parameters.js
construct/call-parameters-new-target.js
Expand Down Expand Up @@ -3139,7 +3137,7 @@ built-ins/undefined 0/8 (0.0%)

~intl402

language/arguments-object 185/263 (70.34%)
language/arguments-object 184/263 (69.96%)
mapped/mapped-arguments-nonconfigurable-3.js non-strict
mapped/mapped-arguments-nonconfigurable-delete-1.js non-strict
mapped/mapped-arguments-nonconfigurable-delete-2.js non-strict
Expand Down

0 comments on commit ce2c9af

Please sign in to comment.