internal-lisp-face-p is not very discriminating, is it?

internal-lisp-face-p is not very discriminating, is it?

[Juanma Barranquero]

ELISP> (facep 'my-face)
nil
ELISP> (put 'my-face 'face-alias 'my-face)
my-face
ELISP> (facep 'my-face)
[face unspecified unspecified unspecified unspecified unspecified
unspecified unspecified unspecified unspecified unspecified
unspecified unspecified unspecified unspecified unspecified
unspecified]

I would've expected the same behavior that `internal-lisp-face-empty-p':

ELISP> (internal-lisp-face-empty-p 'my-face)
*** Eval error ***  List contains a loop: my-face

Now, I've taken a look at xfaces.c and I understand why it is
happening: internal_lisp_face_p calls lframe_from_face_name passing a
0 for the signal_p argument, so it returns a default face instead of
signaling an error.

Still, it seems very wrong that just "face-alias"ing a symbol to
itself suddenly turns it into a (fake) face.

It is not possible to modify internal_lisp_face_p to pass signal_p  =
1 to lframe_from_face_name, because then dumping fails when loading
face.el. However, the attached minimal patch seems to work.

Does anyone see anything wrong with this change?

     Juanma



Index: src/xfaces.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xfaces.c,v
retrieving revision 1.360
diff -u -r1.360 xfaces.c
--- src/xfaces.c	26 May 2007 17:21:13 -0000	1.360
+++ src/xfaces.c	29 May 2007 14:34:37 -0000
@@ -3935,6 +3935,8 @@
 {
   Lisp_Object lface;

+  face = resolve_face_name (face, 1);
+
   if (!NILP (frame))
     {
       CHECK_LIVE_FRAME (frame);

Re: internal-lisp-face-p is not very discriminating, is it?

[Richard Stallman]

    Still, it seems very wrong that just "face-alias"ing a symbol to
    itself suddenly turns it into a (fake) face.

Lisp gives you a lot of rope to hang yourself.
At rather internal levels like this, we don't need to take steps
to defend against putting perverse values in data structures.

    It is not possible to modify internal_lisp_face_p to pass signal_p  =
    1 to lframe_from_face_name, because then dumping fails when loading
    face.el. However, the attached minimal patch seems to work.

    Does anyone see anything wrong with this change?

I have nothing against it if it works out well.

Re: internal-lisp-face-p is not very discriminating, is it?

[Juanma Barranquero]

On 6/3/07, Richard Stallman <rms@gnu.org> wrote:

> Lisp gives you a lot of rope to hang yourself.
> At rather internal levels like this, we don't need to take steps
> to defend against putting perverse values in data structures.

Well, other internal functions visible to lisp do take efforts to
defend (internal-lisp-face-empty-p, for example).

Moreover, (put 'my-face 'face-alias 'another-face) is *the* high level
way to create a face alias; there's no deffacealias equivalent to
`defalias' and `defvaralias'. And `facep' is the high level way to
test for a face.

> I have nothing against it if it works out well.

OK, I've installed it, and also a subsequent pach to `face-id' to make
it follow the face-alias chain to find the ID. We discussed it a while
ago:

Me: Shouldn't `face-id' return an answer also for face aliases?
You: In principle, that sounds right, but I don't want to risk the change now.

Both changes have been working fine in my setup, and they're easy
enough to revert, should something unexpected happen.

             Juanma