Subject: Re: bug#51577: 27.2; Regression: reproducible hang with face functions

From: Eli Zaretskii <[email-masked]>
Date: Fri, 05 Nov 2021 09:19:05 +0200

> From: Lars Ingebrigtsen <[email-masked]>
> Cc: [email-masked],  [email-masked]
> Date: Thu, 04 Nov 2021 23:56:16 +0100
> 
> Eli Zaretskii <[email-masked]> writes:
> 
> > Detecting loops in those cases would require recording face
> > specs/names we already saw in some list, and each time we get an
> > :inherit attribute, check if its value is already in the list.  Is
> > that worth our while, if the result will be a silent error message in
> > *Messages*?
> 
> Well, the result is that Emacs doesn't hang.  :-)  I'm not so worried
> about issuing any messages here.

I actually worried about the CPU cycles spent checking that end in
silence than about the silence itself.

> But I wonder -- could we do this (expensive) checking in defface (or
> rather custom-declare-face)?  That could be helpful -- evalling a
> circular face definition should signal an error.

Checking this at defface time could only solve some small subset of
the cases.  It wouldn't solve this one, for example, because the
original defface:

  (defface alt-region '((t :background "gray70" :inherit region))
    "..." :group 'faces)

is completely innocent.  It's only later, when the code does this:

    (apply #'set-face-attribute 'region frame plist)))

that it actually makes the 'region' face inherit from itself.

Similarly, inheritance loops can (and usually do) appear at run time,
because faces are added or their attributes modified or merged with
other faces.

So, bottom line, I don't think that detection at defface time will
help here.