Subject: bug#4835: 23.1; Improper `Invalid face reference' messages. Performance degraded.

From: "Drew Adams" <[email-masked]>
Date: Sat, 31 Oct 2009 15:40:26 -0700

> > Nonsense. You could say that it's crystal clear from the 
> > Elisp manual that _function_ was meant (since that's what
> > is says) and not symbol function or "function name" (which
> > would be a string). The code is bugged wrt its mission as
> > documented in the manual, which is the
> > closest thing we have to a spec.
> 
> Nonsense.  The manual is not a spec.

What else do we have to go on? The code is bugged, and no one knows what the
original intention was. How can you tell that function was _not_ intended and
using `fboundp' instead of `functionp' was _not_ just an oversight?

As I mentioned, the font-lock code _itself_ apparently uses a lambda form as a
function here. And the doc clearly says a function is called for, without any
further qualification.

You seem to be arbitrarily imposing a restriction which is unwarranted. Your
only reasons have been:

1. The doc string says "function name". That is wrong anyway, since that would
be a string (which won't work). And if the manual is no spec, a doc string is
even less of one.

You are favoring an incorrect doc string over a lengthy passage in the manual
that mentions the criterion of being a "function" multiple times. Why?

2. The currently bugged behavior, in at least one case when a lambda form is
tried. Why canonize this bugged behavior?

But:

a. Font-lock itself uses a lambda form (without an error, for some reason - just
why should be investigated).

b. The manual clearly says that a function is allowed here. It says so in
several places in several ways. "A function" means any function, not just a
symbol.

c. There is no comment in the code indicating that we intentionally use
`fboundp' for some reason. Nothing speaks about an intended restriction or what
problems `fboundp' avoids. Nothing indicates that `fboundp' is really intended
instead of `functionp', and not just an oversight.

d. Replacing `fboundp' by `functionp' might well fix the bug and thus align the
code with the manual's description of what it's supposed to do.

The remaining thing to do would then be to fix the doc string - which needs to
be fixed anyway.

Instead of fixing the bug (by allowing any function), you want to cast it in
concrete and document the bugged behavior as if it were intentional. Doesn't
make sense to me.

What does the code actually _do_ with this function? That is the real question.
Until we answer that question, there is no sense imposing an arbitrary
interpretation on things and adding unnecessary restrictions.

AFAICT, the code simply passes the function to `funcall'. Why is there any need
to restrict funcall's arg here to be an `fboundp' symbol?

You haven't given one reason why this should be limited to a symbol. Show why
`functionp' is problematic, and you might have an argument.

All you've shown is that the call to `fboundp' fails for a lambda form, so the
wrong code path is followed. The solution to that is to use `functionp' so the
right code path will be followed.