GNU bug report logs -
#73725
Master: Wrong position for byte compiler warning message.
Previous Next
Full log
Message #38 received at 73725 <at> debbugs.gnu.org (full text, mbox):
>> Is this needed? I'd expect we need `macroexp-preserve-posification`
>> only at those places where an optimization returns a form whose `car` is
>> different from that of FORM. So I expect this to happen in more
>> specific places inside byte-opt.el than here.
> Yes, it is needed. byte-optimize-form-code-walker sometimes does return
> a form with a different car. For example, a progn form with a single
> sub-form comes back without the progn.
This should usually be harmless since the subform should come with its own
position information.
> Even if there are several sub-forms, the code uses macroexp-progn to
> substitute a different progn symbol without the position. I don't
> know why it does this.
This is more annoying, indeed. Ideally, we'd move the
`macroexp-preserve-posification` to the place(s) where this remove+readd
`progn` happens.
> One of my ideas was to fix byte-optimize-form-code-walker by fixing each
> individual bit where the SWP got lost. In the end I gave that up as too
> much work, and too difficult to test.
Hmm... I see. We can leave for a later optimization of
`byte-optimize-form-code-walker`.
>> IOW, this deserves a clear comment explaining why we need it here,
>> probably with some kind of example.
> OK, I'll put one in, along the lines of the above, but less wordy.
Thanks.
> Good point. The doc string of that function is a little clumsy, as it
> refers to the doc string of the next function macroexp--posify-form,
That part did not bother me. These are internal functions so it's OK if
the docstring is a bit less "self contained".
> But I'll write somewhere that we modify "a single symbol", or something
> like that.
Thanks.
>> > (defun macroexp-macroexpand (form env)
>> > "Like `macroexpand' but checking obsolescence."
>> > (let* ((macroexpand-all-environment env)
>> > new-form)
>> > + (macroexp-preserve-posification
>> > + form
>> > (while (not (eq form (setq new-form (macroexpand-1 form env))))
>> > + (setq macroexpanded t)
>> > (let ((fun (car-safe form)))
>> > (setq form
>> > (if (and fun (symbolp fun)
>
>> This `(setq macroexpanded t)` looks like some leftover code, at least
>> I couldn't find this var declared or used elsewhere.
>
> I remember removing it, but can't remember exactly why. When I byte
> compile the code, I don't get an undeclared variable warning for it, for
> some reason.
[ Interesting. I'll try to remember to track down this sucker later. ]
>> > -(defun macroexpand-all (form &optional environment)
>> > +(defun macroexpand-all (form &optional environment keep-pos)
>> Any reason why we need this new argument?
>> Can't we just always try to preserve the positions?
> There are lots of calls to macroexpand-all (around 37), and I was
> concerned about possible accidental side effects in these.
I think we should assume that those potential side effects would more
likely be beneficial than harmful.
Another way to look at it is to look at the doc you provided:
KEEP-POS, if non-nil, specifies that any symbol-with-position for
FORM should be preserved, later to be usable by
`byte-compile--warning-source-offset'.
Even when `keep-pos` is nil, `macroexpand-all` will preserve most of the
SWPs, so the doc is misleading.
> Also, always trying to preserve the position might slow down
> compilation, but I haven't measured that yet.
I think the calls where you currently ask for `keep-pos` account for the
vast majority of the time spent macro-expanding, so I'd be surprised if
doing it in all cases would make it measurably worse.
Stefan
This bug report was last modified 107 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.