ruricolist / serapeum

Utilities beyond Alexandria
MIT License
420 stars 41 forks source link

Several nice-to-have changes in defining-types.lisp #87

Closed astrangeguy closed 2 years ago

astrangeguy commented 3 years ago

Didn't want to open up 4 issues separately for optimizations/feature-requests.

ruricolist commented 3 years ago

This looks good, but before I merge it I need to do some testing to make sure that including the supertype in the type disjunction doesn't break exhaustiveness checking.

astrangeguy commented 3 years ago

bce2807 does indeed break exhaustiveness checking since one could use defstruct to define a new subtype of <super>. This is unsupported according to the docs of defconstructor though.

I solved it by creating a new derived type serapeum:union-type that acts like a type disjunction during exhaustiveness checking but is simply the super-class type everywhere else.

Tests added to check if typecase-of warns correctly with nested union type specifiers.

ruricolist commented 3 years ago

I've merged the other PR that fixes a CCL bug. I'm not merging this one yet, however, as the fix for exhaustiveness checking doesn't work on SBCL -- I'm not sure why yet.

astrangeguy commented 3 years ago

I've had no problems with SBCL 2.1.0 on Linux with the fix, How does it fail?

ruricolist commented 3 years ago

On SBCL 2.0.9:

; file: /home/pmr/quicklisp/local-projects/serapeum/tests/defining-types.lisp
; in: DEFUN KDR
;     (SERAPEUM:MATCH-OF SERAPEUM.TESTS::LISZT
;         SERAPEUM.TESTS::L
;       ((SERAPEUM.TESTS::KONS SERAPEUM.TESTS::_ SERAPEUM.TESTS::B)
;        SERAPEUM.TESTS::B)
;       (SERAPEUM.TESTS::KNIL SERAPEUM.TESTS::KNIL))
; 
; caught WARNING:
;   Non-exhaustive match: (OR KONS KNIL) is a proper subtype of LISZT.
astrangeguy commented 3 years ago

Ah, got it. match-of does manual destructuring of its type argument. Is there any other macro that checks for exhaustiveness, other than typecase-of and match-of?

astrangeguy commented 3 years ago

Ok, seems to me that SBCL always caches type expansions. Scrapping that commit. Ill leave the new tests in the PR, since they explicitly test exhaustiveness with defunions.

Gonna see if there is any sane way to still keep exhaustiveness checking whilst compiling to <super>