[fpc-devel] ConvUtils: question regarding Delphi compatibility. How to proceed?

[Bart]

Hi,

The RegisterConversionType() function has an overload with two
paramters of type TConversionProc.
It is used for conversions that have different offsets on their
respective scales (like temperature).

If used like:
=== code ===
tuFahrenheit :=
RegisterConversionType(cbTemperature,txttuFahrenheit, at FahrenheitToCelsius, at CelsiusToFahrenheit);
=== end code===

This will work as expected.

Nothing however prevents the user to use nil for one or even both of
these parameters.

Currently trying to do so will immediately raise an exception in our
implementation if the first one is nil.
Delphi however allows this and raises an exception when the conversion
requested invokes the TConversionProc that is nil.


Consider the following piece of code:

=== code begin ===
function DummyToProc(const AValue: Double): Double;
begin
  Result := 123.456;
end;

function DummyFromProc(const AValue: Double): Double;
begin
  Result := -987.654;
end;

procedure TestNilProc;
var
  Fam: TConvFamily;
  dummy_1, dummy_2: TConvType;
  D: Double;
  RegisterFailed: Boolean;
begin
  RegisterFailed := False;
  Fam := RegisterConversionFamily('TestNilProc');
  try
    write('RegisterConversionType(Fam, ''dummy_1'', {$ifdef
fpc}@{$endif}DummyToProc, nil)=');
    dummy_1 := RegisterConversionType(Fam, 'dummy_1', {$ifdef
fpc}@{$endif}DummyToProc, nil);
    writeln(dummy_1);
  except
    on E: Exception do
    begin
      writeln(E.ClassName,': ',E.Message);
      RegisterFailed := True;
    end;
  end;
  try
    write('RegisterConversionType(Fam, ''dummy_2'', nil, {$ifdef
fpc}@{$endif}DummyFromProc)=');
    dummy_2 := RegisterConversionType(Fam, 'dummy_2', nil, {$ifdef
fpc}@{$endif}DummyFromProc);
    writeln(dummy_2);
  except
    on E: Exception do
    begin
      writeln(E.ClassName,': ',E.Message);
      RegisterFailed := True;
    end;
  end;

  if RegisterFailed then
  begin
    writeln('RegisterFailed ...');
    Exit;
  end;

  try
    write('Convert(1.0,dummy_1,dummy_2)=');
    D := Convert(1.0,dummy_1,dummy_2);
    writeln(D:10:4);
  except
    on E: Exception do writeln(' ',E.ClassName,': ',E.Message);
  end;
  try
    write('Convert(1.0,dummy_2,dummy_1)=');
    D := Convert(1.0,dummy_2,dummy_1);
    writeln(D:10:4);
  except
    on E: Exception do writeln(E.ClassName,': ',E.Message);
  end;
end;


procedure TestDoubleNilProc;
var
  Fam: TConvFamily;
  dummy_1, dummy_2: TConvType;
  D: Double;
  RegisterFailed: Boolean;
begin
  Fam := RegisterConversionFamily('TestDoubleNilProc');
  RegisterFailed := False;
  try
    write('RegisterConversionType(Fam, ''dummy_1'',nil , nil)=');
   dummy_1 := RegisterConversionType(Fam, 'dummy_1',nil , nil);
   writeln(dummy_1);
  except
    on E: Exception do
    begin
      writeln(E.ClassName,': ',E.Message);
      RegisterFailed := True;
    end;
  end;
  try
    write('RegisterConversionType(Fam, ''dummy_2'', nil, nil)=');
    dummy_2 := RegisterConversionType(Fam, 'dummy_2', nil, nil);
    writeln(dummy_2);
  except
    on E: Exception do
    begin
      writeln(E.ClassName,': ',E.Message);
      RegisterFailed := True;
    end;
  end;

  if RegisterFailed then
  begin
    writeln('RegisterFailed ...');
    Exit;
  end;

  try
    write('Convert(1.0,dummy_1,dummy_2)=');
    D := Convert(1.0,dummy_1,dummy_2);
    writeln(D:10:4);
  except
    on E: Exception do writeln(E.ClassName,': ',E.Message);
  end;
  try
    write('Convert(1.0,dummy_2,dummy_1)=');
    D := Convert(1.0,dummy_2,dummy_1);
    writeln(D:10:4);
  except
    on E: Exception do writeln(E.ClassName,': ',E.Message);
  end;
end;
=== code end ===

Fpc output:
RegisterConversionType(Fam, 'dummy_1', {$ifdef fpc}@{$endif}DummyToProc, nil)=0
RegisterConversionType(Fam, 'dummy_2', nil, {$ifdef
fpc}@{$endif}DummyFromProc)=EAccessViolation: Access violation
RegisterFailed ...

RegisterConversionType(Fam, 'dummy_1',nil , nil)=EAccessViolation:
Access violation
RegisterConversionType(Fam, 'dummy_2', nil, nil)=EAccessViolation:
Access violation
RegisterFailed ...

Delphi 7 output:
RegisterConversionType(Fam, 'dummy_1', {$ifdef fpc}@{$endif}DummyToProc, nil)=1
RegisterConversionType(Fam, 'dummy_2', nil, {$ifdef
fpc}@{$endif}DummyFromProc)=2
Convert(1.0,dummy_1,dummy_2)= -987.6540
Convert(1.0,dummy_2,dummy_1)=EAccessViolation: Access violation at
address 00000000. Read of address 00000000

RegisterConversionType(Fam, 'dummy_1',nil , nil)=3
RegisterConversionType(Fam, 'dummy_2', nil, nil)=4
Convert(1.0,dummy_1,dummy_2)=EAccessViolation: Access violation at
address 00000000. Read of address 00000000
Convert(1.0,dummy_2,dummy_1)=EAccessViolation: Access violation at
address 00000000. Read of address 00000000


In our implementation of RegisterConversionType we also calculate the
conversion ratio disregarding the offset diffenrence in scales.
This is done so that the Convert() with 5 parameters correcty works.
E.g. one can convert from degrees Fahrenheit per minute degrees Kelvin
per second.
If you try that in Delphi 7 you get random output.

Notice also that if botr params are nil, then Delphi still tries to
call them, meaning they internally use a different mechanism to detect
wether or not a TConversionProc has been registered (we currently
check using Assigned(), which of course will return False if you
register with nil as parameter).

I could refactor things so that we postpone calculating the conversion
ratio and calculate it in Convert() (with 5 params), and use yet
another field in the type ResourceData type to indicate we must always
use a TConversionProc.

It feels a bit counter-intuïtive to me though.
IMO it should simply not be allowed to call RegisterConversionType()
with either of the TConversionProc parameters being nil, since that
makes no sense to me, and it will inevitably lead to exceptions later
on in the code.

The behaviour that Delphi 7 exhibits is not ducumented in the official
documentation about the ConvUtils unit.
So we could interpret this as an implementation detail: the behaviour
with either of these paramaters is undefined.
Which means, we do nothing at this point of time.
Or we could follow Delphi here (and further complicate our implementation).
(Or we could wait until a bugreport about this is filed with a real
life application in the wild being affected (not likely to happen))

Does anybody have an opinion about this?
--
Bart