[m-rev.] for review: delete see/seen/tell/told from io.m

[Julien Fischer]

On Tue, 8 Mar 2022, Zoltan Somogyi wrote:

> 2022-03-05 18:51 GMT+11:00 "Julien Fischer" <jfischer at opturion.com>:
>>> An issue just came up when I moved the code for reading/writing
>>> bitmaps to bitmap.m: where should we keep the various  instances of typeclasses
>>> such as stream.bulk_reader and stream.writer that say how to read/write
>>> non-primitive types? Specifically, if the reading/writing code for that type
>>> has been moved to another module, should the instance declarations
>>> go with them, or should they stay in io.m?
>>>
>>> I can see arguments in favor of both answers. Logically, an instance that
>>> implements a typeclass method using a predicate should be in the module
>>> that defines that predicate. OTOH, this may require modules to import
>>> the modules that define the typeclasses just for the sake of the instance
>>> declaration, and it makes checking whether a class has an instance for
>>> a given type harder.
>>
>> Since those instances are for types (i.e. io.state and bitmap) from both
>> modules, they could be resasonably declared in either.
>>
>> On balance, I favour moving them to bitmap.m.
>
> The attached diff, which I just committed, does that.
>
> The diff does raise an issue. io.m had three instances involving bitmaps,
> but only two were exported; one was not. Since io.m exported the fact
> that bitmap.slice is a member of a typeclass, but did not export the fact
> that bitmap itself is a member of that same typeclass, I think this is
> a bug, but since I never worked with bitmaps, I am not sure. Does anyone
> know?

It is almost certainly an oversight and it should be exported.
(I don't think those instances have had a lot of use since Simon added
them.)

As an aside: the bitmap type has always been a bit awkward; it serves
two roles, first as compact implementation of map(int, bool) and second
as a container for binary data.  The fact that its API allows for
partial final bytes is not a natural fit for the second role.  (I raise
this because the type class instances are mostly concerned with the
second role.)

I think we might do better if we introduce two new types for holding
binary data, essentially equivalent to an array of uint8 (at the target
language level).  There would be two, because we would have an immutable
read-only version and a mutable version.

> Also, I noticed that in io.m, the abstract and concrete instance declarations
> are not in the same order. I intend to put them into the same order;
> this should reveal whether there are any more intentionally unexported
> instances.

A possibly useful addition to the compiler might be to have a new
warning (disabled by default) that checks for non-exported type class
instances whose argument vectors contain only non-local types.

Julien.