bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place.

[Alan Mackenzie]

On Fri, Sep 16, 2016 at 06:09:38PM +0300, Eli Zaretskii wrote:
> > Date: Fri, 16 Sep 2016 13:33:52 +0000
> > Cc: 24449@debbugs.gnu.org
> > From: Alan Mackenzie <acm@muc.de>
> > 
> > > If you didn't, you should, because after you do, you will never again
> > > wonder why an incorrect line number is reported.  In fact, now that I
> > > did look there, I'm surprised it reports a correct line number at all,
> > > let alone as often as it does.  It's sheer luck.
> > 
> > This is not a Good Thing.  Even its own comment describes itself as a
> > "gross hack".  Surely we can do better?

> I certainly hope we can.  But, unless I misunderstood something, the
> way it's designed makes that really hard.

After studying `byte-compile-set-symbol-position' for several hours, I
can now see what it's meant to do, and the bug that prevents it doing
it.

The variable `last' was intended to record the previous value of
byte-compile-last-position to ensure that its next value would be higher
than the previous one.  Part of the comment was intended to express
this.  But in some sort of coding error, `last' ended up being set at
each iteration of the loop, making it purposeless.

By binding `last' to its intended value at the start of
`b-c-set-symbol-position', and amending the loop condition to avoid an
infinite loop, the warning message for the faulty cc-engine.el now comes
out at the right place.  I'm not sure my new version provides any
guarantee of correctness either, but I think it's more likely.

Here is my patch.  I still think I should amend the comment preceding
it.



diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index b6bb1d6..2502323 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -1042,19 +1042,20 @@ byte-compile-delete-first
 ;; gross hack?  And the answer, of course, would be yes.
 (defun byte-compile-set-symbol-position (sym &optional allow-previous)
   (when byte-compile-read-position
-    (let (last entry)
+    (let ((last byte-compile-last-position)
+          entry)
       (while (progn
-              (setq last byte-compile-last-position
-                    entry (assq sym read-symbol-positions-list))
+              (setq entry (assq sym read-symbol-positions-list))
               (when entry
                 (setq byte-compile-last-position
                       (+ byte-compile-read-position (cdr entry))
                       read-symbol-positions-list
                       (byte-compile-delete-first
                        entry read-symbol-positions-list)))
-              (or (and allow-previous
-                        (not (= last byte-compile-last-position)))
-                  (> last byte-compile-last-position)))))))
+              (and entry
+                    (or (and allow-previous
+                             (not (= last byte-compile-last-position)))
+                        (> last byte-compile-last-position))))))))
 
 (defvar byte-compile-last-warned-form nil)
 (defvar byte-compile-last-logged-file nil)


Comments?

-- 
Alan Mackenzie (Nuremberg, Germany).