[PATCH 04/10] jscript: Defer lookup to the actual locals for function statements.

[Jacek Caban]

On 4/13/22 13:43, Gabriel Ivăncescu wrote:
> On 12/04/2022 21:16, Jacek Caban wrote:
>> Hi Gabriel,
>>
>> On 4/12/22 16:47, Gabriel Ivăncescu wrote:
>>> diff --git a/dlls/mshtml/tests/es5.js b/dlls/mshtml/tests/es5.js
>>> index e8ee713..2c08080 100644
>>> --- a/dlls/mshtml/tests/es5.js
>>> +++ b/dlls/mshtml/tests/es5.js
>>> @@ -1691,6 +1691,65 @@ sync_test("functions scope", function() {
>>>       func_outer(o);
>>>       func();
>>>       ok(ret === o, "ret != o");
>>> +
>>> +    func_outer = function g(i)
>>> +    {
>>> +        ok(ret === (i ? 42 : o), "ret during g(" + i + ") = " + ret);
>>> +        ok(typeof g == "function", "typeof g == " + typeof g);
>>> +
>>> +        g = function() { ok(false, "redefined g was executed"); }
>>> +        ret = 42;
>>> +        if(!i) g(1);
>>> +    }
>>> +    func_outer(0);
>>> +
>>> +    function h()
>>> +    {
>>> +        ok(typeof h == "function", "typeof h == " + typeof h);
>>> +        var f = function func_inner(i)
>>> +        {
>>> +            if(i === 101) {
>>> +                ok(h === "string", "h during old h(101) = " + h);
>>> +                ret = -2;
>>> +                return;
>>> +            }
>>> +            if(i === 100) {
>>> +                ok(h.toString() === "function foo() {}", 
>>> "h.toString() during old h(100) = " + h.toString());
>>> +                h = "string";
>>> +                ok(h === "string", "h during old h(100) after set 
>>> to string = " + h);
>>> +                ret = -1;
>>> +                return;
>>> +            }
>>> +            if(i === 1) {
>>> +                ok(h !== func_inner, "h during h(1) === func_inner");
>>> +                return;
>>> +            }
>>> +            ok(h === func_inner, "h during h() !== func_inner");
>>> +            if(i) {
>>> +                ok(ret === 42, "ret during h(2) = " + ret);
>>> +                return;
>>> +            }
>>> +            ret = 13;
>>> +        }
>>> +        f(1);
>>> +        h = f;
>>> +        h(2);
>>> +    }
>>> +    func_outer = h;
>>> +    h();
>>> +    ok(ret === 42, "ret after calling h() first time = " + ret);
>>> +    ok(func_outer !== h, "func_outer after calling h() first time 
>>> === h");
>>> +    func_outer = h;
>>> +    h();
>>> +    ok(ret === 13, "ret after calling h() second time = " + ret);
>>> +    ok(func_outer === h, "func_outer after calling h() second time 
>>> === h");
>>> +    h = function foo() {}
>>> +    ok(func_outer !== h, "func_outer after setting h to empty 
>>> function === h");
>>> +    func_outer(100);
>>> +    ok(ret === -1, "ret after calling old h(100) = " + ret);
>>> +    ok(h === "string", "h after calling old h(100) = " + h);
>>> +    func_outer(101);
>>> +    ok(ret === -2, "ret after calling old h(101) = " + ret);
>>>   });
>>
>>
>> I think that you made it much more complicated than it needs to be. 
>> See the attached patch for an example how it could be made more 
>> readable.
>>
>>
>> Thanks,
>>
>> Jacek
>
> Hi Jacek,
>
> I wanted to be thorough with the tests and include weird cases (such 
> as when it's detached vs not) just to make sure, but if you think your 
> tests are enough I'd be happy to resort to it. 


Having thorough tests is obviously welcomed, but it doesn't mean that 
they need to be so hard to read. The version that I sent you already 
tests detached function statement scopes, it should be easy to test 
function expressions in a similar way. Is there anything else we'd miss 
from your tests?


Thanks,

Jacek