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

Zoltan Somogyi zoltan.somogyi at runbox.com
Mon Jan 3 17:44:05 AEDT 2022


For review by anyone.

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?

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.

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.

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.

BTW, I am keeping the clones of unsafe_{get,set}_byte
until the next run on testing shows that this diff does eliminate
all the warnings on that machine.

Zoltan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Log.bm2
Type: application/octet-stream
Size: 1650 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20220103/ad77887b/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DIFF.bm2
Type: application/octet-stream
Size: 5760 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20220103/ad77887b/attachment-0001.obj>


More information about the reviews mailing list