[regression] eval() is broken: it is disconnected from the surrounding context

[yurivict]

The testcase

function func(x) {
        var fnBody = "arg+0.1";
        eval("function calc (arg) {return Math.floor("+fnBody+");}");
        print("calc("+x+")="+calc(x));
}
func(0.9)

worked in 8c86834 but now in fe71080 breaks with unknown symbol 'calc'.

[yurivict]

I bisected, and found that this commit broke it: d248b0ce1800a1ebf2c853f205c1947642185c6a (Bug 701886: Always create new scope for eval(). )

JavaScript's eval() function should have access to its context. See this browser JS example: https://www.w3schools.com/jsref/jsref_eval.asp

[yurivict]

If it is important to have eval running in its own context a build-time or run-time option can be introduced to control this.

[yurivict]

Suggested solution: https://github.com/ccxvii/mujs/pull/140

[aksyom]

I would like to add some context to this regression. I was the one who first reported a peculiar behaviour in eval(); namely, it would set variable to undefined if it was redeclared with 'var' inside an eval string. I made this bug report from here: https://bugs.ghostscript.com/show_bug.cgi?id=701886

Now, I have also realized the "fix", ie. create new closure for eval, is not the right way to fix this. The way I would have wanted eval() to be fixed is for it to work exactly how it works by the standard. And by that, I mean how it works in just about any other Javascript runtime: eval'd code is evaluated in the parent context, AS IF the code was evaluated in-line. Does that make sense?

I suggest not adding any extra options to either enable or disable the creation of new context for eval, because that will NOT fix the original issue, where eval would not behave as it should! Even if you disable the option of new context for eval, variables redefined inside the eval'd code would still be reset to undefined, because the root cause the problem, whatever it may be, would not have been fixed.

[ccxvii]

@aksyom Do the commits 9f34a074eb4f6a1ef4f27a85da65eb37f4149a87 and a34fdf2af87cc13b1d85cd19812c4d0b722f3e3a fix the issues you see with eval?