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

Jacek Caban jacek at codeweavers.com
Tue Jun 15 04:09:18 CDT 2021



On 6/15/21 8:29 AM, Paul Gofman wrote:
> 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.

The optimization would be ultimitely nice, but it's fine to have it less 
optimized if it makes things much easier for now.

>> 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?

It's fine to leave it for later.

Jacek



More information about the wine-devel mailing list