bellard / quickjs

Public repository of the QuickJS Javascript Engine.
https://bellard.org/quickjs
Other
8.28k stars 866 forks source link

A bug in implementation of `switch` statement. #130

Open AidPaike opened 1 year ago

AidPaike commented 1 year ago

Version

2022-03-07

Test case

var foo = function (n) {
    switch (n) {
        case 1:
            print(1);
            function f() { print('f1') }
            break;
        case 2:
            print(2);
            function f() { print('f2') }
            break;
    }
    f();
};
foo(1);

Execution Command

/root/.jsvu/quickjs Testcase.js

Output

1 f1

Correct Output

1 f2

Description

When executing the test case, QuickJS prints out 1 f1 while the correct output should be 1 f2. This is because the scope of functions f() declared in switch expressions belong to the block scope, which means the function f() in case 2 (line 9) overrides the one in case 1 (line 5), making the test program always executing the f() function in case 2. The same bug has reported to GraalJS(https://github.com/oracle/graaljs/issues/583) and has been fixed.

chqrlie commented 7 months ago

The scope of local function declarations (ECMA 15.2) seem to be the same as var scope, which is not exactly block scope, but the scope of the function body.

IIRC, we compile these definitions as var f = function() { print("f1"); } Which gives the same scope to the f identifier but binds the definition dynamically, which is not correct semantically.

Such semantics are so counter intuitive! For f to be defined as a function, the function definition function f() { print('f1'); } must be evaluated at runtime, but since there is a later definition function f() { print('f2'); } that is present in the same scope, that definition is used. And all this with an artificial scope hauling whereby the block introduced by the {} in the switch statement is ignored to make f visible at the enclosing scope. I cannot understand the purpose of these counter intuitive semantics.

I shall look into how to fix this, but it might not be so simple.