Skip to content

Commit 0f617f9

Browse files
gbrailgbrail
authored
Adjust order of evaluation of function arguments to match the spec
Adjust order of evaluation of function arguments to match the spec The spec is very consistent about the order in which function arguments are evaluated in cases where the function itself is not found, or is not a function object. This PR changes that ordering to match the standard. To implement this, the "lastStoredScriptable" in the Context is replaced with a temporary object used to return state whenever a name or property is looked up. We also simplified the bytecode generation code for calling functions, eliminating a lot of special cases that don't seem to help with performance any more.
1 parent 95206bc commit 0f617f9

12 files changed

Lines changed: 604 additions & 337 deletions

File tree

‎rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1258,7 +1258,6 @@ private CompleteOptionalCallJump completeOptionalCallJump() {
12581258

12591259
// Put undefined
12601260
addIcode(Icode_POP);
1261-
addIcode(Icode_POP);
12621261
addStringOp(Token.NAME, "undefined");
12631262
int afterLabel = iCodeTop;
12641263
addGotoOp(Token.GOTO);

‎rhino/src/main/java/org/mozilla/javascript/Interpreter.java‎

Lines changed: 45 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1851,19 +1851,14 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl
18511851
// stringReg: name
18521852
++stackTop;
18531853
stack[stackTop] =
1854-
ScriptRuntime.getNameFunctionAndThis(
1855-
stringReg, cx, frame.scope);
1856-
++stackTop;
1857-
stack[stackTop] = ScriptRuntime.lastStoredScriptable(cx);
1854+
ScriptRuntime.getNameAndThis(stringReg, cx, frame.scope);
18581855
continue Loop;
18591856
case Icode_NAME_AND_THIS_OPTIONAL:
18601857
// stringReg: name
18611858
++stackTop;
18621859
stack[stackTop] =
1863-
ScriptRuntime.getNameFunctionAndThisOptional(
1860+
ScriptRuntime.getNameAndThisOptional(
18641861
stringReg, cx, frame.scope);
1865-
++stackTop;
1866-
stack[stackTop] = ScriptRuntime.lastStoredScriptable(cx);
18671862
continue Loop;
18681863
case Icode_PROP_AND_THIS:
18691864
{
@@ -1872,10 +1867,8 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl
18721867
obj = ScriptRuntime.wrapNumber(sDbl[stackTop]);
18731868
// stringReg: property
18741869
stack[stackTop] =
1875-
ScriptRuntime.getPropFunctionAndThis(
1870+
ScriptRuntime.getPropAndThis(
18761871
obj, stringReg, cx, frame.scope);
1877-
++stackTop;
1878-
stack[stackTop] = ScriptRuntime.lastStoredScriptable(cx);
18791872
continue Loop;
18801873
}
18811874
case Icode_PROP_AND_THIS_OPTIONAL:
@@ -1885,10 +1878,8 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl
18851878
obj = ScriptRuntime.wrapNumber(sDbl[stackTop]);
18861879
// stringReg: property
18871880
stack[stackTop] =
1888-
ScriptRuntime.getPropFunctionAndThisOptional(
1881+
ScriptRuntime.getPropAndThisOptional(
18891882
obj, stringReg, cx, frame.scope);
1890-
++stackTop;
1891-
stack[stackTop] = ScriptRuntime.lastStoredScriptable(cx);
18921883
continue Loop;
18931884
}
18941885
case Icode_ELEM_AND_THIS:
@@ -1899,10 +1890,9 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl
18991890
Object id = stack[stackTop];
19001891
if (id == DBL_MRK)
19011892
id = ScriptRuntime.wrapNumber(sDbl[stackTop]);
1902-
stack[stackTop - 1] =
1903-
ScriptRuntime.getElemFunctionAndThis(
1904-
obj, id, cx, frame.scope);
1905-
stack[stackTop] = ScriptRuntime.lastStoredScriptable(cx);
1893+
stackTop--;
1894+
stack[stackTop] =
1895+
ScriptRuntime.getElemAndThis(obj, id, cx, frame.scope);
19061896
continue Loop;
19071897
}
19081898
case Icode_ELEM_AND_THIS_OPTIONAL:
@@ -1913,21 +1903,18 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl
19131903
Object id = stack[stackTop];
19141904
if (id == DBL_MRK)
19151905
id = ScriptRuntime.wrapNumber(sDbl[stackTop]);
1916-
stack[stackTop - 1] =
1917-
ScriptRuntime.getElemFunctionAndThisOptional(
1906+
stackTop--;
1907+
stack[stackTop] =
1908+
ScriptRuntime.getElemAndThisOptional(
19181909
obj, id, cx, frame.scope);
1919-
stack[stackTop] = ScriptRuntime.lastStoredScriptable(cx);
19201910
continue Loop;
19211911
}
19221912
case Icode_VALUE_AND_THIS:
19231913
{
19241914
Object value = stack[stackTop];
19251915
if (value == DBL_MRK)
19261916
value = ScriptRuntime.wrapNumber(sDbl[stackTop]);
1927-
stack[stackTop] =
1928-
ScriptRuntime.getValueFunctionAndThis(value, cx);
1929-
++stackTop;
1930-
stack[stackTop] = ScriptRuntime.lastStoredScriptable(cx);
1917+
stack[stackTop] = ScriptRuntime.getValueAndThis(value, cx);
19311918
continue Loop;
19321919
}
19331920
case Icode_VALUE_AND_THIS_OPTIONAL:
@@ -1936,10 +1923,7 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl
19361923
if (value == DBL_MRK)
19371924
value = ScriptRuntime.wrapNumber(sDbl[stackTop]);
19381925
stack[stackTop] =
1939-
ScriptRuntime.getValueFunctionAndThisOptional(
1940-
value, cx);
1941-
++stackTop;
1942-
stack[stackTop] = ScriptRuntime.lastStoredScriptable(cx);
1926+
ScriptRuntime.getValueAndThisOptional(value, cx);
19431927
continue Loop;
19441928
}
19451929
case Icode_CALLSPECIAL:
@@ -2803,27 +2787,28 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl
28032787
: ScriptRuntime.wrapNumber(interpreterResultDbl);
28042788
}
28052789

