bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

[Andrea Corallo]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

Hello all,

while performing some development I noticed that 'number-or-marker' is
mentioned as a type in 'cl--typeof-types'.

Unfortunatelly its entry is missing in 'cl-deftype-satisfies' (see
cl-macs.el:3469).

My question is, why do we consider 'number-or-marker' in the first place
a type if we support the or syntax in `cl-typep' like
(cl-typep 3 '(or marker number)) ?

I'd like to fix this inconsistency in order to progress with my
development, originally I worked out the attached patch but I now
suspect that (unless there's a specific reason) we should just remove
'number-or-marker' as a type entirely instead.

WDYT? Thanks

  Andrea

PS also I think we have a similar issue/question with
'integer-or-marker'.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-two-missing-number-or-marker-entries-to-the-cl-m.patch --]
[-- Type: text/x-diff, Size: 3563 bytes --]

From 05d85f19e51c15df8824df8e8608784ec79b37cf Mon Sep 17 00:00:00 2001
From: Andrea Corallo <acorallo@gnu.org>
Date: Wed, 18 Oct 2023 16:10:08 +0200
Subject: [PATCH] Add two missing 'number-or-marker' entries to the cl
 machinery.

Assuming 'number-or-marker' is a type (as present multiple times in
cl--typeof-types) adding some missing entries for coherency.

* lisp/emacs-lisp/cl-preloaded.el (cl--typeof-types): Add
'number-or-marker' as supertype of 'number' in the 'float' branch.

* lisp/emacs-lisp/cl-macs.el (cl-deftype-satisfies): Add
'number-or-marker'.

* test/lisp/emacs-lisp/comp-cstr-tests.el (comp-cstr-typespec-tests-alist):
Update test.

* test/src/comp-tests.el (comp-tests-type-spec-tests): Update two testes.
---
 lisp/emacs-lisp/cl-macs.el              | 3 ++-
 lisp/emacs-lisp/cl-preloaded.el         | 4 ++--
 test/lisp/emacs-lisp/comp-cstr-tests.el | 2 +-
 test/src/comp-tests.el                  | 4 ++--
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 8025a64f1bf..722d561b9f4 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -3502,7 +3502,8 @@ cl--macroexp-fboundp
                  (symbol	. symbolp)
                  (vector	. vectorp)
                  (window	. windowp)
-                 ;; FIXME: Do we really want to consider this a type?
+                 ;; FIXME: Do we really want to consider these types?
+                 (number-or-marker . number-or-marker-p)
                  (integer-or-marker . integer-or-marker-p)
                  ))
   (put type 'cl-deftype-satisfies pred))
diff --git a/lisp/emacs-lisp/cl-preloaded.el b/lisp/emacs-lisp/cl-preloaded.el
index 676326980aa..96e288db7d5 100644
--- a/lisp/emacs-lisp/cl-preloaded.el
+++ b/lisp/emacs-lisp/cl-preloaded.el
@@ -58,8 +58,8 @@ cl--typeof-types
     ;; Markers aren't `numberp', yet they are accepted wherever integers are
     ;; accepted, pretty much.
     (marker number-or-marker atom)
