* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. @ 2016-09-16 11:31 Alan Mackenzie 2016-09-16 13:22 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Alan Mackenzie @ 2016-09-16 11:31 UTC (permalink / raw) To: 24449 Hello, Emacs. Using Emacs 25.1 RC2, and lisp/progmodes/cc-engine.el from commit 33f856ba01d13f649cf5c848b322ecb0dbfc02fc (Fri Sep 16 10:47:55 2016 +0000), $ emacs -Q -batch -f batch-byte-compile cc-engine.el . This outputs the following warning: In c-forward-decl-or-cast-1: cc-engine.el:8105:22:Warning: reference to free variable `eq' . The use of `eq' on L8105 is entirely correct. The error is at L8636, where the following appears: (and eq context nil (match-beginning 1)) . Clearly parentheses around the `eq' form are missing. The compiler should have output its warning for L8636, not L8105. -- Alan Mackenzie (Nuremberg, Germany). ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-16 11:31 bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place Alan Mackenzie @ 2016-09-16 13:22 ` Eli Zaretskii 2016-09-16 13:33 ` Alan Mackenzie 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2016-09-16 13:22 UTC (permalink / raw) To: Alan Mackenzie; +Cc: 24449 > Date: Fri, 16 Sep 2016 11:31:25 +0000 > From: Alan Mackenzie <acm@muc.de> > > $ emacs -Q -batch -f batch-byte-compile cc-engine.el > > . This outputs the following warning: > > In c-forward-decl-or-cast-1: > cc-engine.el:8105:22:Warning: reference to free variable `eq' > > . The use of `eq' on L8105 is entirely correct. The error is at L8636, > where the following appears: > > (and eq context nil > (match-beginning 1)) > > . Clearly parentheses around the `eq' form are missing. > > The compiler should have output its warning for L8636, not L8105. Did you look at how the byte compiler determines the line number it will include in the warning/error message? 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-16 13:22 ` Eli Zaretskii @ 2016-09-16 13:33 ` Alan Mackenzie 2016-09-16 15:09 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Alan Mackenzie @ 2016-09-16 13:33 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 24449 Hello, Eli. On Fri, Sep 16, 2016 at 04:22:08PM +0300, Eli Zaretskii wrote: > > Date: Fri, 16 Sep 2016 11:31:25 +0000 > > From: Alan Mackenzie <acm@muc.de> > > > > $ emacs -Q -batch -f batch-byte-compile cc-engine.el > > > > . This outputs the following warning: > > > > In c-forward-decl-or-cast-1: > > cc-engine.el:8105:22:Warning: reference to free variable `eq' > > > > . The use of `eq' on L8105 is entirely correct. The error is at L8636, > > where the following appears: > > > > (and eq context nil > > (match-beginning 1)) > > > > . Clearly parentheses around the `eq' form are missing. > > > > The compiler should have output its warning for L8636, not L8105. > Did you look at how the byte compiler determines the line number it > will include in the warning/error message? I'm looking at it now, with a view to making it work. > 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? -- Alan Mackenzie (Nuremberg, Germany). ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-16 13:33 ` Alan Mackenzie @ 2016-09-16 15:09 ` Eli Zaretskii 2016-09-16 16:08 ` Alan Mackenzie 2016-09-16 18:34 ` Alan Mackenzie 0 siblings, 2 replies; 15+ messages in thread From: Eli Zaretskii @ 2016-09-16 15:09 UTC (permalink / raw) To: Alan Mackenzie; +Cc: 24449 > 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-16 15:09 ` Eli Zaretskii @ 2016-09-16 16:08 ` Alan Mackenzie 2016-09-16 18:34 ` Alan Mackenzie 1 sibling, 0 replies; 15+ messages in thread From: Alan Mackenzie @ 2016-09-16 16:08 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 24449 Hello, Eli. 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. Yes. I understand it better now, though the comments before `byte-compile-set-symbol-position' are not as helpful as they might be. Given that the byte compiler works by first reading an entire top-level form, and only then going to work on it, the only handle the compiler has on the original source is this list `read-symbol-positions-list' produced by the reader. (It was probably invented for the byte compiler). So without rewriting one or both of the byte compiler and the reader, there doesn't seem to be a different strategy available for determining the position in the raw source. Correction: there might be: On processing a symbol, at the moment the earliest occurrence of that symbol in `read-symbol-positions-list' is removed from it. Instead, we could remove everything up to and including that symbol. Maybe. What I'm guessing happened in my particular case is that several instances of 'eq in that list failed to get removed because it's a function that undergoes compiler optimisation. Or something like that. So, the thing for me to check first is that `byte-compile-set-symbol-position' gets called for _everything_ it should be. -- Alan Mackenzie (Nuremberg, Germany). ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-16 15:09 ` Eli Zaretskii 2016-09-16 16:08 ` Alan Mackenzie @ 2016-09-16 18:34 ` Alan Mackenzie 2016-09-16 19:03 ` Eli Zaretskii 1 sibling, 1 reply; 15+ messages in thread From: Alan Mackenzie @ 2016-09-16 18:34 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 24449 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). ^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-16 18:34 ` Alan Mackenzie @ 2016-09-16 19:03 ` Eli Zaretskii 2016-09-17 8:29 ` Alan Mackenzie 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2016-09-16 19:03 UTC (permalink / raw) To: Alan Mackenzie; +Cc: 24449 > Date: Fri, 16 Sep 2016 18:34:51 +0000 > Cc: 24449@debbugs.gnu.org > From: Alan Mackenzie <acm@muc.de> > > Comments? Please perform a full bootstrap and compare the warning messages before and after the change. Then tell what you found. Thanks. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-16 19:03 ` Eli Zaretskii @ 2016-09-17 8:29 ` Alan Mackenzie 2016-09-17 9:35 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Alan Mackenzie @ 2016-09-17 8:29 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 24449 Hello, Eli. On Fri, Sep 16, 2016 at 10:03:54PM +0300, Eli Zaretskii wrote: > > Date: Fri, 16 Sep 2016 18:34:51 +0000 > > Cc: 24449@debbugs.gnu.org > > From: Alan Mackenzie <acm@muc.de> > > > > Comments? > Please perform a full bootstrap and compare the warning messages > before and after the change. Then tell what you found. This was actually very interesting. I've never really looked at warning messages all that closely before. Here's what I saw: 1. The column numbers of all positions are irritatingly 1-based, so are 1 off from the column number displayed in Emacs's mode line. 2. Most of the warnings are "foo is an obsolete function", for which the position given is only vaguely in the same area as the offending function, both with and without the change. It seems that for this warning, `byte-compile-set-symbol-position' is not being called at all. As an example of where the positions in the warnings differ: (Before change): emulation/viper.el:986:44:Warning: `interactive-p' is an obsolete function (as of 23.2); use `called-interactively-p' instead. (After change): emulation/viper.el:1023:69:Warning: `interactive-p' is an obsolete function (as of 23.2); use `called-interactively-p' instead. The actual occurrence of `interactive-p' is at 937:15. Both of the positions given are at occurrences of the symbol `vi-state'. It is perhaps a little worrying that the output positions are both beyond the actual position, suggesting that some call of `byte-compile-set-symbol-position' has already moved the position too far before this warning gets generated. 3. Most of the other warnings actually output were similarly vague in their positions. This includes warnings generated by packages other than the byte compiler, e.g. cedet. 4. The three "free variable" warnings' inaccurate positions (before change) were given accurate positions (after change). As well as the occurrence in cc-engine.el, we have: (Before change): org/org.el:12842:19:Warning: assignment to free variable `e' org/org.el:12842:19:Warning: reference to free variable `e (After change): org/org.el:12840:20:Warning: assignment to free variable `e' org/org.el:12840:20:Warning: reference to free variable `e' 5. The change I made yesterday appears not to have made anything any worse. ========================================================================= What I would suggest should get done: we should make the column numbers 0-based; suitable places to call `byte-compile-set-symbol-position' for the error messages we see should be identified, the calls inserted, and another bootstrap build done to see how much this helps. > Thanks. -- Alan Mackenzie (Nuremberg, Germany). ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-17 8:29 ` Alan Mackenzie @ 2016-09-17 9:35 ` Eli Zaretskii 2016-09-17 10:06 ` Andreas Schwab 2016-09-17 12:58 ` Alan Mackenzie 0 siblings, 2 replies; 15+ messages in thread From: Eli Zaretskii @ 2016-09-17 9:35 UTC (permalink / raw) To: Alan Mackenzie; +Cc: 24449 > Date: Sat, 17 Sep 2016 08:29:52 +0000 > Cc: 24449@debbugs.gnu.org > From: Alan Mackenzie <acm@muc.de> > > 5. The change I made yesterday appears not to have made anything any > worse. Thanks, I guess that means you should push it. > What I would suggest should get done: we should make the column numbers > 0-based This should be a separate change, and before doing it, we should make sure to fix code that assumes the columns to be 1-based (e.g, what does "C-x `" do?). > suitable places to call `byte-compile-set-symbol-position' for > the error messages we see should be identified, the calls inserted, and > another bootstrap build done to see how much this helps. Sounds a good idea, thanks. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-17 9:35 ` Eli Zaretskii @ 2016-09-17 10:06 ` Andreas Schwab 2016-09-17 12:58 ` Alan Mackenzie 1 sibling, 0 replies; 15+ messages in thread From: Andreas Schwab @ 2016-09-17 10:06 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 24449, Alan Mackenzie On Sep 17 2016, Eli Zaretskii <eliz@gnu.org> wrote: > This should be a separate change, and before doing it, we should make > sure to fix code that assumes the columns to be 1-based (e.g, what > does "C-x `" do?). It uses compilation-first-column. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-17 9:35 ` Eli Zaretskii 2016-09-17 10:06 ` Andreas Schwab @ 2016-09-17 12:58 ` Alan Mackenzie 2016-09-17 19:56 ` Michael Heerdegen 1 sibling, 1 reply; 15+ messages in thread From: Alan Mackenzie @ 2016-09-17 12:58 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 24449-done Hello, Eli. On Sat, Sep 17, 2016 at 12:35:05PM +0300, Eli Zaretskii wrote: > > Date: Sat, 17 Sep 2016 08:29:52 +0000 > > Cc: 24449@debbugs.gnu.org > > From: Alan Mackenzie <acm@muc.de> > > 5. The change I made yesterday appears not to have made anything any > > worse. > Thanks, I guess that means you should push it. Done. I took the liberty of amending the comment before `byte-compile-set-symbol-position'. > > What I would suggest should get done: we should make the column numbers > > 0-based > This should be a separate change, and before doing it, we should make > sure to fix code that assumes the columns to be 1-based (e.g, what > does "C-x `" do?). Ah. OK. > > suitable places to call `byte-compile-set-symbol-position' for > > the error messages we see should be identified, the calls inserted, and > > another bootstrap build done to see how much this helps. > Sounds a good idea, thanks. Done this too, in one place, which causes the "obsolete function" messages to get the correct position. Messages generated by `macroexp--warn-and-return' continue to have wrong positions. I'm not sure it's possible to fix this, and my intellect isn't up to working out how it works, at least not today. The messages about unknown functions, or not known to be defined at runtime functions continue to give EOF as their position. This was the use case for the parameter `allow-previous' in `byte-compile-set-symbol-position', but it didn't work before, and it continues not to work just as well now. It would be possible to fix this by not deleting elements from `read-symbol-positions-list', but this would slow down compilation (even if only a little), and generally seems not to be worth the trouble. -- Alan Mackenzie (Nuremberg, Germany). ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-17 12:58 ` Alan Mackenzie @ 2016-09-17 19:56 ` Michael Heerdegen 2016-09-17 20:46 ` Alan Mackenzie 0 siblings, 1 reply; 15+ messages in thread From: Michael Heerdegen @ 2016-09-17 19:56 UTC (permalink / raw) To: 24449; +Cc: acm Hello Alan, do your commits maybe fix any of the bugs related to #22288 25.0.50; Incorrect line and column number in byte-compilation warning (22288,2681,8774,9109,24128)? Thanks, Michael. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-17 19:56 ` Michael Heerdegen @ 2016-09-17 20:46 ` Alan Mackenzie 2016-09-17 20:51 ` Michael Heerdegen 0 siblings, 1 reply; 15+ messages in thread From: Alan Mackenzie @ 2016-09-17 20:46 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 24449 Hello, Michael. On Sat, Sep 17, 2016 at 09:56:46PM +0200, Michael Heerdegen wrote: > Hello Alan, > do your commits maybe fix any of the bugs related to #22288 > 25.0.50; Incorrect line and column number in byte-compilation warning > (22288,2681,8774,9109,24128)? Outch! I was so angry with myself for making that stupid commit to cc-engine.el (which I wouldn't have done without the bug), that I raised bug #24449 before checking for existing bugs. To answer your question: #24449 should fix #22288 (though I haven't verified this). I doubt it will fix #24128. (Though I might be able to knock this one down with a bit of hacking.) I think it will fix #9109. I think it will fix #8774. I think it will fix #2681 At this point, we should decide which one of us is going to check these earlier bugs. It would be a waste of effort if we both did. I will definitely be having a look at #24128. > Thanks, > Michael. -- Alan Mackenzie (Nuremberg, Germany). ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-17 20:46 ` Alan Mackenzie @ 2016-09-17 20:51 ` Michael Heerdegen 2016-09-17 20:59 ` Alan Mackenzie 0 siblings, 1 reply; 15+ messages in thread From: Michael Heerdegen @ 2016-09-17 20:51 UTC (permalink / raw) To: Alan Mackenzie; +Cc: 24449 Alan Mackenzie <acm@muc.de> writes: > At this point, we should decide which one of us is going to check > these earlier bugs. It would be a waste of effort if we both did. Since I'm very busy currently, I would be glad if you could care about this stuff. Some more days don't count given how long the general issue existed, so no hurry. Thanks, Michael. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place. 2016-09-17 20:51 ` Michael Heerdegen @ 2016-09-17 20:59 ` Alan Mackenzie 0 siblings, 0 replies; 15+ messages in thread From: Alan Mackenzie @ 2016-09-17 20:59 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 24449 Hello, Michael. On Sat, Sep 17, 2016 at 10:51:55PM +0200, Michael Heerdegen wrote: > Alan Mackenzie <acm@muc.de> writes: > > At this point, we should decide which one of us is going to check > > these earlier bugs. It would be a waste of effort if we both did. > Since I'm very busy currently, I would be glad if you could care about > this stuff. Some more days don't count given how long the general issue > existed, so no hurry. OK, I'll look at these either today (:-) or tomorrow. > Thanks, > Michael. -- Alan Mackenzie (Nuremberg, Germany). ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-09-17 20:59 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-09-16 11:31 bug#24449: Emacs 25.1 RC2: Byte compiler reports error in wrong place Alan Mackenzie 2016-09-16 13:22 ` Eli Zaretskii 2016-09-16 13:33 ` Alan Mackenzie 2016-09-16 15:09 ` Eli Zaretskii 2016-09-16 16:08 ` Alan Mackenzie 2016-09-16 18:34 ` Alan Mackenzie 2016-09-16 19:03 ` Eli Zaretskii 2016-09-17 8:29 ` Alan Mackenzie 2016-09-17 9:35 ` Eli Zaretskii 2016-09-17 10:06 ` Andreas Schwab 2016-09-17 12:58 ` Alan Mackenzie 2016-09-17 19:56 ` Michael Heerdegen 2016-09-17 20:46 ` Alan Mackenzie 2016-09-17 20:51 ` Michael Heerdegen 2016-09-17 20:59 ` Alan Mackenzie
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).