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

Zoltan Somogyi zoltan.somogyi at runbox.com
Tue Mar 8 13:17:20 AEDT 2022


2022-03-08 13:13 GMT+11:00 "Julien Fischer" <jfischer at opturion.com>:
>> 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.)

Ok, will fix.

> 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.

I am not putting my hand up for that one.
 
>> 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.

Ok, I will look into that. I was already considering an equivalent of
decl vs defn order warnings for instances.

Zoltan.


More information about the reviews mailing list