2806-
private static final NewState doCallByteCode(
2790+
private static NewState doCallByteCode(
28072791
Context cx,
28082792
CallFrame frame,
28092793
boolean instructionCounting,
28102794
int op,
28112795
int stackTop,
28122796
int indexReg) {
2797+
28132798
Object[] stack = frame.stack;
28142799
double[] sDbl = frame.sDbl;
28152800

28162801
if (instructionCounting) {
28172802
cx.instructionCount += INVOCATION_COST;
28182803
}
2819-
// stack change: function thisObj arg0 .. argN -> result
2804+
// stack change: lookup_result arg0 .. argN -> result
28202805
// indexReg: number of arguments
2821-
stackTop -= 1 + indexReg;
2822-
2823-
// CALL generation ensures that fun and funThisObj
2824-
// are already Scriptable and Callable objects respectively
2825-
Callable fun = (Callable) stack[stackTop];
2826-
Scriptable funThisObj = (Scriptable) stack[stackTop + 1];
2806+
stackTop -= indexReg;
2807+
ScriptRuntime.LookupResult result = (ScriptRuntime.LookupResult) stack[stackTop];
2808+
// Check if the lookup result is a function and throw if it's not
2809+
// must not be done sooner according to the spec
2810+
Callable fun = result.getCallable();
2811+
Scriptable funThisObj = result.getThis();
28272812
Scriptable funHomeObj =
28282813
(fun instanceof BaseFunction) ? ((BaseFunction) fun).getHomeObject() : null;
28292814
if (op == Icode_CALL_ON_SUPER) {
@@ -2835,7 +2820,7 @@ private static final NewState doCallByteCode(
28352820
}
28362821

28372822
if (op == Token.REF_CALL) {
2838-
Object[] outArgs = getArgsArray(stack, sDbl, stackTop + 2, indexReg);
2823+
Object[] outArgs = getArgsArray(stack, sDbl, stackTop + 1, indexReg);
28392824
stack[stackTop] =
28402825
ScriptRuntime.callRef(
28412826
fun, funThisObj,
@@ -2868,28 +2853,28 @@ private static final NewState doCallByteCode(
28682853
// funThisObj becomes fun
28692854
fun = ScriptRuntime.getCallable(funThisObj);
28702855
// first arg becomes thisObj
2871-
funThisObj = getApplyThis(cx, stack, sDbl, stackTop + 2, indexReg, fun, frame);
2856+
funThisObj = getApplyThis(cx, stack, sDbl, stackTop + 1, indexReg, fun, frame);
28722857
if (BaseFunction.isApply(kfun)) {
28732858
// Apply: second argument after new "this"
28742859
// should be array-like
28752860
// and we'll spread its elements on the stack
28762861
Object[] callArgs =
28772862
indexReg < 2
28782863
? ScriptRuntime.emptyArgs
2879-
: ScriptRuntime.getApplyArguments(cx, stack[stackTop + 3]);
2864+
: ScriptRuntime.getApplyArguments(cx, stack[stackTop + 2]);
28802865
int alen = callArgs.length;
2881-
stack = frame.ensureStackLength(alen + stackTop + 2);
2866+
stack = frame.ensureStackLength(alen + stackTop + 1);
28822867
sDbl = frame.sDbl;
2883-
System.arraycopy(callArgs, 0, stack, stackTop + 2, alen);
2868+
System.arraycopy(callArgs, 0, stack, stackTop + 1, alen);
28842869
indexReg = alen;
28852870
} else {
28862871
// Call: shift args left, starting from 2nd
28872872
if (indexReg > 0) {
28882873
if (indexReg > 1) {
28892874
System.arraycopy(
2890-
stack, stackTop + 3, stack, stackTop + 2, indexReg - 1);
2875+
stack, stackTop + 2, stack, stackTop + 1, indexReg - 1);
28912876
System.arraycopy(
2892-
sDbl, stackTop + 3, sDbl, stackTop + 2, indexReg - 1);
2877+
sDbl, stackTop + 2, sDbl, stackTop + 1, indexReg - 1);
28932878
}
28942879
indexReg--;
28952880
}
@@ -2910,24 +2895,24 @@ private static final NewState doCallByteCode(
29102895
Object[] boundArgs = bfun.getBoundArgs();
29112896
int blen = boundArgs.length;
29122897
if (blen > 0) {
2913-
stack = frame.ensureStackLength(blen + stackTop + 2 + indexReg);
2898+
stack = frame.ensureStackLength(blen + stackTop + 1 + indexReg);
29142899
sDbl = frame.sDbl;
2915-
System.arraycopy(stack, stackTop + 2, stack, stackTop + 2 + blen, indexReg);
2916-
System.arraycopy(sDbl, stackTop + 2, sDbl, stackTop + 2 + blen, indexReg);
2917-
System.arraycopy(boundArgs, 0, stack, stackTop + 2, blen);
2900+
System.arraycopy(stack, stackTop + 1, stack, stackTop + 1 + blen, indexReg);
2901+
System.arraycopy(sDbl, stackTop + 1, sDbl, stackTop + 1 + blen, indexReg);
2902+
System.arraycopy(boundArgs, 0, stack, stackTop + 1, blen);
29182903
indexReg += blen;
29192904
}
29202905
} else if (fun instanceof NoSuchMethodShim) {
29212906
NoSuchMethodShim nsmfun = (NoSuchMethodShim) fun;
29222907
// Bug 447697 -- make best effort to keep
29232908
// __noSuchMethod__ within this interpreter loop
29242909
// invocation.
2925-
stack = frame.ensureStackLength(stackTop + 4);
2910+
stack = frame.ensureStackLength(stackTop + 3);
29262911
sDbl = frame.sDbl;
2927-
Object[] elements = getArgsArray(stack, sDbl, stackTop + 2, indexReg);
2912+
Object[] elements = getArgsArray(stack, sDbl, stackTop + 1, indexReg);
29282913
fun = nsmfun.noSuchMethodMethod;
2929-
stack[stackTop + 2] = nsmfun.methodName;
2930-
stack[stackTop + 3] = cx.newArray(calleeScope, elements);
2914+
stack[stackTop + 1] = nsmfun.methodName;
2915+
stack[stackTop + 2] = cx.newArray(calleeScope, elements);
29312916
indexReg = 2;
29322917
} else if (fun == null) {
29332918
throw ScriptRuntime.notFunctionError(null, null);
@@ -2974,7 +2959,7 @@ private static final NewState doCallByteCode(
29742959
funHomeObj,
29752960
stack,
29762961
sDbl,
2977-
stackTop + 2,
2962+
stackTop + 1,
29782963
indexReg,
29792964
ifun,
29802965
callParentFrame);
@@ -2996,8 +2981,8 @@ private static final NewState doCallByteCode(
29962981
if (indexReg == 0) {
29972982
cjump.result = undefined;
29982983
} else {
2999-
cjump.result = stack[stackTop + 2];
3000-
cjump.resultDbl = sDbl[stackTop + 2];
2984+
cjump.result = stack[stackTop + 1];
2985+
cjump.resultDbl = sDbl[stackTop + 1];
30012986
}
30022987

30032988
// Start the real unwind job
@@ -3020,7 +3005,7 @@ private static final NewState doCallByteCode(
30203005
cx,
30213006
calleeScope,
30223007
funThisObj,
3023-
getArgsArray(stack, sDbl, stackTop + 2, indexReg));
3008+
getArgsArray(stack, sDbl, stackTop + 1, indexReg));
30243009

30253010
return new ContinueLoop(frame, stackTop, indexReg);
30263011
}
@@ -3262,18 +3247,18 @@ private static int doCallSpecial(
32623247
ScriptRuntime.newSpecial(cx, function, outArgs, frame.scope, callType);
32633248
} else {
32643249
// stack change: function thisObj arg0 .. argN -> result
3265-
stackTop -= 1 + indexReg;
3250+
stackTop -= indexReg;
32663251

32673252
// Call code generation ensure that stack here
32683253
// is ... Callable Scriptable
3269-
Scriptable functionThis = (Scriptable) stack[stackTop + 1];
3270-
Callable function = (Callable) stack[stackTop];
3271-
Object[] outArgs = getArgsArray(stack, sDbl, stackTop + 2, indexReg);
3254+
ScriptRuntime.LookupResult result = (ScriptRuntime.LookupResult) stack[stackTop];
3255+
Object[] outArgs = getArgsArray(stack, sDbl, stackTop + 1, indexReg);
3256+
Callable function = result.getCallable();
32723257
stack[stackTop] =
32733258
ScriptRuntime.callSpecial(
32743259
cx,
32753260
function,
3276-
functionThis,
3261+
result.getThis(),
32773262
outArgs,
32783263
frame.scope,
32793264
frame.thisObj,

0 commit comments

Comments
 (0)