[fpc-devel] RFC: customvariant handling in variants.pp and fmtbcd.pp

[Sergei Gorelkin]

LacaK пишет:
> Hi Sergei,
>>>
>>> I can fix it by following fixs:
>>>
>>> 1. fmtbcd_castto.diff ... added case when castto varString is 
>>> requested ... then do not use cast throught varDouble (to avoid lost 
>>> of precision), but convert directly from TBCD to string
>>>
>>> 2. variants.pp ... here we must add handling of customvariants into 
>>> sysvartolstr ... I created "helper" function TryCastFromCustomVariant
>>> which can be used multiple times (now in sysvartolstr and 
>>> sysvartoreal) ... I isolate in this function code which is required 
>>> multiple times.
>>>
>>> I am not sure if
>>> 1. is this optimal approach, or is better to put same code repeatedly 
>>> in sysvartolstrm sysvartoreal and in future in others sysvarto... ?
>>> 2. is the name and place of such function good choosen ?
>>>
>> Doing what you propose isn't good. Checking for custom variant type is 
>> expensive because it involves locking, therefore custom variant 
>> handling should be done *after* the standard types.
> ok
> 
>> The real problem is that the standard types are currently handled in 
>> VarUtils.VariantToX functions, which do not depend on Variants unit 
>> and cannot use custom variant stuff.
> yes
> 
>>
>> The correct solution would be to move the code of VariantToX functions 
>> to Variants unit, inlining them to sysvartoX functions. But if you try 
>> that, you'll discover very soon that VariantChangeTypeEx (non-Windows 
>> implementation in varutils.inc) depends on some of them. This is again 
>> not correct, because conversion functions called from 
>> VariantChangeTypeEx do not need to handle Pascal-specific types and 
>> by-reference variants. But a simplifed portion of conversion functions 
>> have to stay in VarUtils unit.
>>
>> This is of course a major rewrite, but I'd stay on it, because with a 
>> number of quick-fixes we risk to end up with unmaintainable code.
>>
> ok I agree, but I think, that this is too dificult for me, so I must 
> wait if somebody will do it.
> 
> So my questions are:
> 1. is acceptable apply my patch (or some variation of it) to variants.pp 
> as a temporary solution ? (you can add there hint "must be chaged, when 
> ...")
> 2. fix for fmtbcd.pp does not affect above mentioned problem, so should 
> you commit it (if is correct from your POV) to not forget about it ?
> 
Both committed in r17170, with some changes:

In fmtbcd.pp, I used VarDataFromStr method instead of assigning individual fields of TVarData;
if I understand things correctly, this method exist precisely for such cases.

In variants.pp, I used an individual function, with an out-parameter of type AnsiString. Such 
approach allows to avoid finalization of the intermediate Variant.

Regards,
Sergei