Misleading error messages from bytecomp.el

[Lars Hansen]

Having line number and column in messages from bytecomp.el is a great
help. When they are correct, that is. Otherwise they are very confusing.
If one compiles a file foo.el with the lines

1: (defun foo (l)
2:   (mapc (lambda (elm)
3:           (consp elm))
4:         l)
5:   (message "foo: %s" elm)
6:   (mapc (lambda (elm)
7:           (consp elm))
8:         l))

the error message is

In foo:
foo.el:7:18:Warning: reference to free variable `elm'

Here the compiler points at the `elm' on line 7 but it should have
pointed at the one on line 5.
In a piece of code as small as the one above, it is not hard to spot the
real error. But erroneous messages from compilation of large functions
are confusing.

I have looked into bytecomp.el to see if I could fix the bug. It is,
however, not easy as the following analysis shows:

bytecomp.el uses the function `byte-compile-set-symbol-position' to set
`byte-compile-last-position'. This is done by looking up the symbol in
the alist `read-symbol-positions-list'. When a symbol is looked up, all
occurrences with a position at or before the current value of
`byte-compile-last-position', are removed from the alist. This means
that `byte-compile-last-position' become wrong if
`byte-compile-set-symbol-position' is called twice for the same
occurrence of a symbol, or if it is called for symbol that is generated
by the compiler itself.

When compiling the function `foo' above, the following happens:
To compile function `foo', the compiler inserts a `lambda' to obtain a
lambda form. Then it calls `byte-compile-set-symbol-position' to set the
position of the `lambda' it just inserted. This means that
`byte-compile-last-position' is advanced to point at `lambda' on line 2.
But then, when `byte-compile-set-symbol-position' is called to set the
position of `lambda' on line 2, `byte-compile-last-position' is advanced
to point at `lambda' on line 6. Finally, when
`byte-compile-set-symbol-position' is called to set the position of
`elm' on line 5, `byte-compile-last-position' is advanced to point at
`elm' on line 7.

It is not hard to ensure that `byte-compile-set-symbol-position' is not
called for `lambda' s inserted by the compiler. But that does not change
the erroneous message above, since `byte-compile-set-symbol-position' is
called twice for the same occurrence of `elm', so still
`byte-compile-last-position' is advanced too far.

The patch below makes the compiler report the right line and column in
the example above. But I am convinced it does not fix the basic problem.

