Subject: Re: bug#77725: 31.0.50; Add support for types accepted by `cl-typep' to cl-generic?

From: Stefan Monnier <[email-masked]>
Date: Wed, 07 May 2025 12:19:22 -0400

>>>            (condition-case-unless-debug e
>>>                (cl-typep object type)
>>> -           (error (setq cl--type-list (delq type cl--type-list))
>>> -                  (warn  "cl-types-of %S: %s"
>>> -                         type (error-message-string e)))))
>>> +           (error
>>> +            ;; Temporarily raise the recursion limit to avoid
>>> +            ;; another recursion error while processing error.
>>> +            (let ((max-lisp-eval-depth (+ 800 max-lisp-eval-depth)))
>> We almost never do that in ELisp except in very specific circumstances
>> where we decide it's really worth the trouble (even in though in theory
>> it could be necessary in pretty much all error handlers).
>> Is there a reason why it's more necessary here than usual?
>> Maybe a simpler fix is to replace `warn` with `message`?
>
> Yes, `message' will definitely work better, but errors will be less
> highlighted.  But a message is always better than nothing ;-)
>
> I wouldn't want ill-defined types entering infinite recursion to
> break error handling, just because the depth limit is reached,

As I said, this is a general problem.  Do you have reasons to believe it
is much more prevalent here than elsewhere?

Also, to some extent it could be fixed in a general way (and indeed we
do have code in `Fsignal` to bump up the depth limit when running some
of the error handling code, so it may just need to be adjusted to cover
more cases).
Maybe it should be a separate bug-report/feature-request.

> thus preventing `warn' to be called.  And no error handling in
> `cl-types-of' is not an option IMHO, because Emacs risks to become
> unusable if methods based on cl-types, used for example in a display
> context, crash.

To the extend the method dispatch only looks at types used in existing
methods, this shouldn't be a big issue: hopefully a method wouldn't use
a type whose definition is broken, and if it does we'd hopefully
discover it quickly so it would be fixed quickly as well.

> In fact, I don't like the idea of ​​well-defined
> things being disrupted by ill-defined ones ;-)

But there's a trade-off.  Also it's easier to add error checking after
the fact than to remove it after the fact, so I'd strongly prefer to let
errors be signaled for now.

> While testing, I discovered two nasty bugs in `cl-types-of' illustrated
> by the following recipe:
>
> (cl-deftype T1 ()
>   "Ill-defined type."
>   '(satisfies unknown-predicate))
>
> (cl-types-of 12)
> Raise this warning as expected:
> Warning (emacs): cl-types-of T1: Symbol’s function definition is void: unknown-predicate
>
> But returns this incorrect list of types:
> => (T1 kmacro-register frame-register fixnum integer number
> integer-or-marker number-or-marker atom t)

Oops, good catch, thanks.

> (cl-defmethod test-T1 ((obj T1))
>   (format "test-T1 %S" obj))
> Should signal an "unknown specializer" for T1, but do not.

I don't think it should: T1 *is* a known specializer.  It's just one
whose definition is broken, but we'll only discover that when we
actually try to use it.

> The second problem is a nasty side effect of `nreverse' which
> causes a wrong key pushed in the `gtype--unique' hash table.

Duh, indeed!

> The attached patch solved both issues for me.  To compute the DAG,
> I preferred to use a `dolist' loop rather than a `mapcar'+`reverse'
> combination, to avoid going through the list twice.

I like `mapcar`, but indeed here we do want the reversal that
dolist+push gives us "for free".  I haven't pushed your patch yet
because git.sv.gnu.org is not responding, but I'll do it "soonish".

> Please find attached another small patch for the method
> `cl-generic-generalizers' in cl-lib.el.
>
> It ensures that the derived type is still present in the
> `cl--type-list', in order to signal an "unkonw specializer"
> error, if the type was removed from the list because it
> was in error.

But this can lead to the weird "unknown specializer" error for a type
which *is* known (just defined poorly enough that it signaled an error
at some point).

        Stefan