[PATCH v5 02/11] jscript: Use to_primitive when getting the default value for js objects.

[Jacek Caban]

On 11/22/21 4:19 PM, Gabriel Ivăncescu wrote:
> On 22/11/2021 16:41, Jacek Caban wrote:
>> On 11/22/21 1:52 PM, Gabriel Ivăncescu wrote:
>>> On 22/11/2021 14:22, Jacek Caban wrote:
>>>> Hi Gabriel,
>>>>
>>>> On 11/19/21 7:03 PM, Gabriel Ivăncescu wrote:
>>>>> diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h
>>>>> index 69897cd..d37fbd1 100644
>>>>> --- a/dlls/jscript/jscript.h
>>>>> +++ b/dlls/jscript/jscript.h
>>>>> @@ -230,6 +230,9 @@ typedef struct {
>>>>>       builtin_setter_t setter;
>>>>>   } builtin_prop_t;
>>>>> +HRESULT 
>>>>> jsdisp_builtin_get_default_value(script_ctx_t*,jsdisp_t*,jsval_t*) 
>>>>> DECLSPEC_HIDDEN;
>>>>> +#define JSDISP_DEFINE_BUILTIN_VALUE(value) {NULL, value,0, 
>>>>> jsdisp_builtin_get_default_value}
>>>>
>>>>
>>>> Could we just handle DISPID_VALUE in dispex.c callers without using 
>>>> builtin_prop_t at all?
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Jacek
>>>>
>>>
>>> How are we going to handle the methods then, for the jsdisps that 
>>> have them? For example, Function_value. Wouldn't it require 
>>> special-casing them? I thought it's less "elegant" since it doesn't 
>>> re-use the same code path as now, just different builtin data.
>>
>>
>> I meant it only for getter, Function_value is for calling 
>> DISPID_VALUE. If getter is the same in every builtin_info_t, then it 
>> means that it's redundant, which is not really elegant. DISPID_VALUE 
>> is special in many ways, I don't see what's wrong with handling it 
>> separately.
>>
>>
>> Jacek
>>
>
> Well, it's not redundant since it's only for DISPID_VALUE props, but 
> the code handling the getters is universal to *all* props not just 
> DISPID_VALUE, which of course is not always the same. Right now we 
> don't add any code and just re-use the existing prop code.
>
> That said, if you still aren't convinced, I have a question. Should I 
> remove the value_prop completely from the builtin_info_t and from the 
> prop list? Right now it's always the first prop, i.e. zero, to match 
> DISPID_VALUE.
>
> Then, since I have to special-case it anyway, I'll just add a 
> builtin_invoke_t value_invoke instead and call it if needed in 
> InvokeEx, without having an actual prop for it.


Yes, removing value_prop seems right. I would call just "call" or 
something that would match [[Call]] from the spec.


Jacek

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20211122/f9d816bd/attachment.htm>