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

[Bart]

Hi,

I would like to raise some concerns about the current implementation
of ordinal bithelpers in r 43417.

I do not have the rights to re-open that issue, and I did not want to
open a new one for it.

My concerns basically are the same I described in the associated
bugtracker issue (https://bugs.freepascal.org/view.php?id=36146)

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)

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

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.


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)

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.

-- 
Bart