Skip to content

Commit

Permalink
FIX: Arguments with default parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xe
0xe committed Jun 10, 2024
1 parent 1d87e40 commit 44e544f
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 46 deletions.
8 changes: 8 additions & 0 deletions rhino-runtime/src/main/java/org/mozilla/javascript/Arguments.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,14 @@ private boolean sharedWithActivation(int index) {
return false;
}
NativeFunction f = activation.function;

// Check if default arguments are present
if (f instanceof InterpretedFunction && ((InterpretedFunction) f).idata.argsHasDefaults) {
return false;
} else if (f instanceof NativeFunction && f.hasDefaultParameters()) {
return false;
}

int definedCount = f.getParamCount();
if (index < definedCount) {
// Check if argument is not hidden by later argument with the same
Expand Down
5 changes: 5 additions & 0 deletions rhino-runtime/src/main/java/org/mozilla/javascript/BaseFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ protected boolean isGeneratorFunction() {
return isGeneratorFunction;
}

// Generated code will override this
protected boolean hasDefaultParameters() {
return false;
}

/**
* Gets the value returned by calling the typeof operator on this object.
*
Expand Down
1 change: 1 addition & 0 deletions rhino-runtime/src/main/java/org/mozilla/javascript/CodeGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ private void generateICodeFromTree(Node tree) {
itsData.argIsConst = scriptOrFn.getParamAndVarConst();
itsData.argCount = scriptOrFn.getParamCount();
itsData.argsHasRest = scriptOrFn.hasRestParameter();
itsData.argsHasDefaults = scriptOrFn.getDefaultParams() != null;

itsData.encodedSourceStart = scriptOrFn.getEncodedSourceStart();
itsData.encodedSourceEnd = scriptOrFn.getEncodedSourceEnd();
Expand Down
40 changes: 21 additions & 19 deletions rhino-runtime/src/main/java/org/mozilla/javascript/IRFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,25 +645,27 @@ private Node transformFunction(FunctionNode fn) {
if (fn.getDefaultParams() != null) {
Object[] defaultParams = fn.getDefaultParams();
for (int i = defaultParams.length - 1; i > 0; ) {
AstNode rhs = (AstNode) defaultParams[i];
String name = (String) defaultParams[i - 1];
/* TODO: default params - error handling */
body.addChildToFront(
createIf(
createBinary(
Token.SHEQ,
new Name(fn.getPosition(), name),
new Name(fn.getPosition(), "undefined")),
new Node(
Token.EXPR_VOID,
createAssignment(
Token.ASSIGN,
new Name(fn.getPosition(), name),
transform(rhs)),
body.getLineno()),
null,
body.getLineno()));
i -= 2;
if (defaultParams[i] instanceof AstNode
&& defaultParams[i - 1] instanceof String) {
AstNode rhs = (AstNode) defaultParams[i];
String name = (String) defaultParams[i - 1];
body.addChildToFront(
createIf(
createBinary(
Token.SHEQ,
new Name(fn.getPosition(), name),
new Name(fn.getPosition(), "undefined")),
new Node(
Token.EXPR_VOID,
createAssignment(
Token.ASSIGN,
new Name(fn.getPosition(), name),
transform(rhs)),
body.getLineno()),
null,
body.getLineno()));
i -= 2;
}
}
}

