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

[Gabriel Ivăncescu]

On 13/04/2022 16:18, Jacek Caban wrote:
> 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
> 

It's probably not relevant, but for example testing stuff like checking 
the function 'g' after setting it within the function itself (in fact, 
this was the main thing I had to fix it for) to see whether it was 
changed as well, rather than just outside of it. I should probably add 
those of course since they don't make it complicated.

Most of my tests were just an artifact of my debugging trying a bunch of 
things to fix it until I realized how it's supposed to work :-)