Hi,

I've been trying to improve the ConvUtils unit.
Now I've ran into a problem.

ConvTypeToFamily returns CIllegalConvFamily (=0) if AType is not registered. This however is a problem. If AType is a type registered in the first registered family, ConvTypeToFamily will also return 0. This means that the result value of ConvTypeToFamily cannot be used to determine wether or not AType actually is registered.

This is IMHO not good.

Related to this is the fact that in Delphi ConvFamilyToDescription(0) will return the strng "Illegal family" always, wether or not any families have been registered.
(Tested with Delphi 7)

========= code begin ==========
program dconv;
{$apptype console}
{$ifdef fpc}
{$mode objfpc}{$h+}
{$endif}
uses
  sysutils,convutils;
procedure ListFamilies;
var
  Fam, FirstFam, SecFam: TConvFamily;
  Fams: TConvFamilyArray;
  tuFirstType, tuSecType: TConvType;
  Len, ExpLen: Integer;
begin
  GetConvFamilies(Fams);
  Len := Length(Fams);
  if (Len=0) then
    writeln('Nothing registered yet');
  writeln('Length(Fams)=',Len);
  if Len>0 then
  begin
  for Fam := Low(Fams) to High(Fams) do
    writeln(format('%d: "%s"',[ord(fam),ConvFamilyToDescription(fam)]));
  end
  else
  begin
    writeln('Testing with out of bound values');
    writeln('ConvFamilyToDescription(0)="',ConvFamilyToDescription(0),'"');
    writeln('ConvFamilyToDescription(1)="',ConvFamilyToDescription(1),'"');
  end;
  writeln;
  writeln('Registering First and Second Family');
  FirstFam := RegisterConversionFamily('First Family');
  tuFirstType := RegisterConversionType(FirstFam, 'FirstType', 123.0);
  SecFam := RegisterConversionFamily('Second Family');
  tuSecType := RegisterConversionType(SecFam, 'SecondType', 321.0);
  GetConvFamilies(Fams);
  ExpLen := Len+2;
  Len := Length(Fams);
  write('Length(Fams)=',Len);
  if (LenExpLen) then writeln('  FAIL: Expected ',ExpLen) else writeln;
  if Len>0 then
  begin
  for Fam := Low(Fams) to High(Fams) do
    writeln(format('%d: "%s"',[ord(fam),ConvFamilyToDescription(fam)]));
  end;
  writeln('ConvFamilyToDescription(0)="',ConvFamilyToDescription(0),'"');
end;
begin
  {$ifdef fpc}
  writeln('FPC');
  {$else}
  writeln('Delphi');
  {$endif}
  ListFamilies;
end. ============ code end ==========

Output in FPC:
FPC
Nothing registered yet
Length(Fams)=0
Testing with out of bound values
ConvFamilyToDescription(0)=""
ConvFamilyToDescription(1)=""

Registering First and Second Family
Length(Fams)=2
0: "First Family"
1: "Second Family"
ConvFamilyToDescription(0)="First Family"

Output in Delphi:
Delphi
Nothing registered yet
Length(Fams)=0
Testing with out of bound values
ConvFamilyToDescription(0)="Illegal family"
ConvFamilyToDescription(1)="[$00000001]"

Registering First and Second Family
Length(Fams)=2
0: "Illegal family"
1: "First Family"
ConvFamilyToDescription(0)="Illegal family"

The output of Delphi (7) is obviously totally wrong.

Before I can even start to try to fix this I really need to know how current Delphi actually behaves.

As long as we want to have ConvTypeToFamily and CIllegalConvFamily compatible with Delphi, the function RegisterConversionFamily() should never return CIllegalConvFamily .
I.o.w. the internal array should not start at 0 (or the 0-index should be filled with info that represents an illegal family). So, any fix would break Delphi compatibility?

Q1: Can somebody test with a modern Delphi?
Q2: Any comments on the pargraph above?

