On 6/16/21 22:35, Jacek Caban wrote:
It would be nice to split it a bit. For example, it seems to me that visiting phrase changes could be a separated patch.
Well, I even had most of the visiting part as a separate patch initially but squashed all that after some consideration. I will pull it back.
+#define MAX_SCOPE_COUNT 1024
Such arbitrary limit seems bad. Scripts can be really long, so it's not unrealistic to hit the limit. At the same time, it's wasteful for short scripts like "runOnload()".
I will switch that to dynamic reallocation.
+ TRACE("iter->scope_index %d, ctx->func->local_scope_count %d.\n", iter->scope_index, ctx->func->local_scope_count);
Is it a debugging leftover? I wouldn't mind adding some debug traces, but this one seems kind of random.
It is not exactly a leftover but yes, it is not particularly useful. I will remove it.
{
+ statement_ctx_t stat_ctx = {0, TRUE}; + BOOL needs_scope; HRESULT hres; + needs_scope = block && block->scope_index; + if (needs_scope) + { + if(!push_instr(ctx, OP_new_obj)) + return E_OUTOFMEMORY; + if(FAILED(hres = push_instr_uint_uint(ctx, OP_push_scope, block->scope_index, TRUE))) + return hres;
Creating a new object on each block entry is a rather expensive. Could we create those object on demand instead (like in detach_variable_object())? Given that, it seems to me that a new dedicated opcode would appropriate here.
I should note that it is not for each block but only for blocks which has any 'let' variables or functions defined in it. But it won't hurt to optimize for the case when there are just local variables and the scope ends up not being referenced on leave. I will add the opcode and create the object on detach.