GNU bug report logs - #77725
31.0.50; Add support for types accepted by `cl-typep' to cl-generic?

Previous Next

Package: emacs;

Reported by: David Ponce <da_vid <at> orange.fr>

Date: Fri, 11 Apr 2025 07:16:01 UTC

Severity: normal

Found in version 31.0.50

Full log


View this message in rfc822 format

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: David Ponce <da_vid <at> orange.fr>
Cc: 77725 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: bug#77725: 31.0.50; Add support for types accepted by `cl-typep' to cl-generic?
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





This bug report was last modified 10 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.