I believe line numbers can become wrong unless
`byte-compile-set-symbol-position' is never called for symbols generated
by the compiler itself and never called twice for the same occurrence of
a symbol. But ensuring that is close to impossible due to the recursive
nature of the compiler. Also `lambda' is not the only symbol that is
inserted by the compiler. `macroexpand' may insert anything.

The only good solution I can think of is making the compiler work on
some kind of annotated lisp forms rather than the lisp forms them
selves. One may replace symbols in the input stream with conses (SYMBOL
. POSITION). Symbols added by the compiler should be added as (SYMBOL .
nil). But that would be a major change.

Any ideas?


*** cvsroot/emacs/lisp/emacs-lisp/bytecomp.el   2005-08-08 14:15:23.000000000 
+0200
--- emacs/LH-work/byte-compile/bytecomp.2.178.patched.el        2005-10-16 
14:34:54.000000000 +0200
***************
*** 2304,2310 ****
                                 ',name ',declaration))
                   outbuffer)))))
  
!     (let* ((new-one (byte-compile-lambda (cons 'lambda (nthcdr 2 form))))
           (code (byte-compile-byte-code-maker new-one)))
        (if this-one
          (setcdr this-one new-one)
--- 2304,2310 ----
                                 ',name ',declaration))
                   outbuffer)))))
  
!     (let* ((new-one (byte-compile-lambda (nthcdr 2 form) t))
           (code (byte-compile-byte-code-maker new-one)))
        (if this-one
          (setcdr this-one new-one)
***************
*** 2500,2509 ****
  ;; Byte-compile a lambda-expression and return a valid function.
  ;; The value is usually a compiled function but may be the original
  ;; lambda-expression.
! (defun byte-compile-lambda (fun)
!   (unless (eq 'lambda (car-safe fun))
!     (error "Not a lambda list: %S" fun))
!   (byte-compile-set-symbol-position 'lambda)
    (byte-compile-check-lambda-list (nth 1 fun))
    (let* ((arglist (nth 1 fun))
         (byte-compile-bound-variables
--- 2500,2511 ----
  ;; Byte-compile a lambda-expression and return a valid function.
  ;; The value is usually a compiled function but may be the original
  ;; lambda-expression.
! (defun byte-compile-lambda (fun &optional add-lambda)
!   (if add-lambda
!       (setq fun (cons 'lambda fun))
!     (unless (eq 'lambda (car-safe fun))
!       (error "Not a lambda list: %S" fun))
!     (byte-compile-set-symbol-position 'lambda))
    (byte-compile-check-lambda-list (nth 1 fun))
    (let* ((arglist (nth 1 fun))
         (byte-compile-bound-variables
***************
*** 2755,2763 ****
                    (or (not (byte-compile-version-cond
                              byte-compile-compatibility))
                        (not (get (get fn 'byte-opcode) 'emacs19-opcode))))
!              (progn
!                (byte-compile-set-symbol-position fn)
!                (funcall handler form))
             (when (memq 'callargs byte-compile-warnings)
               (if (memq fn '(custom-declare-group custom-declare-variable 
custom-declare-face))
                   (byte-compile-nogroup-warn form))
--- 2757,2763 ----
                    (or (not (byte-compile-version-cond
                              byte-compile-compatibility))
                        (not (get (get fn 'byte-opcode) 'emacs19-opcode))))
!                (funcall handler form)
             (when (memq 'callargs byte-compile-warnings)
               (if (memq fn '(custom-declare-group custom-declare-variable 
custom-declare-face))
                   (byte-compile-nogroup-warn form))
***************
*** 3671,3677 ****
         (list 'fset
               (list 'quote (nth 1 form))
               (byte-compile-byte-code-maker
!               (byte-compile-lambda (cons 'lambda (cdr (cdr form)))))))
        (byte-compile-discard))
      ;; We prefer to generate a defalias form so it will record the function
      ;; definition just like interpreting a defun.
--- 3671,3677 ----
         (list 'fset
               (list 'quote (nth 1 form))
               (byte-compile-byte-code-maker
!               (byte-compile-lambda (cdr (cdr form)) t))))
        (byte-compile-discard))
      ;; We prefer to generate a defalias form so it will record the function
      ;; definition just like interpreting a defun.
***************
*** 3679,3685 ****
       (list 'defalias
           (list 'quote (nth 1 form))
           (byte-compile-byte-code-maker
!           (byte-compile-lambda (cons 'lambda (cdr (cdr form))))))
       t))
    (byte-compile-constant (nth 1 form)))
  
--- 3679,3685 ----
       (list 'defalias
           (list 'quote (nth 1 form))
           (byte-compile-byte-code-maker
!           (byte-compile-lambda (cdr (cdr form)) t)))
       t))
    (byte-compile-constant (nth 1 form)))
  
***************
*** 3688,3695 ****
    (byte-compile-body-do-effect
     (list (list 'fset (list 'quote (nth 1 form))
               (let ((code (byte-compile-byte-code-maker
!                           (byte-compile-lambda
!                            (cons 'lambda (cdr (cdr form)))))))
                 (if (eq (car-safe code) 'make-byte-code)
                     (list 'cons ''macro code)
                   (list 'quote (cons 'macro (eval code))))))
--- 3688,3694 ----
    (byte-compile-body-do-effect
     (list (list 'fset (list 'quote (nth 1 form))
               (let ((code (byte-compile-byte-code-maker
!                           (byte-compile-lambda (cdr (cdr form)) t))))
                 (if (eq (car-safe code) 'make-byte-code)
                     (list 'cons ''macro code)
                   (list 'quote (cons 'macro (eval code))))))