Hello Jacek,
On 6/18/21 13:16, Jacek Caban wrote:
Hi Paul,
Patches look mostly good to me now, I have one doubt through:
On 6/17/21 11:25 PM, Paul Gofman wrote:
case STAT_WITH: { with_statement_t*with_stat = (with_statement_t*)stat; + statement_ctx_t stat_ctx = {0, TRUE}; hres = visit_expression(ctx, with_stat->expr); if(FAILED(hres)) return hres; - hres = visit_statement(ctx, with_stat->statement); + if (ctx->parser->script->version >= SCRIPTLANGUAGEVERSION_ES5) + { + if (!alloc_local_scope(ctx, &with_stat->scope_index)) + return E_OUTOFMEMORY;
+ stat_ctx.scope_index = with_stat->scope_index; + }
Do we need to alloc local scope for with statement? They only use passed object for the scope and, unless I'm missing something, we should never need this local scope anyway.
I recall it was needed for something at some earlier version of the patches, but I checked now, and it actually seems redundant. More so, I tested the function definition under 'with' without explicit block, and it revealed some bugs WRT current 'with' scope enumeration handling, as well that the 'with' doesn't affect the function definition ('with' scope shouldn't be attached to the function).
So, I will add the test and remove the scope enumeration for 'with' and dependent parts.
I also tried testing:
with({w:10}) let e = w
But figured that is syntax error. LexicalDeclaration shouldn't be actually a part of Statement but rather a part of a new Declaration statement which should be referenced from Block statement declaration. I suggest I fix that after this patch series.