jcbeaudoin / MKCL

ManKai Common Lisp
Other
33 stars 8 forks source link

Upgraded array element types: signed vs. unsigned #34

Closed davidmullen closed 6 months ago

davidmullen commented 6 months ago

Section 15.1.2.1 of ANSI says: "Also, if a type Tx is a subtype of another type Ty, then the upgraded array element type of Tx must be a subtype of the upgraded array element type of Ty." In this example, the first type is upgraded to mkcl:integer8, and the second type is upgraded to mkcl:natural8. This may not be a problem in practice, but it would appear to be a violation of ANSI.

> (subtypep '(integer 0 20)
            '(integer 0 200))

T
T

> (subtypep (upgraded-array-element-type '(integer 0 20))
            (upgraded-array-element-type '(integer 0 200)))

NIL
T
jcbeaudoin commented 6 months ago

Seems that whoever implemented upgraded-array-element-type followed C style of integer type promotion. It's probably time to revise that in favor of more conformance to ANSI-CL instead.

jcbeaudoin commented 6 months ago

Could you please try the latest commits on master/HEAD (commit c8ccab5 and commit 789f741). They should solve the issue.

davidmullen commented 6 months ago

This works for me. There's just a corner case with nil:

> (upgraded-array-element-type nil)

BIT

Which is not in agreement with array-element-type:

> (array-element-type (make-array 0 :element-type nil))

NIL

Aside from that, the only remaining question is whether to do type normalization or not:

> (upgraded-array-element-type `(integer ,most-negative-fixnum ,most-positive-fixnum))

INTEGER64

Compared with:

> (upgraded-array-element-type 'fixnum)

FIXNUM

But this is probably fine.

jcbeaudoin commented 6 months ago

Nope! That was not fine. How about with this commit 6e289d4.

davidmullen commented 6 months ago

Yes, this works well for me.