[PATCH 4/4] jscript: Support nested scopes for functions defined inside.

[Paul Gofman]

Hello Jacek,

On 6/14/21 21:17, Jacek Caban wrote:
> Hi Paul,
>
> On 6/11/21 7:46 PM, Paul Gofman wrote:
>> Signed-off-by: Paul Gofman<pgofman at codeweavers.com>
>> ---
>>   dlls/jscript/compile.c   | 34 +++++++++++++++++++++--
>>   dlls/jscript/engine.c    | 59 +++++++++++++++++++++++++++++++++++++--
>>   dlls/jscript/engine.h    |  2 ++
>>   dlls/jscript/parser.h    |  1 +
>>   dlls/jscript/parser.y    |  1 +
>>   dlls/mshtml/tests/es5.js | 60 ++++++++++++++++++++++++++++++++++++++--
>>   6 files changed, 150 insertions(+), 7 deletions(-)
>
>
> It would be nice to have tests for more interesting cases. I attached
> a test shows scope chain life time and it's failing for me with your
> patches.

Yes, thanks. It seems to me it is not exactly about the scope life time,
looks like I need to detach scope when leaving it if it is still
referenced (similar to leaving the function frame; currently it is not
detached and while scope is alive the on-stack value is retrieved which
is wrong). Executing detach_variable_object() on leaving the scope helps
in this case. It can be optimized by detaching the scope being left
only, but then it will need a neater tracking of deoptimized case in
stack_topn_exprval() for local refs. I am not yet quite sure if such
optimization worth it.


>
> While writing the test, I also noticed that 'let' inside 'for'
> statement doesn't work, for example:
> for(let i = ...; ...)
>
Yes. The 'let' and 'var' in 'for' is a separate syntax rule in spec and
the same is for 'var' in our implementation. I thought of it as a
separate change which can be added later (the handling of let in for
should be easy on top of these patches). Or do you think it should be
included at once?