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

[Gabriel Ivăncescu]

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.

Thanks,
Gabriel