[PATCH v5 4/7] vbscript: Append to the global lists when the code is executed, rather than compiled.

[Jacek Caban]

On 11/8/19 2:28 PM, Gabriel Ivăncescu wrote:
> On 11/8/19 3:16 PM, Jacek Caban wrote:
>> On 11/8/19 1:08 PM, Gabriel Ivăncescu wrote:
>>> On 11/7/19 8:28 PM, Jacek Caban wrote:
>>>> Hi Gabriel,
>>>>
>>>> On 11/7/19 2:28 PM, Gabriel Ivăncescu wrote:
>>>>> diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h
>>>>> index 04a9b83..efc3585 100644
>>>>> --- a/dlls/vbscript/vbscript.h
>>>>> +++ b/dlls/vbscript/vbscript.h
>>>>> @@ -350,6 +350,9 @@ struct _vbscode_t {
>>>>>       unsigned bstr_cnt;
>>>>>       heap_pool_t heap;
>>>>> +    dynamic_var_t *global_vars;
>>>>
>>>>
>>>> It seems to me that we don't need it. Now that we post-process 
>>>> variables to append to global variable set anyway, we could just 
>>>> get rid of special-case for global code in compiler and use 
>>>> variables stored in vbscode_t::main_code when we need to add them 
>>>> to global variables set.
>>>>
>>>>
>>>> Jacek
>>>>
>>>
>>> I've been looking at the code a bit and I'm afraid I'm missing 
>>> something. What exactly do you mean by post-process variables?
>>>
>>> As it is now, the code is wrong and will not pass the tests I wrote. 
>>> When the code is executed (and it can be executed several times) it 
>>> should add its vars/functions to the global lists so they get 
>>> DISPIDs and so on.
>>>
>>> Or do you have a better place to add its functions and variables to 
>>> the global lists? It has to be a place where they get added if the 
>>> code gets re-executed (pending code), though. And only after the 
>>> script is started.
>>>
>>> Sorry for the confusion.
>>
>>
>> See how compile_func allocates variables. It handles global variables 
>> differently from variables inside functions. The reason for that is 
>> that in the past, global variables were added straight to the 
>> context. Now that we have an extra step anyway, I wonder if we still 
>> need that. Maybe, instead of exposing this special case outside 
>> compiler, we could just use single representation in all cases. It's 
>> already exposed in function_t (although we'd need to add is_const to 
>> var_desc_t so that we have all needed informations exposed). In 
>> exec_script, you could iterate code->main_code.vars instead of newly 
>> exposed global_vars. The only downside is that we'd need to allocate 
>> dynamic_var_t later anyway, like we do for implicitly created 
>> variables (we could eventually delay that through, but that's 
>> additional work not necessary for what you need).
>>
>>
>> Jacek
>>
>
> Thanks for the explanation.
>
> So, I've been fiddling with some more tests, it seems that the 
> explicit vars (the ones compiled) need to be added to DISPID as soon 
> as script starts executing, even if they are never set.


I didn't mean to suggest changing that. The comment about delaying was 
for the storage only (at some point we could want to store VARIANT 
outside dynamic_var_t for different reasons), not for something that 
would be visible outside interpreter. Please do not bother about that.


>
> I've updated the tests I wrote (from "dim z\nz=3\n" to just "dim z") 
> and they still pass on Windows, but fail on Wine if I remove that 
> special case.
>
> On a side note, I don't think `is_const' is needed in var_desc_t 
> because to reconstruct the global vars in exec_global_script, they are 
> always set to FALSE currently. So there's no need to add it to 
> var_desc_t.
>
>
> So let me summarize and see if I got this right:
>
> Get rid of the special case for global function in compile_func.
>
> Then, in exec_global_code, walk through the code->main_code.vars and 
> allocate the variables there instead of in compile_func, the same way.
>
> This second step is required because those vars need to exist as soon 
> as the script is executed (even if they are never set), and besides, 
> they need to be added *before* the dynamic (implicit) vars, for the 
> TypeInfo anyway.


Yes, that's what I meant.


Thanks,

Jacek