Expand Down
1 change: 1 addition & 0 deletions rhino-runtime/src/main/java/org/mozilla/javascript/InterpreterData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ private void init() {
boolean[] argIsConst;
int argCount;
boolean argsHasRest;
boolean argsHasDefaults;

int itsMaxCalleeArgs;

Expand Down
10 changes: 2 additions & 8 deletions rhino-runtime/src/main/java/org/mozilla/javascript/Parser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -862,10 +862,7 @@ && matchToken(Token.ASSIGN, true)) {
fnNode.setDefaultParams(existing);
current = existing;
} else {
current =
new Object
[existing.length
+ 2]; /* TODO: A more flexible structure? */
current = new Object[existing.length + 2];
System.arraycopy(existing, 0, current, 0, existing.length);
current_size = existing.length;
existing = null;
Expand Down Expand Up @@ -1114,11 +1111,9 @@ private void arrowFunctionParams(
fnNode.setDefaultParams(existing);
current = existing;
} else {
current =
new Object[existing.length + 2]; /* TODO: A more flexible structure? */
current = new Object[existing.length + 2];
System.arraycopy(existing, 0, current, 0, existing.length);
current_size = existing.length;
existing = null;
}
current[current_size] = paramName;
current[current.length - 1] = rhs;
Expand All @@ -1127,7 +1122,6 @@ private void arrowFunctionParams(
} else {
reportError("msg.no.parm", params.getPosition(), params.getLength());
fnNode.addParam(makeErrorNode());
return;
}
} else {
reportError("msg.no.parm", params.getPosition(), params.getLength());
Expand Down
1 change: 1 addition & 0 deletions rhino-runtime/src/main/java/org/mozilla/javascript/ast/FunctionNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public static enum Form {
private int rp = -1;
private boolean hasRestParameter;

@Override
public Object[] getDefaultParams() {
return defaultParams;
}
Expand Down
4 changes: 4 additions & 0 deletions rhino-runtime/src/main/java/org/mozilla/javascript/ast/ScriptNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,10 @@ public boolean hasRestParameter() {
return false;
}

public Object[] getDefaultParams() {
return null;
}

void addSymbol(Symbol symbol) {
if (variableNames != null) codeBug();
if (symbol.getDeclType() == Token.LP) {
Expand Down
16 changes: 15 additions & 1 deletion rhino-runtime/src/main/java/org/mozilla/javascript/optimizer/Codegen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,8 @@ private void generateNativeFunctionOverrides(ClassFileWriter cfw, String encoded
final int Do_getParamOrVarConst = 5;
final int Do_isGeneratorFunction = 6;
final int Do_hasRestParameter = 7;
final int SWITCH_COUNT = 8;
final int Do_hasDefaultParameters = 8;
final int SWITCH_COUNT = 9;

for (int methodIndex = 0; methodIndex != SWITCH_COUNT; ++methodIndex) {
if (methodIndex == Do_getEncodedSource && encodedSource == null) {
Expand Down Expand Up @@ -791,6 +792,10 @@ private void generateNativeFunctionOverrides(ClassFileWriter cfw, String encoded
methodLocals = 1; // Only this
cfw.startMethod("hasRestParameter", "()Z", ACC_PUBLIC);
break;
case Do_hasDefaultParameters:
methodLocals = 1; // Only this
cfw.startMethod("hasDefaultParameters", "()Z", ACC_PUBLIC);
break;
default:
throw Kit.codeBug();
}
Expand Down Expand Up @@ -935,6 +940,15 @@ private void generateNativeFunctionOverrides(ClassFileWriter cfw, String encoded
cfw.add(ByteCode.IRETURN);
break;

case Do_hasDefaultParameters:
if (n instanceof FunctionNode) {
cfw.addPush(n.getDefaultParams() != null);
} else {
cfw.addPush(false);
}
cfw.add(ByteCode.IRETURN);
break;

case Do_getEncodedSource:
// Push number encoded source start and end
// to prepare for encodedSource.substring(start, end)
Expand Down
39 changes: 23 additions & 16 deletions tests/src/test/java/org/mozilla/javascript/tests/DefaultParametersTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.mozilla.javascript.Context;
import org.mozilla.javascript.EcmaError;
import org.mozilla.javascript.Scriptable;
import org.mozilla.javascript.Undefined;

/*
Many of these are taken from examples at developer.mozilla.org
Expand Down Expand Up @@ -106,7 +107,6 @@ public void destructuringAssigmentDefaultObject() throws Exception {
}

@Test
@Ignore("argument-object-should-not-be-mutated")
public void defaultParametersWithArgumentsObject() throws Exception {
final String script =
"function f(a = 55) {\n"
Expand All @@ -124,8 +124,11 @@ public void defaultParametersWithArgumentsObject() throws Exception {
+ " return arguments.length;\n"
+ "}\n";
assertIntEvaluates(10, script + "f(10)");
assertIntEvaluates(99, script + "g(10)");
assertIntEvaluates(55, script + "f()");
assertIntEvaluates(10, script + "g(10)");
assertEvaluates(Undefined.instance, script + "g()");
assertIntEvaluates(0, script + "h()");
assertIntEvaluates(1, script + "h(10)");
}

@Test
Expand Down Expand Up @@ -184,25 +187,29 @@ private static void assertIntEvaluates(final Object expected, final String sourc
assertIntEvaluatesWithLanguageLevel(expected, source, Context.VERSION_ES6);
}

private static void assertEvaluates(final Object expected, final String source) {
assertIntEvaluatesWithLanguageLevel(expected, source, Context.VERSION_ES6);
}

private static void assertIntEvaluatesWithLanguageLevel(
final Object expected, final String source, int languageLevel) {
Utils.runWithAllOptimizationLevels(
cx -> {
if (cx.getOptimizationLevel() == -1) {
int oldVersion = cx.getLanguageVersion();
cx.setLanguageVersion(languageLevel);
try {
final Scriptable scope = cx.initStandardObjects();
final Object rep = cx.evaluateString(scope, source, "test.js", 0, null);
if (rep instanceof Double)
assertEquals((int) expected, ((Double) rep).intValue());
else assertEquals(expected, rep);
return null;
} finally {
cx.setLanguageVersion(oldVersion);
}
// if (cx.getOptimizationLevel() == 0) {
int oldVersion = cx.getLanguageVersion();
cx.setLanguageVersion(languageLevel);
try {
final Scriptable scope = cx.initStandardObjects();
final Object rep = cx.evaluateString(scope, source, "test.js", 0, null);
if (rep instanceof Double)
assertEquals((int) expected, ((Double) rep).intValue());
else assertEquals(expected, rep);
return null;
} finally {
cx.setLanguageVersion(oldVersion);
}
return null;
// }
// return null;
});
}
}
3 changes: 1 addition & 2 deletions tests/testsrc/test262.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2424,7 +2424,7 @@ built-ins/WeakMap 1/88 (1.14%)
built-ins/WeakSet 1/75 (1.33%)
proto-from-ctor-realm.js {unsupported: [Reflect]}

language/arguments-object 189/260 (72.69%)
language/arguments-object 188/260 (72.31%)
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 Expand Up @@ -2460,7 +2460,6 @@ language/arguments-object 189/260 (72.69%)
mapped/nonwritable-nonenumerable-nonconfigurable-descriptors-basic.js non-strict
mapped/nonwritable-nonenumerable-nonconfigurable-descriptors-set-by-arguments.js non-strict
mapped/nonwritable-nonenumerable-nonconfigurable-descriptors-set-by-param.js non-strict
unmapped/via-params-dflt.js non-strict
unmapped/via-params-dstr.js non-strict
unmapped/via-params-rest.js non-strict
arguments-caller.js
Expand Down

0 comments on commit 44e544f

Please sign in to comment.