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

[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 ?

Thanks
-Laco.