And another observation: on Delphi 7 TConvType seems to be unsiged (in fpc it's signed).

AM Bart wrote

And another observation: on Delphi 7 TConvType seems to be unsiged (in fpc it's signed).

And as a consequence compilation in Delphi fails if a negative value is supplied to a function that takes a TConvType as a parameter, but also code like this will not raise an exception Delphi, while it does in Fpc:

var
  L: Integer;
begin
  L:=-1;
  writeln('ConvTypeToDescription(L)=',ConvTypeToDescription(L));  //
prints [$0000FFFF] in D7, EAccessViolation in fpc
end.

AM Bart wrote

And another observation: on Delphi 7 TConvType seems to be unsiged (in fpc it's signed).

Actually it is documented to be of type Word:
https://docwiki.embarcadero.com/Libraries/Sydney/en/System.ConvUtils.TConvType. I guess nobody needs more than 65536 conversion types, so most likely not a problem.

So, this is not an implementation detail (which we could simply ignore), but actually it is now a bug.

Changing our TConvType to word will potentially break existing programs though.
Consider the following code:
uses
  convutils,sysutils;
var
  Fam: TConvFamily;
  Fams: TConvFamilyArray;
  Len: Integer;
begin
  GetConvFamilies(Fams);
  Len := Length(Fams);
  if (Len=0) then
    writeln('Nothing registered yet');
  for Fam := Low(Fams) to High(Fams) do
    writeln(format('%d: "%s"',[ord(fam),ConvFamilyToDescription(fam)]));
end.

If Len=0 this then High(Fams) will be Word(-1) and the loop will print 65536 non-existing families.
(Which is why you should always use signed integers in a for loop)

All this leads me to the conclusion that they (the Greek) did not really desing this unit very well.

Bart wrote

Changing our TConvType to word will potentially break existing programs though.

I doubt that there are mission critical convutils programs out there, given the fact that it took 10+ years  to find out the temperature conversion didn't work :-)

Usage is low, risk is small, I say, go ahead.

Actually the behavior in Delphi depends on the $R switch.

PM Stefan Glienke wrote

Actually the behavior in Delphi depends on the $R switch.

D7:
{$R+}
ConvTypeToDescription(-1)=[$0000FFFF]
{$R-}
ConvTypeToDescription(-1)=[$0000FFFF]

Bart

PM Marco van de Voort wrote

I doubt that there are mission critical convutils programs out there, given the fact that it took 10+ years to find out the temperature conversion didn't work :-)

I agree.

I only stubled upon this unit because I have unit that converses integers to string in a base (up to base 36) and I wondered if fpc had a general conversion unit, where perhaps it could be added. That's how I found the StdConvs unit and then discovered the temperature bug. The rest is, as they say, history.

Usage is low, risk is small, I say, go ahead.

This still leaves us with the returnvalue of ConvTypeToFamily, which should indicate if an has occurred.
However, at least in D7, ConvTypeToFamily does NOT return 0 if an error occurs, but it raises an exception.
So, what to do for that one: follow the docs, or follow the observed behavoir of D7 (again, someone should test a newer version)?

Basically since both TConvFamily and TConvType are of type Word (in Delphi), it is impossible to return a number that indicates an error at all (unless you fill both Families[0] and RegisteredConvTypes[0] with info indicating they are invalid (which D7 seems not to do). Which makes the CIllegalConvFamily and CIllegalConvType constants rather useless.
The only function that actually returns a TConvType (AFAICS) raises an exception upon error.

So, we can go 2 ways:
1.
Leave TConvFamily and TConvType and change CIllegalConvFamily to -1 Not Delphi compatible

2.
Make TConvFamily and TConvType of type Word, raise an exception in ConvTypeToFamily upon error.
This needs some refactoring to limit the amount of TConvTypes that can be registered.
It's not compliant with the Delphi docs.

I would tend to also implement TryXXX functions for all procs/functions that will raise conversion exceptions in that case. Just to be consistent with most of our conversion routines we have.

Then there is the issue with ConvFamilyToDescription: should it return an empty string if the family does not exist (current behaviour) or "Illegal family", as D7 does?
I would leave it as it is, since "illegal family" is a legal name for a family to register...

@Marco: I first need to have the patches in #39774 and #39773 applied before embarking on this.

I usually don't take 20 year old compilers as reference

PM Stefan Glienke wrote

I usually don't take 20 year old compilers as reference

That's all that I have.
I do Lazarus/Fpc as a hobby and am unwilling to spend lost of money on Delphi. Also the latest "free" versions are way to bloated to have them on my regular computer (of which I only have one)

Please post the output of a modern Delhi with the code above.

It will raise an ERangeError with $R+.

Done.
https://gitlab.com/freepascal.org/fpc/source/-/issues/39776

Applied, thank you very much !

Michael.