[fpc-devel] RFC: customvariant handling in variants.pp and fmtbcd.pp
Hi,
I would like ask for your opinion in this case.
I found, that in this sample program is raised "Invalid variant ..."
var bcd1: TBCD;
v,v1: variant;
s: string;
begin
bcd1:=2;
v1:=varfmtbcdcreate(bcd1);
s:=v1; //assigment from customvariant to string is not handled in
sysvartolstr()
end;
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 ?
Can somebody look at it, and commit what is good and change what is not
good ;-) ?
Thanks
-Laco.
--- f:\tmp\fmtbcd.pp.oldMon Mar 21 07:37:34 2011
+++ f:\tmp\fmtbcd.ppWed Mar 23 10:34:38 2011
@@ -4006,9 +3992,18 @@ begin
begin
VarDataInit(v);
try
- v.vType:=varDouble;
- v.vDouble:=TFMTBcdVarData(Source.vPointer).BCD;
- VarDataCastTo(Dest, v, aVarType); //now cast Double to any requested type
+ if aVarType = varString then
+ begin
+Dest.vType:=varString;
+Dest.vString:=nil;
+
AnsiString(Dest.vString):=BCDToStr(TFMTBcdVarData(Source.vPointer).BCD);
+ end
+ else
+ begin
+v.vType:=varDouble;
+v.vDouble:=BCDToDouble(TFMTBcdVarData(Source.vPointer).BCD);
+VarDataCastTo(Dest, v, aVarType); //now cast Double to any requested
type
+ end;
finally
VarDataClear(v);
end;
--- f:\tmp\variants.pp.ori Thu Dec 2 10:03:42 2010
+++ f:\tmp\variants.pp Wed Mar 23 10:32:52 2011
@@ -602,7 +602,6 @@ begin
TVarData(V).vType := varEmpty;
end;
-
procedure sysvarclear(var v : Variant);
begin
if TVarData(v).vType and varComplexType <> 0 then
@@ -611,6 +610,16 @@ begin
TVarData(v).vType := varEmpty;
end;
+function TryCastFromCustomVariant(const v : Variant; const aVarType: TVarType;
out Dest: TVarData):boolean;
+var Handler: TCustomVariantType;
+begin
+ Result:=FindCustomVariantType(TVarData(v).vType, Handler);
+ if Result then
+ begin
+VariantInit(Dest);
+Handler.CastTo(dest, TVarData(v), aVarType);
+ end;
+end;
function Sysvartoint (const v : Variant) : Integer;
begin
@@ -661,20 +670,15 @@ end;
{$ifndef FPUNONE}
function sysvartoreal (const v : Variant) : Extended;
-var Handler: TCustomVariantType;
-dest: TVarData;
+var dest: TVarData;
begin
if VarType(v) = varNull then
if NullStrictConvert then
VarCastError(varNull, varDouble)
else
Result := 0
- else if FindCustomVariantType(TVarData(v).vType, Handler) then
- begin
-VariantInit(dest);
-Handler.CastTo(dest, TVarData(v), varDouble);
-Result := dest.vDouble;
- end
+ else if TryCastFromCustomVariant(v, varDouble, dest) then
+Result := dest.vDouble
else
Result := VariantToDouble(TVarData(V));
end;
@@ -694,12 +698,15 @@ end;
procedure sysvartolstr (var s : AnsiString; const v : Variant);
+var dest: TVarData;
begin
if VarType(v) = varNull then
if NullStrictConvert then
VarCastError(varNull, varString)
else
s := NullAsStringValue
+ else if TryCastFromCustomVariant(v, varString, dest) then
+s := AnsiString(dest.vString)
else
S := VariantToAnsiString(TVarData(V));
end;
___
fpc-devel maillist - fpc-devel@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] RFC: customvariant handling in variants.pp and fmtbcd.pp
LacaK пишет: 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. 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. 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. Regards, Sergei ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] RFC: customvariant handling in variants.pp and fmtbcd.pp
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. ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [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 ? 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 ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] RFC: customvariant handling in variants.pp and fmtbcd.pp
Hi Sergei, 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. Both committed in r17170, with some changes: great thanks! 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. ok thanks, I have attached minor "rearangement", consider them please 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. ok, thank you very much I have looked again at function FindCustomVariantType and I see there : 3590 Result:=(aVarType>=CMinVarType); 3591 if Result then So it seems to me, that for standard variant types is control immediately returned, so no performance problem should exist there with entering critical section and so on. Or I missed something ? Laco. procedure TFMTBcdFactory.CastTo(var Dest: TVarData; const Source: TVarData; const aVarType: TVarType); var v: TVarData; begin if Source.vType=VarType then if aVarType = varString then VarDataFromStr(Dest, BCDToStr(TFMTBcdVarData(Source.vPointer).BCD)) else begin VarDataInit(v); try v.vType:=varDouble; v.vDouble:=BCDToDouble(TFMTBcdVarData(Source.vPointer).BCD); VarDataCastTo(Dest, v, aVarType); //now cast Double to any requested type finally VarDataClear(v); end; end else inherited; end;___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
