Simplify the output of combine-values-type using subtypep

[digikar99]

This avoids creating chains like (and t t t some-non-t-type) and aids debuggability.

[alex-gutev]

This causes some test failures.

Failure Details:
 --------------------------------
 COMBINE-VALUES-TYPE-ALLOW-OTHER-KEYS in VALUES-TYPES [Test combining two VALUES types, one with &ALLOW-OTHER-KEYS]: 

(COMBINE-VALUES-TYPES 'OR '(VALUES INTEGER &REST FIXNUM &ALLOW-OTHER-KEYS)
                      '(VALUES &OPTIONAL NUMBER))

 evaluated to 

(VALUES &OPTIONAL INTEGER &REST (OR NIL FIXNUM) &ALLOW-OTHER-KEYS)

 which is not 

VALUES-TYPE=

 to 

(VALUES &OPTIONAL (OR INTEGER NUMBER) &REST (OR FIXNUM NIL) &ALLOW-OTHER-KEYS)

 --------------------------------
 --------------------------------
 COMBINE-VALUES-TYPE-REST-OPTIONAL-3 in VALUES-TYPES [Test combining two VALUES types, both with &REST and &OPTIONAL]: 

(COMBINE-VALUES-TYPES 'OR '(VALUES &OPTIONAL INTEGER &REST FIXNUM)
                      '(VALUES NUMBER &OPTIONAL (INTEGER 3) &REST INTEGER))

 evaluated to 

(VALUES &OPTIONAL INTEGER (OR FIXNUM (INTEGER 3)) &REST (OR FIXNUM INTEGER))

 which is not 

VALUES-TYPE=

 to 

(VALUES &OPTIONAL (OR INTEGER NUMBER) (OR FIXNUM (INTEGER 3)) &REST
        (OR FIXNUM INTEGER))

 --------------------------------
 --------------------------------
 COMBINE-VALUES-TYPE-REST-OPTIONAL-2 in VALUES-TYPES [Test combining two VALUES types, one with &REST and &OPTIONAL]: 

(COMBINE-VALUES-TYPES 'OR '(VALUES INTEGER &REST FIXNUM)
                      '(VALUES NUMBER &OPTIONAL (INTEGER 3)))

 evaluated to 

(VALUES INTEGER &OPTIONAL (OR FIXNUM (INTEGER 3)) &REST (OR FIXNUM NIL))

 which is not 

VALUES-TYPE=

 to 

(VALUES (OR INTEGER NUMBER) &OPTIONAL (OR FIXNUM (INTEGER 3)) &REST
        (OR FIXNUM NIL))

 --------------------------------
 --------------------------------
 COMBINE-VALUES-TYPE-REST-OPTIONAL in VALUES-TYPES [Test combining two VALUES types, one with &REST and &OPTIONAL]: 

(COMBINE-VALUES-TYPES 'OR '(VALUES &OPTIONAL INTEGER &REST FIXNUM)
                      '(VALUES NUMBER))

 evaluated to 

(VALUES &OPTIONAL INTEGER &REST (OR NIL FIXNUM))

 which is not 

VALUES-TYPE=

 to 

(VALUES &OPTIONAL (OR INTEGER NUMBER) &REST (OR FIXNUM NIL))

 --------------------------------
 --------------------------------
 COMBINE-VALUES-TYPE-REST-LAST in VALUES-TYPES [Test combining two VALUES types, one with &REST at last element]: 

(COMBINE-VALUES-TYPES 'OR '(VALUES INTEGER &REST) '(VALUES NUMBER))

 evaluated to 

(VALUES INTEGER &REST (OR NIL T))

 which is not 

VALUES-TYPE=

 to 

(VALUES (OR INTEGER NUMBER) &REST (OR T NIL))

 --------------------------------
 --------------------------------
 COMBINE-VALUES-TYPE-REST-3 in VALUES-TYPES [Test combining two VALUES types, both with &REST]: 

(COMBINE-VALUES-TYPES 'OR '(VALUES INTEGER &REST FIXNUM)
                      '(VALUES NUMBER (INTEGER 3) &REST INTEGER))

 evaluated to 

(VALUES INTEGER (OR FIXNUM (INTEGER 3)) &REST (OR FIXNUM INTEGER))

 which is not 

VALUES-TYPE=

 to 

(VALUES (OR INTEGER NUMBER) (OR FIXNUM (INTEGER 3)) &REST (OR FIXNUM INTEGER))

 --------------------------------
 --------------------------------
 COMBINE-VALUES-TYPE-REST-2 in VALUES-TYPES [Test combining two VALUES types, one with &REST]: 

(COMBINE-VALUES-TYPES 'OR '(VALUES INTEGER &REST FIXNUM)
                      '(VALUES NUMBER (INTEGER 3)))

 evaluated to 

(VALUES INTEGER (OR FIXNUM (INTEGER 3)) &REST (OR FIXNUM NIL))

 which is not 

VALUES-TYPE=

 to 

(VALUES (OR INTEGER NUMBER) (OR FIXNUM (INTEGER 3)) &REST (OR FIXNUM NIL))

 --------------------------------
 --------------------------------
 COMBINE-VALUES-TYPE-REST in VALUES-TYPES [Test combining two VALUES types, one with &REST]: 

(COMBINE-VALUES-TYPES 'OR '(VALUES INTEGER &REST FIXNUM) '(VALUES NUMBER))

 evaluated to 

(VALUES INTEGER &REST (OR NIL FIXNUM))

 which is not 

VALUES-TYPE=

 to 

(VALUES (OR INTEGER NUMBER) &REST (OR FIXNUM NIL))

 --------------------------------
 --------------------------------
 COMBINE-VALUES-OPTIONAL-TYPES-2 in VALUES-TYPES [Test combining two VALUES types, second with &OPTIONAL]: 

(COMBINE-VALUES-TYPES 'OR '(VALUES INTEGER &OPTIONAL CHARACTER)
                      '(VALUES (EQL 10) (EQL #\x)))

 evaluated to 

(VALUES (EQL 10) &OPTIONAL (EQL #\x))

 which is not 

VALUES-TYPE=

 to 

(VALUES (OR INTEGER (EQL 10)) &OPTIONAL (OR CHARACTER (EQL #\x)))

 --------------------------------
 --------------------------------
 COMBINE-VALUES-TYPE-WITH-SIMPLE in VALUES-TYPES [Test combining a VALUES types with simple type]: 

(COMBINE-VALUES-TYPES 'OR '(EQL 10) '(VALUES INTEGER CHARACTER))

 evaluated to 

(VALUES (EQL 10) (OR NIL CHARACTER))

 which is not 

VALUES-TYPE=

 to 

(VALUES (OR (EQL 10) INTEGER) (OR NIL CHARACTER))

 --------------------------------
 --------------------------------
 COMBINE-SIMPLE-WITH-VALUES-TYPES in VALUES-TYPES [Test combining a simple type with a VALUES types]: 

(COMBINE-VALUES-TYPES 'OR '(VALUES INTEGER CHARACTER) '(EQL 10))

 evaluated to 

(VALUES (EQL 10) (OR NIL CHARACTER))

 which is not 

VALUES-TYPE=

 to 

(VALUES (OR INTEGER (EQL 10)) (OR CHARACTER NIL))

[digikar99]

Do we rewrite the tests? Also, is there an opinion about whether this should rather be done by specifically focusing on and and or and letting the other combinators use the combinations as defined before this PR?

[alex-gutev]

If the actual output is equivalent to the expected output, but simpler, then yes we should rewrite the tests. However, some of the outputs don't look right to me. For example in the last failed test (EQL 10) is not equivalent to (OR INTEGER (EQL 10)) since the latter type specifier includes the value 11 and any other integer value but the former only includes the value 10. If anything the simplified type specifier should be just INTEGER. Also (OR INTEGER NUMBER), in the first failed test, is not equivalent to INTEGER.

Keep in mind the tests are using alexandria:type= to compare each type specifier in the values type specifiers, and not just a symbol/list comparison.

[digikar99]

Other than the following two tests, the rest should pass now.

https://github.com/alex-gutev/cl-form-types/blob/11c725e514013a6a27afd708c60ee980df3a55a9/test/values-types.lisp#L65-L74

[alex-gutev]

These two tests can be rewritten to test with alexandria:type= rather than equal. Also since we're expecting a simple output we change the expected output of the first test from '(and integer (eql 10)) to '(eql 10).

[digikar99]

Does that look good?

[alex-gutev]

Yes that looks good. All tests pass now. Thanks for the contribution.