Bug 18634 – SQUIRRELFISH: correctly handle variable and function declarations in eval code

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Cameron Zwarich (cpst)]

SquirrelFish currently doesn't correctly handle variable and function declarations in eval code. The problems are as follows:

1) Variables that are declared in eval code are not initialized to undefined at the beginning of the eval code block. This affects at least the following tests, in the simple form of a "var MYVAR;" declaration:

ecma/Expressions/11.3.1.js
ecma/Expressions/11.3.2.js
ecma/Expressions/11.4.4.js
ecma/Expressions/11.4.5.js

2) Variables defined in eval code are always initialized in global scope. This is a side effect of the above problem. If the first problem was fixed and undefined was stored in the slot of each new variable, then the code emitted for AssignResolveNode would find the right base for the variable and update that object correctly. We need to be careful here that we always use the right variable object, which is the activation object of the calling function in the case of direct eval inside of a function body, and the global object otherwise. We can't just grab the top of the scope chain thanks to 'with', and we no longer correctly track variable objects in ExecStates, but we should still be able to determine whether or not we are global code and grab the activation object of the calling function via callFrame[Machine::OptionalCalleeActivation] if we are not.

3) Function declarations in eval code don't work at all. I think this will be easily fixable once we fix the other two problems.

[Cameron Zwarich (cpst)]

Created attachment 20714 [details]
Proposed patch for variable declarations

Here is a patch that adds support for variable declarations but not function declarations.

[Maciej Stachowiak]

Comment on attachment 20714 [details]
Proposed patch for variable declarations

r=me

[Cameron Zwarich (cpst)]

Created attachment 20716 [details]
Revised proposed patch

Made a minor change as per Maciej's suggestion on IRC. Strangely enough, this is a 1.5% performance progression on SunSpider.

[Maciej Stachowiak]

Comment on attachment 20716 [details]
Revised proposed patch

r=me

[Cameron Zwarich (cpst)]

Created attachment 20718 [details]
Revised proposed patch

It turns out that the cause of the speedup was a GC bug in string-tagcloud, where the EvalCodeBlocks were not getting marked. Here is a patch that fixes the GC bug by treating EvalCodeBlocks like ProgramCodeBlocks. This time there is no difference on SunSpider.

[Maciej Stachowiak]

Comment on attachment 20718 [details]
Revised proposed patch

r=me

[Cameron Zwarich (cpst)]

This bug isn't quite fixed yet. The patch to support variable declarations landed in r32290, but there are still function declarations to support. I'll hopefully upload a patch soon.

[Cameron Zwarich (cpst)]

Created attachment 20761 [details]
Proposed patch implementing function declarations in eval code

This patch implements function declarations in eval code. In the process, it fixes 12 JavaScriptCore test failures.

Should declaredVariables and declaredFunctions be declaredVariableIdentifiers and declaredFunctionIdentifiers?

[Cameron Zwarich (cpst)]

Created attachment 20762 [details]
Revised proposed patch

Here is the same patch with a slight change in naming to make things more clear. SunSpider reports a 0.5% performance progression from ToT, which is mostly concentrated on the math tests.

[Maciej Stachowiak]

Comment on attachment 20762 [details]
Revised proposed patch

r=me