-    (overlay atom) (float number atom) (window-configuration atom)
-    (process atom) (window atom)
+    (overlay atom) (float number number-or-marker atom)
+    (window-configuration atom) (process atom) (window atom)
     ;; FIXME: We'd want to put `function' here, but that's only true
     ;; for those `subr's which aren't special forms!
     (subr atom)
diff --git a/test/lisp/emacs-lisp/comp-cstr-tests.el b/test/lisp/emacs-lisp/comp-cstr-tests.el
index a4f282fcfef..d2f552af6fa 100644
--- a/test/lisp/emacs-lisp/comp-cstr-tests.el
+++ b/test/lisp/emacs-lisp/comp-cstr-tests.el
@@ -191,7 +191,7 @@
       ;; 74
       ((and boolean (or number marker)) . nil)
       ;; 75
-      ((and atom (or number marker)) . (or marker number))
+      ((and atom (or number marker)) . number-or-marker)
       ;; 76
       ((and symbol (or number marker)) . nil)
       ;; 77
diff --git a/test/src/comp-tests.el b/test/src/comp-tests.el
index 4444ab61219..31871bb2eec 100644
--- a/test/src/comp-tests.el
+++ b/test/src/comp-tests.el
@@ -977,7 +977,7 @@ comp-tests-check-ret-type-spec
          (if (= x y)
              x
            'foo))
-       '(or (member foo) marker number))
+       '(or (member foo) marker-or-number))
 
       ;; 14
       ((defun comp-tests-ret-type-spec-f (x)
@@ -1117,7 +1117,7 @@ comp-tests-check-ret-type-spec
       ((defun comp-tests-ret-type-spec-f (x)
 	 (when (> x 1.0)
 	   x))
-       '(or null marker number))
+       '(or null number-or-marker))
 
       ;; 36
       ((defun comp-tests-ret-type-spec-f (x y)
-- 
2.25.1

bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> My question is, why do we consider 'number-or-marker' in the first place
> a type if we support the or syntax in `cl-typep' like
> (cl-typep 3 '(or marker number)) ?

I'm not sure I can give a good answer in general, but I can tell you
some reasons that explain some of what we see:

- There is a `number-or-marker-p` primitive and `cl-typep` doesn't know
  how to use it for `(or number marker)`.
- method specializers (currently) can't be `(or number marker)` but can be
  `number-or-marker`.

> I'd like to fix this inconsistency in order to progress with my
> development, originally I worked out the attached patch but I now
> suspect that (unless there's a specific reason) we should just remove
> 'number-or-marker' as a type entirely instead.

I'd lean towards keeping it :-)

> PS also I think we have a similar issue/question with
> 'integer-or-marker'.

Yup.


        Stefan

bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

[Andrea Corallo]

[re-replaying as for some reason our responses didn't reach the list]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> My question is, why do we consider 'number-or-marker' in the first place
>> a type if we support the or syntax in `cl-typep' like
>> (cl-typep 3 '(or marker number)) ?
>
> I'm not sure I can give a good answer in general, but I can tell you
> some reasons that explain some of what we see:
>
> - There is a `number-or-marker-p` primitive and `cl-typep` doesn't know
>   how to use it for `(or number marker)`.

Well we could just remove 'number-or-marker-p' 😃

> - method specializers (currently) can't be `(or number marker)` but can be
>   `number-or-marker`.

Okay this is more difficult to fix... :/

>> I'd like to fix this inconsistency in order to progress with my
>> development, originally I worked out the attached patch but I now
>> suspect that (unless there's a specific reason) we should just remove
>> 'number-or-marker' as a type entirely instead.
>
> I'd lean towards keeping it :-)

I see your point, actually my main drive is to make the situation more
coherent so I'm unblocked in the first place, just the method
specializer functionality is a blocker for removing 'number-or-marker'.

I think adding 'number-or-marker' where missing is probably the best
solution for now.

Thanks

  Andrea

bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

[Andrea Corallo]

Andrea Corallo <acorallo@gnu.org> writes:

> [re-replaying as for some reason our responses didn't reach the list]
>
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> My question is, why do we consider 'number-or-marker' in the first place
>>> a type if we support the or syntax in `cl-typep' like
>>> (cl-typep 3 '(or marker number)) ?
>>
>> I'm not sure I can give a good answer in general, but I can tell you
>> some reasons that explain some of what we see:
>>
>> - There is a `number-or-marker-p` primitive and `cl-typep` doesn't know
>>   how to use it for `(or number marker)`.
>
> Well we could just remove 'number-or-marker-p' 😃
>
>> - method specializers (currently) can't be `(or number marker)` but can be
>>   `number-or-marker`.
>
> Okay this is more difficult to fix... :/
>
>>> I'd like to fix this inconsistency in order to progress with my
>>> development, originally I worked out the attached patch but I now
>>> suspect that (unless there's a specific reason) we should just remove
>>> 'number-or-marker' as a type entirely instead.
>>
>> I'd lean towards keeping it :-)
>
> I see your point, actually my main drive is to make the situation more
> coherent so I'm unblocked in the first place, just the method
> specializer functionality is a blocker for removing 'number-or-marker'.
>
> I think adding 'number-or-marker' where missing is probably the best
> solution for now.

Okay with a567faf4c2b I added 'number-or-marker' where it was missing.
Closing this, happy to reopen if necessary.

Thanks!

  Andrea

bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> - There is a `number-or-marker-p` primitive and `cl-typep` doesn't know
>>>   how to use it for `(or number marker)`.
>> Well we could just remove 'number-or-marker-p' 😃

Indeed, or we could teach `cl-typep` how to make use of it.

>> I see your point, actually my main drive is to make the situation more
>> coherent so I'm unblocked in the first place, just the method
>> specializer functionality is a blocker for removing 'number-or-marker'.

That's what I had understood, and indeed there were some loose ends there.

> Okay with a567faf4c2b I added 'number-or-marker' where it was missing.

Thanks.  I also added corresponding `integer-or-marker`.


        Stefan

bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>> - There is a `number-or-marker-p` primitive and `cl-typep` doesn't know
>>>>   how to use it for `(or number marker)`.
>>> Well we could just remove 'number-or-marker-p' 😃
>
> Indeed, or we could teach `cl-typep` how to make use of it.
>
>>> I see your point, actually my main drive is to make the situation more
>>> coherent so I'm unblocked in the first place, just the method
>>> specializer functionality is a blocker for removing 'number-or-marker'.
>
> That's what I had understood, and indeed there were some loose ends there.
>
>> Okay with a567faf4c2b I added 'number-or-marker' where it was missing.
>
> Thanks.  I also added corresponding `integer-or-marker`.

Hi Stefan thanks,

I've to question now the following entry introduced in
`cl--typeof-types' for correctness:

(integer number integer-or-marker number-or-marker atom)

The doc says :
Each element has the form (TYPE . SUPERTYPES) where TYPE is one of
the symbols returned by ‘type-of’, and SUPERTYPES is the list of its
supertypes from the most specific to least specific.

And indeed not every 'number' is an 'integer-or-marker'.

I suspect that's the reason why this commit introduces few failures in
the native-comp testsuite.

At this point of the night I'm not sure if and how 'integer-or-marker'
can be added correctly to this representation.

Best Regards

  Andrea

bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I've to question now the following entry introduced in
> `cl--typeof-types' for correctness:
>
> (integer number integer-or-marker number-or-marker atom)
>
> The doc says :
> Each element has the form (TYPE . SUPERTYPES) where TYPE is one of
> the symbols returned by ‘type-of’, and SUPERTYPES is the list of its
> supertypes from the most specific to least specific.
>
> And indeed not every 'number' is an 'integer-or-marker'.

The subtype relation doesn't derive from a tree but a DAG so the
SUPERTYPES represents *a* linearization of the parents but just because
`number` appears before `integer-or-marker` doesn't mean that it's
a subtype of it.  It just means I judged that it should be considered
"more specific".

The same effect is in play for:

    (null symbol list sequence atom)

where clearly `symbol` is a subtype of neither `list` nor `sequence`.

> I suspect that's the reason why this commit introduces few failures in
> the native-comp testsuite.

If you need to know the list of supertypes of `number`, we could add
that info explicitly, or you could "guess" it as the intersection of all
the types that appear after `number` in `cl--typeof-types`:

    (integer number integer-or-marker number-or-marker atom)
    [...]
    (float number number-or-marker atom)
    [...]

i.e. the intersection of (integer-or-marker number-or-marker atom)
and (number-or-marker atom).


        Stefan

bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I've to question now the following entry introduced in
>> `cl--typeof-types' for correctness:
>>
>> (integer number integer-or-marker number-or-marker atom)
>>
>> The doc says :
>> Each element has the form (TYPE . SUPERTYPES) where TYPE is one of
>> the symbols returned by ‘type-of’, and SUPERTYPES is the list of its
>> supertypes from the most specific to least specific.
>>
>> And indeed not every 'number' is an 'integer-or-marker'.
>
> The subtype relation doesn't derive from a tree but a DAG so the
> SUPERTYPES represents *a* linearization of the parents but just because
> `number` appears before `integer-or-marker` doesn't mean that it's
> a subtype of it.  It just means I judged that it should be considered
> "more specific".
>
> The same effect is in play for:
>
>     (null symbol list sequence atom)
>
> where clearly `symbol` is a subtype of neither `list` nor `sequence`.

Hi Stefan,

I understand now thanks, the trouble is that the hierarchy is not a tree
but a DAG and these are just single linearizations.

I think we should improve this part of the docstring as doesn't sound
complete to me.

Also shouldn't we add (null boolean symbol atom) to cover this other
path as well?

>> I suspect that's the reason why this commit introduces few failures in
>> the native-comp testsuite.
>
> If you need to know the list of supertypes of `number`, we could add
> that info explicitly, or you could "guess" it as the intersection of all
> the types that appear after `number` in `cl--typeof-types`:
>
>     (integer number integer-or-marker number-or-marker atom)
>     [...]
>     (float number number-or-marker atom)
>     [...]
>
> i.e. the intersection of (integer-or-marker number-or-marker atom)
> and (number-or-marker atom).

I think I understand how to do it, OTOH might be better to have the
hierarchy made explicit somewhere?  Still have to think to make an
opinion on this.

Thanks

  Andrea

bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I think we should improve this part of the docstring as doesn't sound
> complete to me.

Suggestions welcome.

> Also shouldn't we add (null boolean symbol atom) to cover this other
> path as well?

You mean add `boolean` in there?
No objection on my side.


        Stefan

bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I think we should improve this part of the docstring as doesn't sound
>> complete to me.
>
> Suggestions welcome.
>
>> Also shouldn't we add (null boolean symbol atom) to cover this other
>> path as well?
>
> You mean add `boolean` in there?
> No objection on my side.

Sorry I'm just trying to understand, I'm happy to write patches but I
need to understand first.

IIUC for this sub part of the hierarchy we have:

   ┌─────► t ◄─────┐
   │               │
   │              atom
   │               ▲
   │               │
sequence         symbol
   ▲               ▲
   │               │
   │               │
 list ◄───┐      boolean
   ▲      │        ▲
   │      │        │
   │      │        │
   │      └────────┤
   │               │
  cons            null


now I'm fine with having say (getting the relevant parts from
`cl--typeof-types' and adding boolean in the null's one as you
suggested):

(null boolean symbol list sequence atom)
(cons list sequence)
(symbol atom)

as entries, but how are these sufficient to reconstruct this hierarchy?

(null boolean symbol list sequence atom) is not a linearization of the
DAG, is just (TYPE . SUPERTYPES) where SUPERTYPES have no specific
order.  Am I wrong?

How can current code (say dispatch on builtin types) work if we can't
infer if 'sequence' is higher or lower in the hierarchy respect to
'list'?

I think the original idea (as expressed by the doc) of having "the list
of its supertypes from the most specific to least specific" works for
reconstructing the DAG if we have one entry per path to the top, say:

(null boolean symbol atom)
(null list sequence)
(cons list sequence)

But I might be easyly missing many things here.

Thanks!

(A confused) Andrea

bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> (null boolean symbol list sequence atom) is not a linearization of the
> DAG,

AFAIK it is.  A linearization can be "depth first" or "breadth" first or
any mix of it.  It's basically any total order compatible with the
partial order imposed by the DAG.

> is just (TYPE . SUPERTYPES) where SUPERTYPES have no specific
> order.  Am I wrong?

The order is from most specific to least specific.  For types which are
"incomparable", the choice is a judgment call.  The above ordering seems
acceptable to me since `atom` is arguably a larger type than `sequence`,
but

    (null boolean symbol atom list sequence)

is acceptable as well.

This said, in general we'd like to avoid situations where type T1
appears before type T2 in one case and after it in another (it may not
always be avoidable, sadly).  And `atom` appears after `sequence` in
cases like `vector` and `string`, so I think we should prefer

    (null boolean symbol list sequence atom)

over

    (null boolean symbol atom list sequence)

> How can current code (say dispatch on builtin types) work if we can't
> infer if 'sequence' is higher or lower in the hierarchy respect
> to 'list'?

The linearization is specific to a given type, so we use the ordering
provided by `cl--typeof-types` and we never care to know the full DAG.

[ BTW, we should be able to infer that `list` is lower because every
  time it appears in `cl--typeof-types` it is always followed by
  `sequence`.  ]

> I think the original idea (as expressed by the doc) of having "the list
> of its supertypes from the most specific to least specific" works for
> reconstructing the DAG if we have one entry per path to the top, say:
>
> (null boolean symbol atom)
> (null list sequence)
> (cons list sequence)

Indeed.  But then it doesn't say how to make this into a total order
when `cl-defmethod` needs to decide which method should come first.

IOW, there is supposedly a DAG, but until now we've never actually
needed the DAG itself, instead we've needed only a linearization of the
ancestors of the leaves of that DAG, which is what
`cl--typeof-types` stores.

If you need the DAG, then maybe we need to store more info (tho I still
suspect we should be able to extract that info from `cl--typeof-types`).


        Stefan

bug#66615: 30.0.50; Inconsistent 'number-or-marker' type definition in the cl- machinery

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> (null boolean symbol list sequence atom) is not a linearization of the
>> DAG,
>
> AFAIK it is.  A linearization can be "depth first" or "breadth" first or
> any mix of it.  It's basically any total order compatible with the
> partial order imposed by the DAG.
>
>> is just (TYPE . SUPERTYPES) where SUPERTYPES have no specific
>> order.  Am I wrong?
>
> The order is from most specific to least specific.  For types which are
> "incomparable", the choice is a judgment call.  The above ordering seems
> acceptable to me since `atom` is arguably a larger type than `sequence`,
> but
>
>     (null boolean symbol atom list sequence)
>
> is acceptable as well.
>
> This said, in general we'd like to avoid situations where type T1
> appears before type T2 in one case and after it in another (it may not
> always be avoidable, sadly).  And `atom` appears after `sequence` in
> cases like `vector` and `string`, so I think we should prefer
>
>     (null boolean symbol list sequence atom)
>
> over
>
>     (null boolean symbol atom list sequence)
>
>> How can current code (say dispatch on builtin types) work if we can't
>> infer if 'sequence' is higher or lower in the hierarchy respect
>> to 'list'?
>
> The linearization is specific to a given type, so we use the ordering
> provided by `cl--typeof-types` and we never care to know the full DAG.
>
> [ BTW, we should be able to infer that `list` is lower because every
>   time it appears in `cl--typeof-types` it is always followed by
>   `sequence`.  ]
>
>> I think the original idea (as expressed by the doc) of having "the list
>> of its supertypes from the most specific to least specific" works for
>> reconstructing the DAG if we have one entry per path to the top, say:
>>
>> (null boolean symbol atom)
>> (null list sequence)
>> (cons list sequence)
>
> Indeed.  But then it doesn't say how to make this into a total order
> when `cl-defmethod` needs to decide which method should come first.
>
> IOW, there is supposedly a DAG, but until now we've never actually
> needed the DAG itself, instead we've needed only a linearization of the
> ancestors of the leaves of that DAG, which is what
> `cl--typeof-types` stores.
>
> If you need the DAG, then maybe we need to store more info (tho I still
> suspect we should be able to extract that info from `cl--typeof-types`).

Yep, I think I'll go for adding the DAG somewhere else, extratcting it
from `cl--typeof-types` doesn't look trivial at all (if even possible)
and its definition is at this point kind of weak IMO for that.

Thanks

  Andrea