Re: Correct byte compiler error/warning positions. The solution!

[Alan Mackenzie <acm@muc.de>]

Hello, Eli.

On Sat, Nov 27, 2021 at 12:51:13 +0200, Eli Zaretskii wrote:
> > Date: Sat, 27 Nov 2021 10:33:07 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > > I meant that NILP will be implemented as a binary comparison against
> > > > zero, omitting the awkward test for symbols_with_position_enabled.

> > > Please don't write code that assumes Qnil is zero.  (Or maybe I still
> > > misunderstand what exactly you mean by the above.)

> > Don't worry!  The new code merely assumes Qnil is a symbol, and leaves it
> > to the compiler to optimise for Qnil being binary zero.

> Once again, you cannot compare structures with == in portable C.  And
> the Lisp_Object type can be struct.  As long as what you say above
> doesn't mean that you compare with ==, I'm okay with what you propose.

Sorry, I was perhaps a wee bit too informal when I said "==".  The
actual code in lisp.h looks like this:

    #define lisp_h_NILP(x) ((XLI (x) == XLI (Qnil)))

..

Anyhow, it's now working!  I can bootstrap the new code, although I
haven't tried it with native compilation, yet.  For timings, I've used
my favourite informal benchmark, M-; (time-scroll) on xdisp.c:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(defmacro time-it (&rest forms)
  "Time the running of a sequence of forms using `float-time'.
Call like this: \"M-: (time-it (foo ...) (bar ...) ...)\"."
  `(let ((start (float-time)))
    ,@forms
    (- (float-time) start)))

(defun time-scroll (&optional arg)
  (interactive "P")
  (message "%s"
           (time-it
            (condition-case nil
                (while t
                  (if arg (scroll-down) (scroll-up))
                  (sit-for 0))
              (error nil)))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

The timings I have for scrolling in the forward direction (each time
preceded by M-x garbage-collect, and typing/erasing a character to clear
the font-lock settings) are:

(master branch, no native compilation):
20.847s, 21.609s, 19.459s, 21.570s, 21.651

(New code with correct warning messages, no native compilation):
21.284s, 42.152s ????, 19.927s, 19.960s, 22.259s, 22.198, 22.209s.

The input to configure was the same in both cases, namely

  ./configure --with-gif=no --with-tiff=no --with-gpm.

, and the timings were done on the Linux console.

No, I don't understand that 42s run.  A lot of garbage collection,
perhaps?  But otherwise, the timings are broadly similar, with the new
code being slightly slower.  But the variations between runs is more
significant than the variation between versions.

On bug ~9109 from Roland Winkler:

(unwind-protect
    (let ((foo "foo"))
      (insert foo))
  (setq foo "bar"))

Byte compiling this with M-x compile-defun on master produces this
warning message:

  Buffer winkler.el:2:12: Warning: assignment to free variable `foo'

The position 2:12 is clearly wrong.

Byte compiling it with the new code produces:

  Buffer winkler.el:2:12:4:9: Warning: assignment to free variable
`#<symbol foo at 67>'.

There, there are two positions the (old, temporary) 2:12 and the (new,
correct) 4:9.  There is a bit of tidying up of the mechanism to be done
here, obviously, but the correct position is demonstrated.

-- 
Alan Mackenzie (Nuremberg, Germany).