[fpc-devel] Attn Michael: r 43417 (ordinal bithelpers)

[Michael Van Canneyt]

On Fri, 8 Nov 2019, Bart via fpc-devel wrote:

> 1.
> Minor issue.
> Defining the ranges TByteBitIndex will confuse users if they define a
> constant array with that range and then compile their program in
> either mode fpc or mode tp.
>
> I have something like:
> const
>  Expected: array[TIntegerBitIndex] of Integer = ({16 entries here if
> Integer is 16-bit like in mode fpc/tp});
> It fails to compile with: Error: Expected another 16 array elements
>
> (Example taken form the test program in the bugtracker)

Yes, I had the same error when recompiling the test program with the local
indexes removed.

>
> Inside the sysutils unit the use of definition is OK, since this unit
> will alway be compiled in mode objfpc.

That is why I decided to keep it: the mode of sysutils is known and will
never change. A user is supposed to take this into account.

>
> Using an integer type as the index Index parameter would "fix" that,
> at the cost of having runtime errors if the index was out of bounds
> for the ordinal used (e.g. SomeByte.SetBit(31)).
>
> I can live with the current situation though.
> My test program just redefines the ranges to properly reflect the mode
> the program is compiled in.

I am aware of your remark. That is why I defined TLongIntBitIndex.
I decided to keep the TIntegerBitIndex name because the helper is for the
integer type, not for the longint type.

> 2.
> A rather more serious issue.
> Compile time errors occur with e.g.
> ANativeInt.SetBit(High(TNativeIntBitIndex)) in modes tp (32-bit), fpc
> (32-bit), objfpc (32+64-bit) and delphi (32+64-bit)
> Range check error while evaluating constants (2147483648 must be
> between -2147483648 and 2147483647)
> Same error for AnInteger.SetBit(High(TIntegerBitIndex)) in modes
> objfpc (32-bit), delphi (32-bit) and macpas (32-bit)

This error was confirmed as a compiler bug. It also disappears if you remove
the inline from the methods.

Since there is no run-time error for the same value, I kept the code as-is.
I consider this a remote corner case.

> The (tested) remedy for this is to do all bit manipulations on the
> unsigned ordinal instead.
> This will get rid of all compile time errors.
> It will then also pass all tests in the test program for all modes for
> both 32 and 64 bit.
> (I can't test 16-bit compiler).
>
> @Michael: I missed your request to me to make a new patch after I
> fixed the missing parnethesis in Thaddy's latest ClearBit method,
> otherwise I would have posted that for in the bugtracker.

If you send me a patch to rework the bit manipulations, I will check & apply it.

Michael.