[m-rev.] for review: fix gcc warnings for bitmap.c

[Julien Fischer]

Hi Zoltan,

On Mon, 3 Jan 2022, Zoltan Somogyi wrote:

> One question is: should the change to the behavior
> of the some of the operations of this module be described
> in a NEWS file entry? The affected predicates' names
> all start with "unsafe", and their behavior changes ONLY
> when they are given out-of-bounds input, so I would
> vote NO, but there may be arguments for YES as well.
> Can you think of any?

I don't think it is necessary.

> In the long run, these issues should be fixed by moving
> from representing both bitmaps, and indexes into them,
> using unsigned values. When bitmap.m was written, this
> was not an option, but now it is. I see two issues.

There is a third issue: one of our target languages, Java, doesn't
provide unsigned intger types and hence doesn't support unsigned array
accesses.  Shifting the Mercury interface to unsigned values will
complicate the Java implementation of bitmaps.

> The first is whether maintaining backward compatibility
> is worth while.
> The obvious way to switch to unsigned values
> would change to unsigned not only types that are internal
> to bitmap.m, but also exported types, including the types
> of some of the arguments of most exported predicates
> and functions. This would require adjustments to any user
> module that imports bitmap.m. If the user code worked before,
> the adjustments should only be a few casts, but still.
>
> We could avoid imposing this cost on users in one of
> two ways. One way would be to keep the user-visible
> types signed, introduce private unsigned equivalents,
> and convert between them at the boundary. The other way
> would be introduce a new bitmap module that uses
> unsigned values in both the public and private halves.
> The first way would have the disadvantage of squeezing
> communication between any user code that uses
> unsigned values and the internals of bitmap.m which also
> uses unsigned values through an interface that uses
> signed values, requiring conversions on both sides.
> The second way has the major disadvantage of requiring
> double maintenance.

I would be inclined to have a new bitmap module for a period, with the
original one being deprecated and eventually replacing it.
That said, I think some consideration should be given to the role of
unsigned ints across the whole stdlib, rather than just isolated to
one module (e.g. is it worth having an unsigned variant of
list.length/1?)

> The other issue is a detail issue: for each type we change
> from signed to unsigned, we should also consider whether
> it should be just "uint", or "uintN" for some N. In some cases,
> the answer will be obvious (e.g. uint8 for indexing into bytes),
> but in some cases it may not be.

There's no particular reason to use uint8 for indexing into bytes,
you still have to perform a bounds check.  (The existing function
for manipulating bits within uint{8,16,32,64}s value all use uint as
their index.)

> Attempt to fix gcc warnings for library/bitmap.c.
> 
> library/bitmap.m:

The diff looks fine.

Julien.