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

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

[Drew Adams]

[-- Attachment #1: Type: text/plain, Size: 6542 bytes --]

This sure seems like a bug to me. If not, please tell me what the
problem is.
 
Load the attached file, move point to, say, column 15 in some buffer
(e.g. *scratch*), then do `M-x column-marker-1'. Move point a bit, to
be sure to see the column highlighting.
 
Check buffer `*Messages*'. You will see zillions of messages `Invalid
face reference', logged by the C function `merge_face_ref' (in
`xfaces.c').  If you click the mouse in `*Messages*', zillions more
such messages will be displayed. This message logging degrades Emacs
performance considerably.
 
Also, binding `message-log-max' to nil has no effect - it does not
inhibit the message logging. That seems like a bug in itself.  But
perhaps you will say that this message is coming from the display
engine, and that it is appropriate for the display code to ignore
`message-log-max' (but why, especially if it impacts performance so
much?).  At the very least, there is a doc bug, since neither the
manual nor the doc string say anything about the display code ignoring
this variable.
 
Anyway, displaying the message at all seems like a bug in this case.
But if not, I would appreciate understanding what is wrong with the
code I'm using. In the attached code, `M-x column-marker-1' calls:
 
(column-marker-internal
  'column-marker-1
  (1+ (current-column)) 'column-marker-1)
 
That sets the value of symbol `column-marker-1' to the following list
- call it KEYWORDS, where COL is the column that point was in when you
did `M-x column-marker-1' (e.g. 15).
 
KEYWORDS:
 
(((lambda (end)
    (let ((start (point))
          (message-log-max nil)) ; (Has no effect.)
      (when (> end (point-max))
        (setq end (point-max)))
      (unless (< (current-column) COL)
        (forward-line 1))
      (when (< (current-column) COL)
        (move-to-column COL))
      (while (and (< (current-column) COL)
                  (< (point) end)
                  (= 0 (+ (forward-line 1)
                          (current-column))))
        (move-to-column COL))
      (if (and (= COL (current-column))
               (<= (point) end)
               (> (point) start))
          (progn (set-match-data
                  (list (1- (point)) (point)))
                 t)
        (goto-char start)
        nil)))
  (0 column-marker-1 prepend t)))
 
And then `column-marker-internal' calls this, where KEYWORDS is the
list shown above:
 
(font-lock-add-keywords nil KEYWORDS t)
(font-lock-fontify-buffer)
 
The effect is to highlight the column where you issued the command. It
works well, apart from the message logging and its performance impact.
 
The `font-lock-keywords' entry used has a lambda form as car and this
as cadr: (0 column-marker-1 prepend t). Face `column-marker-1' is
defined as follows:
 
(defface column-marker-1 '((t (:background "gray")))
  "Face used for a column marker.  Usually a background color."
  :group 'faces)
 
The lambda form is thus, AFAICT, just the kind of function that
`font-lock(-add)-keywords' expects. Please let me know if I'm missing
something.
 
I think I am correctly using the form (MATCHER HIGHLIGHTER...), from
node `Search-based Fontification' of the Elisp manual.
 
MATCHER here is the lambda form shown above. There is only one
HIGHLIGHTER, with form SUBEXP-HIGHLIGHTER. The SUBEXP-HIGHLIGHTER here
is of form (SUBEXP FACESPEC OVERRIDE LAXMATCH), with SUBEXP=0,
FACESPEC=the symbol (face name) `column-marker-1', OVERRIDE=`prepend',
and LAXMATCH=t.
 
I also tried removing the parens around (0 column-marker-1 prepend t),
making it the cdr instead of the cadr, so it looked like this:
(MATCHER 0 column-marker-1 prepend t), where MATCHER is the same
lambda form. That should match the also-allowed form (MATCHER
. SUBEXP-HIGHLIGHTER). But that didn't change anything.
 
What's odd is that the message complains about a _face reference_, but
the text included with the error message is code from the lambda form
(i.e. from the MATCHER part, not from the HIGHLIGHTER part that
specifies the face).  The display code seems to be walking down the
lambda form, spitting it out bit by bit progressively in the `Invalid
face reference' messages. E.g.:
 
Invalid face reference: end
Invalid face reference: point
Invalid face reference: <=
Invalid face reference: current-column
Invalid face reference: 37
Invalid face reference: =
Invalid face reference: and
Invalid face reference: if
Invalid face reference: 37
Invalid face reference: move-to-column
Invalid face reference: current-column
Invalid face reference: 1
Invalid face reference: forward-line
Invalid face reference: +
Invalid face reference: 0
Invalid face reference: =
Invalid face reference: end
Invalid face reference: point
Invalid face reference: <
Invalid face reference: 37
Invalid face reference: current-column
Invalid face reference: <
Invalid face reference: and
Invalid face reference: while
Invalid face reference: 37
Invalid face reference: move-to-column
Invalid face reference: 37
Invalid face reference: current-column
Invalid face reference: <
Invalid face reference: when
Invalid face reference: 1
Invalid face reference: forward-line
Invalid face reference: 37
Invalid face reference: current-column
Invalid face reference: <
Invalid face reference: unless
Invalid face reference: point-max
Invalid face reference: end
Invalid face reference: setq
Invalid face reference: point-max
Invalid face reference: end
Invalid face reference: >
Invalid face reference: when
Invalid face reference: nil
Invalid face reference: message-log-max
Invalid face reference: point
Invalid face reference: start
Invalid face reference: let
Invalid face reference: end
Invalid face reference: lambda
Invalid face reference: t
Invalid face reference: prepend
Invalid face reference: 0
Invalid face reference: nil
Invalid face reference: start
Invalid face reference: goto-char
Invalid face reference: t
Invalid face reference: point [2 times]
Invalid face reference: 1-
Invalid face reference: list
Invalid face reference: set-match-data
Invalid face reference: progn
Invalid face reference: start
Invalid face reference: point
Invalid face reference: >
Invalid face...
(repeated)
 
It's as if the code were trying to get face specs from the lambda
form, in order to merge those faces. Just a guess. At any rate, it
seems bugged to me, but if not, please let me know what I'm doing
wrong. Thx.
 

In GNU Emacs 23.1.1 (i386-mingw-nt5.1.2600)
 of 2009-07-29 on SOFT-MJASON
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.4)'
 

[-- Attachment #2: column-marker.el --]
[-- Type: application/octet-stream, Size: 10486 bytes --]

;;; column-marker.el --- Highlight certain character columns
;; 
;; Filename: column-marker.el
;; Description: Highlight certain character columns
;; Author: Rick Bielawski <rbielaws@i1.net>
;; Maintainer: Rick Bielawski <rbielaws@i1.net>
;; Created: Tue Nov 22 10:26:03 2005
;; Version: 
;; Last-Updated: Fri Oct 30 13:20:01 2009 (-0700)
;;           By: dradams
;;     Update #: 291
;; Keywords: tools convenience highlight
;; Compatibility: GNU Emacs 21, GNU Emacs 22
;; 
;; Features that might be required by this library:
;;
;;   None
;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 
;;; Commentary: 
;; 
;; Highlights the background at a given character column.
;; 
;; Commands `column-marker-1', `column-marker-2', and
;; `column-marker-3' each highlight a given column (using different
;; background colors, by default).
;;
;; - With no prefix argument, each highlights the current column
;;   (where the cursor is).
;;
;; - With a non-negative numeric prefix argument, each highlights that
;;   column.
;;
;; - With plain `C-u' (no number), each turns off its highlighting.
;;
;; - With `C-u C-u', each turns off all column highlighting.
;;
;; If two commands highlight the same column, the last-issued
;; highlighting command shadows the other - only the last-issued
;; highlighting is seen.  If that "topmost" highlighting is then
;; turned off, the other highlighting for that column then shows
;; through.
;;
;; Examples:
;;
;; M-x column-marker-1 highlights the column where the cursor is, in
;; face `column-marker-1'.
;;
;; C-u 70 M-x column-marker-2 highlights column 70 in face
;; `column-marker-2'.
;;
;; C-u 70 M-x column-marker-3 highlights column 70 in face
;; `column-marker-3'.  The face `column-marker-2' highlighting no
;; longer shows.
;;
;; C-u M-x column-marker-3 turns off highlighting for column-marker-3,
;; so face `column-marker-2' highlighting shows again for column 70.
;;
;; C-u C-u M-x column-marker-1 (or -2 or -3) erases all column
;; highlighting.
;;
;; These commands use `font-lock-fontify-buffer', so syntax
;; highlighting (`font-lock-mode') must be turned on.  There might be
;; a performance impact during refontification.
;;
;;
;; Installation: Place this file on your load path, and put this in
;; your init file (`.emacs'):
;;
;; (require 'column-marker)
;;
;; Other init file suggestions (examples):
;;
;; ;; Highlight column 80 in foo mode.
;; (add-hook foo-mode-hook (lambda () (interactive) (column-marker-1 80)))
;;
;; ;; Use `C-c m' interactively to highlight with face `column-marker-1'.
;; (global-set-key [?\C-c ?m] 'column-marker-1)
;;
;;
;; Please report any bugs!
;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 
;;; Change log:
;;
;; 2008/01/21 dadams
;;     Renamed faces by dropping suffix "-face".
;; 2006/08/18 dadams
;;     column-marker-create: Add newlines to doc-string sentences.
;; 2005/12/31 dadams
;;     column-marker-create: Add marker to column-marker-vars inside the defun,
;;       so it is done in the right buffer, updating column-marker-vars buffer-locally.
;;     column-marker-find: Corrected comment.  Changed or to progn for clarity.
;; 2005/12/29 dadams
;;     Updated wrt new version of column-marker.el (multi-column characters).
;;     Corrected stray occurrences of column-marker-here to column-marker-1.
;;     column-marker-vars: Added make-local-variable.
;;     column-marker-create: Changed positive to non-negative.
;;     column-marker-internal: Turn off marker when col is negative, not < 1.
;; 2005-12-29 RGB
;;     column-marker.el now supports multi-column characters.
;; 2005/11/21 dadams
;;     Combined static and dynamic. 
;;     Use separate faces for each marker.  Different interactive spec.
;; 2005/10/19 RGB
;;     Initial release of column-marker.el.
;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;
;; This program is free software; you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation; either version 2, or (at your option)
;; any later version.

;; This program is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with this program; see the file COPYING.  If not, write to
;; the Free Software Foundation, Inc., 51 Franklin Street, Fifth
;; Floor, Boston, MA 02110-1301, USA.
;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; 
;;; Code:

;;;;;;;;;;;;;;;;;;;;;;


(defface column-marker-1 '((t (:background "gray")))
  "Face used for a column marker.  Usually a background color."
  :group 'faces)

(defvar column-marker-1-face 'column-marker-1
    "Face used for a column marker.  Usually a background color.
Changing this directly affects only new markers.")

(defface column-marker-2 '((t (:background "cyan3")))
  "Face used for a column marker.  Usually a background color."
  :group 'faces)

(defvar column-marker-2-face 'column-marker-2
    "Face used for a column marker.  Usually a background color.
Changing this directly affects only new markers." )

(defface column-marker-3 '((t (:background "orchid3")))
  "Face used for a column marker.  Usually a background color."
  :group 'faces)

(defvar column-marker-3-face 'column-marker-3
    "Face used for a column marker.  Usually a background color.
Changing this directly affects only new markers." )

(defvar column-marker-vars nil
  "List of all internal column-marker variables")
(make-variable-buffer-local 'column-marker-vars) ; Buffer local in all buffers.

(defmacro column-marker-create (var &optional face)
  "Define a column marker named %%colmark%%-VAR.
FACE is the face to use.  If nil, then face `column-marker-1' is used."
  (setq face (or face 'column-marker-1))
  `(progn
     ;; define context variable ,VAR so marker can be removed if desired
     (defvar ,var ()
       "Buffer local. Used internally to store column marker spec.")
     ;; context must be buffer local since font-lock is 
     (make-variable-buffer-local ',var)
     ;; Define wrapper function named ,VAR to call `column-marker-internal'
     (defun ,var (arg)
       ,(concat "Highlight column with face `" (symbol-name face)
                "'.\nWith no prefix argument, highlight current column.\n"
                "With non-negative numeric prefix arg, highlight that column number.\n"
                "With plain `C-u' (no number), turn off this column marker.\n"
                "With `C-u C-u' or negative prefix arg, turn off all column-marker highlighting.")
       (interactive "P")
       (unless (memq ',var column-marker-vars) (push ',var column-marker-vars))
       (cond ((null arg)          ; Default: highlight current column.
              (column-marker-internal ',var (1+ (current-column)) ,face))
             ((consp arg)
              (if (= 4 (car arg))
                  (column-marker-internal ',var nil) ; `C-u': Remove this column highlighting.
                (dolist (var column-marker-vars)
                  (column-marker-internal var nil)))) ; `C-u C-u': Remove all column highlighting.
             ((and (integerp arg) (>= arg 0)) ; `C-u 70': Highlight that column.
              (column-marker-internal ',var (1+ (prefix-numeric-value arg)) ,face))
             (t           ; `C-u -40': Remove all column highlighting.
              (dolist (var column-marker-vars)
                (column-marker-internal var nil)))))))

(defun column-marker-find (col)
  "Returns a function to locate a character in column COL."
  `(lambda (end)
    (let ((start            (point))
          (message-log-max  nil))
      (when (> end (point-max)) (setq end (point-max)))

      ;; Try to keep `move-to-column' from going backward, though it still can.
      (unless (< (current-column) ,col) (forward-line 1))

      ;; Again, don't go backward.  Try to move to correct column.
      (when (< (current-column) ,col) (move-to-column ,col))

      ;; If not at target column, try to move to it.
      (while (and (< (current-column) ,col) (< (point) end)
                  (= 0 (+ (forward-line 1) (current-column)))) ; Should be bol.
        (move-to-column ,col))

      ;; If at target column, not past end, and not prior to start,
      ;; then set match data and return t.  Otherwise go to start
      ;; and return nil.
      (if (and (= ,col (current-column)) (<= (point) end) (> (point) start))
          (progn (set-match-data (list (1- (point)) (point)))
                 t) ; Return t.
        (goto-char start)
        nil))))       ; Return nil.

(defun column-marker-internal (sym col &optional face)
  "SYM is the symbol for holding the column marker context.
COL is the column in which a marker should be set.
Supplying nil or 0 for COL turns off the marker.
FACE is the face to use.  If nil, then face `column-marker-1' is used."
  (setq face (or face 'column-marker-1))
  (when (symbol-value sym)   ; Remove any previously set column marker
    (font-lock-remove-keywords nil (symbol-value sym))
    (set sym nil))
  (when (or (listp col) (< col 0)) (setq col nil)) ; Allow nonsense stuff to turn off the marker
  (when col                             ; Generate a new column marker
    (set sym `((,(column-marker-find col) (0 ,face prepend t))))
    (font-lock-add-keywords nil (symbol-value sym) t))
  (font-lock-fontify-buffer))

;; If you need more markers you can create your own similarly.
;; All markers can be in use at once, and each is buffer-local,
;; so there is no good reason to define more unless you need more
;; markers in a single buffer.
(column-marker-create column-marker-1 column-marker-1-face)
(column-marker-create column-marker-2 column-marker-2-face)
(column-marker-create column-marker-3 column-marker-3-face)

;;;###autoload
(autoload 'column-marker-1 "column-marker" "Highlight a column." t)

;;;;;;;;;;;;;;;;;;

(provide 'column-marker)

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; column-marker.el ends here

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

[Stefan Monnier]

> This sure seems like a bug to me. If not, please tell me what the
> problem is.

The problem is that font-lock-keywords's docstring says:

   [...]
   where MATCHER can be either the regexp to search for, or the function
   name to call to make the search (called with one argument, the limit
   [...]

notice that it says "function name" rather than just "function".  So it
can't just be a lambda expression, it has to be a symbol.


        Stefan

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

[Drew Adams]

> > This sure seems like a bug to me. If not, please tell me what the
> > problem is.
> 
> The problem is that font-lock-keywords's docstring says:
> 
>    where MATCHER can be either the regexp to search for, or 
>    the function name to call to make the search (called with
>    one argument, the limit
> 
> notice that it says "function name" rather than just 
> "function".  So it can't just be a lambda expression, it has
> to be a symbol.


Ah, thank you. I need to put my reading glasses on!

1. However, the Elisp manual, node `Search-based Fontification' speaks of just
`function'. It does not say that it must be a symbol. (Also, the doc string
should be clarified, since "function name" is usually a string, the function
symbol's `symbol-name'.)

  `FUNCTION'
     Find text by calling FUNCTION, and highlight the matches it finds
     using `font-lock-keyword-face'.

     When FUNCTION is called, it receives one argument, the limit of
     the search; it should begin searching at point, and not search
     beyond the limit.  It should return non-`nil' if it succeeds, and
     set the match data to describe the match that was found.
     Returning `nil' indicates failure of the search.

     Fontification will call FUNCTION repeatedly with the same limit,
     and with point where the previous invocation left it, until
     FUNCTION fails.  On failure, FUNCTION need not reset point in any
     particular way.

And further references to it in this node also refer to it only as "a function".
Nowhere does it say that it should be a symbol. So if it must be a symbol, then
this is a doc bug.


2. The code I had seems nevertheless to "work", in the sense that it does what I
expect (highlights the column). Except that it logs those messages and the
performance is terrible. I assume that it is the message logging that degrades
the performance (brings Emacs to its knees).

Is it indeed a bug that binding `message-log-max' to nil does not suppress the
logging here? Or is it simply a doc bug that this limitation of
`message-log-max' is not mentioned?


3. I tried using a symbol, as you and the doc string suggested, but I still get
the same behavior. I used the same code as before, but with `column-marker-find'
redefined as follows, so that it now defines a named function and returns the
new function symbol:

(defun column-marker-find (col)
  (let ((fn-symb  (intern (format "column-marker-move-to-%d" col))))
    (fset
     `,fn-symb
     `(lambda (end)
        (let ((start (point)))
          (when (> end (point-max)) (setq end (point-max)))
          (unless (< (current-column) ,col) (forward-line 1))
          (when (< (current-column) ,col) (move-to-column ,col))
          (while (and (< (current-column) ,col) (< (point) end)
                      (= 0 (+ (forward-line 1) (current-column))))
            (move-to-column ,col))
          (if (and (= ,col (current-column))
                   (<= (point) end) (> (point) start))
              (progn (set-match-data (list (1- (point)) (point)))
                     t)
            (goto-char start)
            nil))))
    fn-symb))

Now I see only these 4 messages repeated over and over, instead of the messages
citing the atoms in the lambda form that I used previously:

Invalid face reference: t
Invalid face reference: prepend
Invalid face reference: 0
Invalid face reference: column-marker-move-to-36

`column-marker-move-to-36' is the symbol created when I did `M-x
column-marker-1'. Its `symbol-function' is this (same as above, with 36 for the
column):

(lambda (end)
  (let ((start (point)))
    (when (> end (point-max))
      (setq end (point-max)))
    (unless (< (current-column) 36)
      (forward-line 1))
    (when (< (current-column) 36)
      (move-to-column 36))
    (while (and (< (current-column) 36)
                (< (point) end)
                (= 0 (+ (forward-line 1)
                        (current-column))))
      (move-to-column 36))
    (if (and (= 36 (current-column))
             (<= (point) end)
             (> (point) start))
        (progn (set-match-data
                (list (1- (point)) (point)))
               t)
      (goto-char start)
      nil)))

The value of variable `column-marker-1' is now this list, which should be OK,
IIUC:

((column-marker-move-to-36
  (0 column-marker-1 prepend t)))

And that is therefore also the relevant portion of `font-lock-keywords'.

What am I missing?


4. Also, the full value of `font-lock-keywords' does in fact have lambda forms
in some of its keyword patterns, but they do not come from this code. They are
present even without it. This, for example:

((lambda (bound)
   (catch 'found
     (while
         (re-search-forward
"\\(\\\\\\\\\\)\\(?:\\(\\\\\\\\\\)\\|\\((\\(?:\\?:\\)?\\|[|)]\\)\\)" bound t)
       (unless (match-beginning 2)
         (let ((face (get-text-property (1- (point)) 'face)))
           (when (or (and (listp face)
                          (memq 'font-lock-string-face face))
                     (eq 'font-lock-string-face face))
             (throw 'found t)))))))
 (1 'font-lock-regexp-grouping-backslash prepend)
 (3 'font-lock-regexp-grouping-construct prepend))

Dunno where that comes from - perhaps from the font-lock machinery itself. In
any case, that lambda form does not seem to be causing any `Invalid face
reference' messages to be logged.


So I still don't understand, and I still haven't found the right way to code
this, so that I don't get the error messages. Please advise.

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

[Chong Yidong]

> 2. The code I had seems nevertheless to "work", in the sense that it
> does what I expect (highlights the column). Except that it logs those
> messages and the performance is terrible.

I suspect this is because parts of font-lock do this:

  (if (stringp matcher)
      (re-search-forward matcher end t)
   (funcall matcher end)))

while other parts do this:

  (font-lock-eval-keywords (if (fboundp keywords)
                               (funcall keywords)
                             (eval keywords)))))

If `matcher' is a lambda expression, the first will work as expected,
because you can pass a lambda to funcall.  But the second will do the
wrong thing.

We should probably fix the code to check for and disallow lambda
expressions (and the docs), assuming not too much external code is
relying on the unintended behavior.

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

[Drew Adams]

> > 2. The code I had seems nevertheless to "work", in the sense that it
> > does what I expect (highlights the column). Except that it 
> > logs those messages and the performance is terrible.
> 
> I suspect this is because parts of font-lock do this:
>   (if (stringp matcher)
>       (re-search-forward matcher end t)
>    (funcall matcher end)))
> 
> while other parts do this:
>   (font-lock-eval-keywords (if (fboundp keywords)
>                                (funcall keywords)
>                              (eval keywords)))))
> 
> If `matcher' is a lambda expression, the first will work as expected,
> because you can pass a lambda to funcall.  But the second will do the
> wrong thing.
> 
> We should probably fix the code to check for and disallow lambda
> expressions (and the docs), assuming not too much external code is
> relying on the unintended behavior.

This part of the thread is only tangentially related to the bug report, but
anyway:

Why is it necessary to restrict the function to a symbol - why disallow lambda
forms? IOW, why can't we use (functionp keywords) instead of (fboundp keywords)?

After all, we're just calling `funcall', and all `funcall' requires of its
function arg is that it be `functionp'. Is there a real reason for the current
restriction to symbols?

Besides, as I noted in a previous mail, the font-lock code itself apparently
uses a lambda form in this way.

Just asking. I don't claim to understand the font-lock code.

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

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

> Why is it necessary to restrict the function to a symbol - why disallow lambda
> forms? IOW, why can't we use (functionp keywords) instead of (fboundp keywords)?

In principle, I don't see why matchers can't be lambda expressions.  The
main concern I have is that other parts of font lock might have subtle
assumptions about the matcher being a function name.  It seems easier to
make this implicit assumption an explicit one.

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

[Drew Adams]

> > Why is it necessary to restrict the function to a symbol - 
> > why disallow lambda forms? IOW, why can't we use
> > (functionp keywords) instead of (fboundp keywords)?
> 
> In principle, I don't see why matchers can't be lambda 
> expressions.  The main concern I have is that other parts
> of font lock might have subtle assumptions about the
> matcher being a function name.  It seems easier to
> make this implicit assumption an explicit one.

Easier is not better.

We should make the right fix, not the easiest one based on our not being sure
what the code does or why.

It would be good if someone knowledgeable in font-lock took a close look and
DTRT. Perhaps Stefan?

If we change `fboundp' to `functionp' here and it turns out that subtle
assumptions are thereby broken, at least we will have discovered those
assumptions, and we can then either comment them clearly or change the code to
use different assumptions (if appropriate).

As it is now, it sounds like we don't really know what the design is or why.

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

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

> We should make the right fix, not the easiest one based on our not
> being sure what the code does or why.

It's pretty clear, from the doc string plus the various parts of
font-lock code that assume function names, that the code was not
intended to work with lambda expressions.  It would be nice if it worked
with lambda expressions, but that would be a new feature, not a bug fix.

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

[Drew Adams]

> > We should make the right fix, not the easiest one based on our not
> > being sure what the code does or why.
> 
> It's pretty clear, from the doc string plus the various parts of
> font-lock code that assume function names, that the code was not
> intended to work with lambda expressions.  It would be nice 
> if it worked with lambda expressions, but that would be a new
> feature, not a bug fix.

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.

Kidding aside, this is a chance to DTRT, not just sidestep the problem because
the code might be unclear or it takes some studying to understand it.

At any rate, as I said, the bug (`Invalid face reference' messages and the
attendant slowdown) seems to be there even when I use a symbol function. Unless
I'm doing something wrong (which is why I asked for help).

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

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

> 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.

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

[Drew Adams]

> > 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.

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

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

> 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.

I'm afraid I don't have time to wade through the sea of words, but I've
stated that it would be nice if a lambda expression can be used.  But I
doubt it's worthwhile to change this for 23.2.  Assuming there's no
third-party code relying on using a lambda expression (which does not
work properly), for now we should simply document the case that works.

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

[Drew Adams]

> > 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.
> 
> I'm afraid I don't have time to wade through the sea of 
> words, but I've stated that it would be nice if a lambda
> expression can be used.  But I doubt it's worthwhile to
> change this for 23.2.

Why? Why wouldn't it be worthwhile to change `fboundp' to `functionp'?

> Assuming there's no third-party code relying on using a
> lambda expression (which does not work properly), for
> now we should simply document the case that works.

Then please consider fixing this to allow all functions for 23.3, if you don't
want to fix it now. Do I need to file another bug report for that?


In any case, _this_ bug is about the `Invalid face reference' messages and
performance degradation. I get those messages even when using a symbol function,
so this bug is not fixed by simply adding a restriction to symbols.

If I'm mistaken and have missed something, please advise. I will be happy if I
can get the function to work without provoking those bizarre error messages and
slowing Emacs to a crawl. I don't have a problem with using a symbol function
instead of a lambda.

IOW, for Emacs generally, I agree with you that "it would be nice if a lambda
expression can be used" at some point. But my concern now is the `Invalid face
reference' messages. I corrected the code to use a symbol, not a lambda, but the
problem remains.

Again, if I missed something, please let me know. I'm no expert on font-lock.
I'm hoping that I simply messed up somewhere, and there is not some bug in the
font-lock code that is causing this behavior. The latter would mean that I won't
be able to make this work in other Emacs versions, even after the bug is fixed.
The former would mean I would learn something; thank you; and be on my way.

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

[Drew Adams]

Any news on this?

> In any case, _this_ bug is about the `Invalid face reference' 
> messages and performance degradation. I get those messages
> even when using a symbol function, so this bug is not fixed
> by simply adding a restriction to symbols.
> 
> If I'm mistaken and have missed something, please advise. I 
> will be happy if I can get the function to work without
> provoking those bizarre error messages and slowing Emacs to
> a crawl. I don't have a problem with using a symbol function
> instead of a lambda.
> 
> IOW, for Emacs generally, I agree with you that "it would be 
> nice if a lambda expression can be used" at some point.
> But my concern now is the `Invalid face reference' messages.
> I corrected the code to use a symbol, not a lambda, but the
> problem remains.
> 
> Again, if I missed something, please let me know. I'm no 
> expert on font-lock. I'm hoping that I simply messed up
> somewhere, and there is not some bug in the font-lock code
> that is causing this behavior. The latter would mean that
> I won't be able to make this work in other Emacs versions,
> even after the bug is fixed. The former would mean I would
> learn something; thank you; and be on my way.

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

[Drew Adams]

[-- Attachment #1: Type: text/plain, Size: 2462 bytes --]

In hopes of getting some new attention to this bug, I've included a simple test
case in the attached file.

Just use emacs -Q, then load the file.
Then `C-x 5 b *Messages*'.
Then `M-x column-marker-1'.
Then click the *Messages* frame to select it.

At the last step, you will see zillions of messages logged. No messages are
logged when the highlighting occurs, however. They appear only when you do
something else (e.g. choose the *Messages* window). And they keep on coming, as
you interact with Emacs. I assume they are coming from the display engine.

[I followed the above recipe with point in column 21. See the comments at the
end of the attached file - you will see that they reference "column 22".]

The relevant part of `font-lock-keywords' is this:
(column-marker-move-to-22 (0 column-marker-1 prepend t))

Function `column-marker-move-to-22' has this as its `symbol-function' when the
highlighting is turned on:

(lambda (end)
  (let ((start (point)))
    (when (> end (point-max))
      (setq end (point-max)))
    (unless (< (current-column) 22) (forward-line 1))
    (when (< (current-column) 22) (move-to-column 22))
    (while (and (< (current-column) 22)
                (< (point) end)
                (= 0 (+ (forward-line 1)(current-column))))
      (move-to-column 22))
    (if (and (= 22 (current-column))
	     (<= (point) end)
	     (> (point) start))
        (progn
          (set-match-data
           (list (1- (point)) (point)))
          t)
      (goto-char start)
      nil)))

[Note: The issue of using a `lambda' form in `font-lock-keywords' was removed
from the bug recipe: I instead now use a symbol that has a function definition.
This unfortunately changes nothing wrt the bug.]

The code works fine: column 22 is highlighted as expected. (You can turn off
highlighting with `C-u M-x column-marker-1'.)

The only problem is the zillions of messages that are logged to *Messages*
endlessly. That degrades performance so much that it brings Emacs to a crawl.
This is the case for both Emacs 22 and 23. It seems that each redisplay causes
this sequence of four messages to be logged many times, over and over:

 Invalid face reference: prepend
 Invalid face reference: 0
 Invalid face reference: column-marker-move-to-22
 Invalid face reference: t

Again, I *hope* I am simply doing something wrong, and you will help me by
letting me know what is wrong and how to fix it. As far as I can tell, however,
this is a serious bug.


[-- Attachment #2: throw-col-marker.el --]
[-- Type: application/octet-stream, Size: 14309 bytes --]

(defface column-marker-1 '((t (:background "gray"))) "" :group 'faces)

(defvar column-marker-vars ())
(make-variable-buffer-local 'column-marker-vars)

(defvar column-marker-1 ())
(make-variable-buffer-local 'column-marker-1)
(defun column-marker-1 (arg)
  (interactive "P")
  (unless (memq 'column-marker-1 column-marker-vars)
    (push 'column-marker-1 column-marker-vars))
  (cond ((null arg)
         (column-marker-internal
          'column-marker-1 (1+ (current-column)) 'column-marker-1))
        ((consp arg)
         (if (= 4 (car arg))
             (column-marker-internal 'column-marker-1 nil)
           (dolist (var column-marker-vars)
             (column-marker-internal var nil))))
        ((and (integerp arg) (>= arg 0))
         (column-marker-internal 'column-marker-1
                                 (1+ (prefix-numeric-value arg))
                                 'column-marker-1))
        (t
         (dolist (var column-marker-vars)
           (column-marker-internal var nil)))))

(defun column-marker-find (col)
  "Defines a function to locate a character in column COL.
Returns the function symbol, named `column-marker-move-to-COL'."
  (let ((fn-symb  (intern (format "column-marker-move-to-%d" col))))
    (fset `,fn-symb
          `(lambda (end)
             (let ((start (point)))
               (when (> end (point-max)) (setq end (point-max)))

               (unless (< (current-column) ,col) (forward-line 1))
               (when (< (current-column) ,col) (move-to-column ,col))
               (while (and (< (current-column) ,col) (< (point) end)
                           (= 0 (+ (forward-line 1) (current-column))))
                 (move-to-column ,col))
               (if (and (= ,col (current-column))
                        (<= (point) end) (> (point) start))
                   (progn (set-match-data (list (1- (point)) (point))) t)
                 (goto-char start)
                 nil))))
    fn-symb))

(defun column-marker-internal (sym col &optional face)
  "SYM is the symbol for holding the column marker context.
COL is the column in which a marker should be set.
Supplying nil or 0 for COL turns off the marker.
FACE is the face to use.  If nil, then face `column-marker-1' is used."
  (setq face (or face 'column-marker-1))
  (when (symbol-value sym)
    (font-lock-remove-keywords nil (symbol-value sym))
    (set sym nil))
  (when (or (listp col) (< col 0)) (setq col nil))
  (when col
    (set sym `((,(column-marker-find col) (0 ,face prepend t))))
    (font-lock-add-keywords nil (symbol-value sym) t))
  (font-lock-fontify-buffer))


;; --------------------------------------------------------
;; These are the messages in *Messages*, with this sequence repeated a
;; zillion times. Note that the messages do *not* appear as soon as I
;; call `M-x column-marker-1' (with point in column 21).  They appear
;; (I think) as soon as I do something that forces redisplay -
;; e.g. select a window showing *Messages*.
;;
;; Invalid face reference: prepend
;; Invalid face reference: 0
;; Invalid face reference: column-marker-move-to-22
;; Invalid face reference: t

;;-----------------------------------------------
;; This is (symbol-function 'column-marker-1):
;; (lambda
;;     (arg)
;;   (interactive "P")
;;   (unless
;;       (memq 'column-marker-1 column-marker-vars)
;;     (push 'column-marker-1 column-marker-vars))
;;   (cond
;;     ((null arg)
;;      (column-marker-internal 'column-marker-1
;;                              (1+
;;                               (current-column))
;;                              'column-marker-1))
;;     ((consp arg)
;;      (if
;;       (= 4
;;          (car arg))
;;       (column-marker-internal 'column-marker-1 nil)
;;        (dolist
;;            (var column-marker-vars)
;;          (column-marker-internal var nil))))
;;     ((and
;;       (integerp arg)
;;       (>= arg 0))
;;      (column-marker-internal 'column-marker-1
;;                              (1+
;;                               (prefix-numeric-value arg))
;;                              'column-marker-1))
;;     (t
;;      (dolist
;;          (var column-marker-vars)
;;        (column-marker-internal var nil)))))

;;-----------------------------------------------
;; This the *value* of `column-marker-1', when highlighted:
;; ((column-marker-move-to-22 (0 column-marker-1 prepend t)))

;;-----------------------------------------------
;; This is (symbol-function 'column-marker-move-to-22) when highlighted:
;; (lambda (end)
;;   (let ((start (point)))
;;     (when (> end (point-max))
;;       (setq end (point-max)))
;;     (unless (< (current-column) 22) (forward-line 1))
;;     (when (< (current-column) 22) (move-to-column 22))
;;     (while (and (< (current-column) 22)
;;                 (< (point) end)
;;                 (= 0 (+ (forward-line 1)(current-column))))
;;       (move-to-column 22))
;;     (if (and (= 22 (current-column))
;; 	     (<= (point) end)
;; 	     (> (point) start))
;;         (progn
;;           (set-match-data
;;            (list (1- (point)) (point)))
;;           t)
;;       (goto-char start)
;;       nil)))

;;-----------------------------------------------
;; This is the part of `font-lock-keywords' that is relevant:
;; (column-marker-move-to-22 (0 column-marker-1 prepend t))

;;-----------------------------------------------
;; This is the full value of `font-lock-keywords':
;; (t
;;  (("(\\(def\\(\\(advice\\|alias\\|generic\\|macro\\*?\\|method\\|setf\\|subst\\*?\\|un\\*?\\|ine-\\(condition\\|\\(?:derived\\|\\(?:global\\(?:ized\\)?-\\)?minor\\|generic\\)-mode\\|method-combination\\|setf-expander\\|skeleton\\|widget\\|function\\|\\(compiler\\|modify\\|symbol\\)-macro\\)\\)\\|\\(const\\(ant\\)?\\|custom\\|varalias\\|face\\|parameter\\|var\\)\\|\\(class\\|group\\|theme\\|package\\|struct\\|type\\)\\)\\)\\>[ 	'(]*\\(setf[ 	]+\\sw+)\\|\\sw+\\)?"
;;    (1 font-lock-keyword-face)
;;    (9
;;     (cond
;;      ((match-beginning 3)
;;       font-lock-function-name-face)
;;      ((match-beginning 6)
;;       font-lock-variable-name-face)
;;      (t font-lock-type-face))
;;     nil t))
;;   ("^;;;###\\(autoload\\)" 1 font-lock-warning-face prepend)
;;   ("\\[\\(\\^\\)" 1 font-lock-negation-char-face prepend)
;;   ("(\\(cond\\(?:ition-case\\)?\\|eval-\\(?:a\\(?:fter-load\\|nd-compile\\|t-startup\\)\\|next-after-load\\|when\\(?:-compile\\)?\\)\\|i\\(?:f\\|nline\\)\\|l\\(?:ambda\\|et\\*?\\)\\|prog[*12nv]?\\|save-\\(?:current-buffer\\|excursion\\|match-data\\|restriction\\|selected-window\\|window-excursion\\)\\|track-mouse\\|unwind-protect\\|w\\(?:hile\\(?:-no-input\\)?\\|ith-\\(?:c\\(?:a\\(?:\\(?:se\\|tegory\\)-table\\)\\|urrent-buffer\\)\\|electric-help\\|local-quit\\|no-warnings\\|output-to-\\(?:string\\|temp-buffer\\)\\|s\\(?:elected-window\\|yntax-table\\)\\|t\\(?:emp-\\(?:buffer\\|\\(?:fil\\|messag\\)e\\)\\|imeout\\(?:-handler\\)?\\)\\)\\)\\)\\>" . 1)
;;   ("(\\(b\\(?:\\(?:loc\\|rea\\)k\\)\\|c\\(?:ase\\|case\\|ompiler-let\\|typecase\\)\\|d\\(?:e\\(?:cla\\(?:im\\|re\\)\\|structuring-bind\\)\\|o\\(?:\\*\\|list\\|times\\)?\\)\\|e\\(?:\\(?:type\\)?case\\)\\|flet\\|go\\|handler-\\(?:bind\\|case\\)\\|i\\(?:gnore-errors\\|n-package\\)\\|l\\(?:abels\\|exical-let\\*?\\|o\\(?:cally\\|op\\)\\)\\|m\\(?:acrolet\\|ultiple-value-\\(?:bind\\|prog1\\)\\)\\|proclaim\\|re\\(?:start-\\(?:bind\\|case\\)\\|turn\\(?:-from\\)?\\)\\|symbol-macrolet\\|t\\(?:agbody\\|\\(?:h\\|ypecas\\)e\\)\\|unless\\|w\\(?:hen\\|ith-\\(?:accessors\\|co\\(?:mpilation-unit\\|ndition-restarts\\)\\|hash-table-iterator\\|input-from-string\\|o\\(?:pen-\\(?:file\\|stream\\)\\|utput-to-string\\)\\|package-iterator\\|s\\(?:imple-restart\\|lots\\|tandard-io-syntax\\)\\)\\)\\)\\>" . 1)
;;   ("(\\(catch\\|throw\\|featurep\\|provide\\|require\\)\\>[ 	']*\\(\\sw+\\)?"
;;    (1 font-lock-keyword-face)
;;    (2 font-lock-constant-face nil t))
;;   ("(\\(abort\\|assert\\|warn\\|check-type\\|cerror\\|error\\|signal\\)\\>" 1 font-lock-warning-face)
;;   ("\\\\\\\\\\[\\(\\sw+\\)\\]" 1 font-lock-constant-face prepend)
;;   ("`\\(\\sw\\sw+\\)'" 1 font-lock-constant-face prepend)
;;   ("\\<:\\sw+\\>" 0 font-lock-builtin-face)
;;   ("\\<\\&\\sw+\\>" . font-lock-type-face)
;;   ((lambda
;;      (bound)
;;      (catch 'found
;;        (while
;; 	   (re-search-forward "\\(\\\\\\\\\\)\\(?:\\(\\\\\\\\\\)\\|\\((\\(?:\\?:\\)?\\|[|)]\\)\\)" bound t)
;; 	 (unless
;; 	     (match-beginning 2)
;; 	   (let
;; 	       ((face
;; 		 (get-text-property
;; 		  (1-
;; 		   (point))
;; 		  'face)))
;; 	     (when
;; 		 (or
;; 		  (and
;; 		   (listp face)
;; 		   (memq 'font-lock-string-face face))
;; 		  (eq 'font-lock-string-face face))
;; 	       (throw 'found t)))))))
;;    (1 'font-lock-regexp-grouping-backslash prepend)
;;    (3 'font-lock-regexp-grouping-construct prepend))
;;   (column-marker-move-to-22
;;    (0 column-marker-1 prepend t)))
;;  ("(\\(def\\(\\(advice\\|alias\\|generic\\|macro\\*?\\|method\\|setf\\|subst\\*?\\|un\\*?\\|ine-\\(condition\\|\\(?:derived\\|\\(?:global\\(?:ized\\)?-\\)?minor\\|generic\\)-mode\\|method-combination\\|setf-expander\\|skeleton\\|widget\\|function\\|\\(compiler\\|modify\\|symbol\\)-macro\\)\\)\\|\\(const\\(ant\\)?\\|custom\\|varalias\\|face\\|parameter\\|var\\)\\|\\(class\\|group\\|theme\\|package\\|struct\\|type\\)\\)\\)\\>[ 	'(]*\\(setf[ 	]+\\sw+)\\|\\sw+\\)?"
;;   (1 font-lock-keyword-face)
;;   (9
;;    (cond
;;     ((match-beginning 3)
;;      font-lock-function-name-face)
;;     ((match-beginning 6)
;;      font-lock-variable-name-face)
;;     (t font-lock-type-face))
;;    nil t))
;;  ("^;;;###\\(autoload\\)"
;;   (1 font-lock-warning-face prepend))
;;  ("\\[\\(\\^\\)"
;;   (1 font-lock-negation-char-face prepend))
;;  ("(\\(cond\\(?:ition-case\\)?\\|eval-\\(?:a\\(?:fter-load\\|nd-compile\\|t-startup\\)\\|next-after-load\\|when\\(?:-compile\\)?\\)\\|i\\(?:f\\|nline\\)\\|l\\(?:ambda\\|et\\*?\\)\\|prog[*12nv]?\\|save-\\(?:current-buffer\\|excursion\\|match-data\\|restriction\\|selected-window\\|window-excursion\\)\\|track-mouse\\|unwind-protect\\|w\\(?:hile\\(?:-no-input\\)?\\|ith-\\(?:c\\(?:a\\(?:\\(?:se\\|tegory\\)-table\\)\\|urrent-buffer\\)\\|electric-help\\|local-quit\\|no-warnings\\|output-to-\\(?:string\\|temp-buffer\\)\\|s\\(?:elected-window\\|yntax-table\\)\\|t\\(?:emp-\\(?:buffer\\|\\(?:fil\\|messag\\)e\\)\\|imeout\\(?:-handler\\)?\\)\\)\\)\\)\\>"
;;   (1 font-lock-keyword-face))
;;  ("(\\(b\\(?:\\(?:loc\\|rea\\)k\\)\\|c\\(?:ase\\|case\\|ompiler-let\\|typecase\\)\\|d\\(?:e\\(?:cla\\(?:im\\|re\\)\\|structuring-bind\\)\\|o\\(?:\\*\\|list\\|times\\)?\\)\\|e\\(?:\\(?:type\\)?case\\)\\|flet\\|go\\|handler-\\(?:bind\\|case\\)\\|i\\(?:gnore-errors\\|n-package\\)\\|l\\(?:abels\\|exical-let\\*?\\|o\\(?:cally\\|op\\)\\)\\|m\\(?:acrolet\\|ultiple-value-\\(?:bind\\|prog1\\)\\)\\|proclaim\\|re\\(?:start-\\(?:bind\\|case\\)\\|turn\\(?:-from\\)?\\)\\|symbol-macrolet\\|t\\(?:agbody\\|\\(?:h\\|ypecas\\)e\\)\\|unless\\|w\\(?:hen\\|ith-\\(?:accessors\\|co\\(?:mpilation-unit\\|ndition-restarts\\)\\|hash-table-iterator\\|input-from-string\\|o\\(?:pen-\\(?:file\\|stream\\)\\|utput-to-string\\)\\|package-iterator\\|s\\(?:imple-restart\\|lots\\|tandard-io-syntax\\)\\)\\)\\)\\>"
;;   (1 font-lock-keyword-face))
;;  ("(\\(catch\\|throw\\|featurep\\|provide\\|require\\)\\>[ 	']*\\(\\sw+\\)?"
;;   (1 font-lock-keyword-face)
;;   (2 font-lock-constant-face nil t))
;;  ("(\\(abort\\|assert\\|warn\\|check-type\\|cerror\\|error\\|signal\\)\\>"
;;   (1 font-lock-warning-face))
;;  ("\\\\\\\\\\[\\(\\sw+\\)\\]"
;;   (1 font-lock-constant-face prepend))
;;  ("`\\(\\sw\\sw+\\)'"
;;   (1 font-lock-constant-face prepend))
;;  ("\\<:\\sw+\\>"
;;   (0 font-lock-builtin-face))
;;  ("\\<\\&\\sw+\\>"
;;   (0 font-lock-type-face))
;;  ((lambda
;;     (bound)
;;     (catch 'found
;;       (while
;; 	  (re-search-forward "\\(\\\\\\\\\\)\\(?:\\(\\\\\\\\\\)\\|\\((\\(?:\\?:\\)?\\|[|)]\\)\\)" bound t)
;; 	(unless
;; 	    (match-beginning 2)
;; 	  (let
;; 	      ((face
;; 		(get-text-property
;; 		 (1-
;; 		  (point))
;; 		 'face)))
;; 	    (when
;; 		(or
;; 		 (and
;; 		  (listp face)
;; 		  (memq 'font-lock-string-face face))
;; 		 (eq 'font-lock-string-face face))
;; 	      (throw 'found t)))))))
;;   (1 'font-lock-regexp-grouping-backslash prepend)
;;   (3 'font-lock-regexp-grouping-construct prepend))
;;  (column-marker-move-to-22
;;   (0 column-marker-1 prepend t))
;;  ("^\\s("
;;   (0
;;    (if
;;        (memq
;; 	(get-text-property
;; 	 (match-beginning 0)
;; 	 'face)
;; 	'(font-lock-string-face font-lock-doc-face font-lock-comment-face))
;;        (list 'face font-lock-warning-face 'help-echo "Looks like a toplevel defun: escape the parenthesis"))
;;    prepend)))

;;---------------------------------------------------------
;; For reference, this is a macro used in the original code.  I simply
;; inlined a call to it with arg `column-marker-1', above, to simplify things.

;; ;; (defmacro column-marker-create (var &optional face)
;;   "Define a column marker named VAR.
;; FACE is the face to use.  If nil, then face `column-marker-1' is used."
;;   (setq face (or face 'column-marker-1))
;;   `(progn
;;      (defvar ,var () "Buffer local. Used internally to store column marker spec.")
;;      (make-variable-buffer-local ',var)
;;      (defun ,var (arg)
;;        (interactive "P")
;;        (unless (memq ',var column-marker-vars) (push ',var column-marker-vars))
;;        (cond ((null arg)
;;               (column-marker-internal ',var (1+ (current-column)) ,face))
;;              ((consp arg)
;;               (if (= 4 (car arg))
;;                   (column-marker-internal ',var nil)
;;                 (dolist (var column-marker-vars)
;;                   (column-marker-internal var nil))))
;;              ((and (integerp arg) (>= arg 0))
;;               (column-marker-internal ',var (1+ (prefix-numeric-value arg)) ,face))
;;              (t
;;               (dolist (var column-marker-vars)
;;                 (column-marker-internal var nil)))))))
;;
;;(column-marker-create column-marker-1)

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

[bojohan+news]

"Drew Adams" <drew.adams@oracle.com> writes:

>     (set sym `((,(column-marker-find col) (0 ,face prepend t))))

(set sym `((,(column-marker-find col) (0 ',face prepend t))))

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

[Drew Adams]

> >     (set sym `((,(column-marker-find col) (0 ,face prepend t))))
> 
> (set sym `((,(column-marker-find col) (0 ',face prepend t))))

Ouch! How embarassing. Thank you so much Johan.


I would close the bug now myself, but other parts are still open:


1. `message-log-max' = nil should turn off message logging for display-engine
messages also.

2. Even if #2 is not feasible, it seems like there should be some way to
combine/collapse such messages, as we sometimes do for normal messages (e.g. "[N
times]".

It would be good to do something, at least, to prevent Emacs slowing to a crawl
with repeated messages that say the same thing.

3. As we agreed, it would be good to allow a lambda form in place of a symbol.
This part is an enhancement request.

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

[Kevin Rodgers]

bojohan+news@dd.chalmers.se wrote:
> "Drew Adams" <drew.adams@oracle.com> writes:
> 
>>     (set sym `((,(column-marker-find col) (0 ,face prepend t))))
> 
> (set sym `((,(column-marker-find col) (0 ',face prepend t))))

Does anyone else find `(... ',form ...) jarring?

I would write it as `(... (quote ,form) ...)

-- 
Kevin Rodgers
Denver, Colorado, USA

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

[Stefan Monnier]

> Does anyone else find `(... ',form ...) jarring?

Probably.

> I would write it as `(... (quote ,form) ...)

I find the ', form easier to recognize, but it's just me,


        Stefan

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

[Lars Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

> The only problem is the zillions of messages that are logged to *Messages*
> endlessly. That degrades performance so much that it brings Emacs to a crawl.
> This is the case for both Emacs 22 and 23. It seems that each redisplay causes
> this sequence of four messages to be logged many times, over and over:
>
>  Invalid face reference: prepend
>  Invalid face reference: 0
>  Invalid face reference: column-marker-move-to-22
>  Invalid face reference: t

I seem to recall some work being done to make redisplay somewhat less
chatty.  Are you still seeing this problem?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Drew Adams]

> I seem to recall some work being done to make redisplay somewhat less
> chatty.  Are you still seeing this problem?

Sorry, I don't recall, and I have no time now to try to look
into it.  Feel free to do with this what you like.