Re: Bug#490722: semantic: invalid read syntax

Re: Bug#490722: semantic: invalid read syntax

[Michael Olson]

[-- Attachment #1: Type: text/plain, Size: 930 bytes --]

Joerg Jaspert <joerg@debian.org> writes:

> semantic-c.el:42:13:Warning:
>     /usr/share/emacs-snapshot/site-lisp/semantic/bovine/semantic-c.el:
>     `semantic-tag-static-p' obsoletes overload `tag-static'
> semantic-c.el:42:21:Error: Invalid read syntax: ")"

This is a bug of emacs-snapshot, not semantic.  The above compilation
error is triggered by the following kind of statement:

(eval-when-compile
   (require 'something)
   (require 'something-else))

The problem goes away when the lines are changed to:

(eval-when-compile (require 'something))
(eval-when-compile (require 'something-else))

This is a regression and needs to be fixed in upstream Emacs.

-- 
|       Michael Olson  |  FSF Associate Member #652     |
| http://mwolson.org/  |  Hobbies: Lisp, HCoop          |
| Projects: Emacs, Muse, ERC, EMMS, ErBot, DVC, Planner |
`-------------------------------------------------------'

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Please fix before the release of 23.1 (was: Bug#490722: semantic: invalid read syntax)

[Michael Olson]

[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]

This is a pretty serious regression, so I want to bring it to the
attention of the list again, in hopes it gets fixed before the release
of 23.1.  I think the problem is in the byte-compiler.

Michael Olson <mwolson@gnu.org> writes:

> Joerg Jaspert <joerg@debian.org> writes:
>
>> semantic-c.el:42:13:Warning:
>>     /usr/share/emacs-snapshot/site-lisp/semantic/bovine/semantic-c.el:
>>     `semantic-tag-static-p' obsoletes overload `tag-static'
>> semantic-c.el:42:21:Error: Invalid read syntax: ")"
>
> This is a bug of emacs-snapshot, not semantic.  The above compilation
> error is triggered by the following kind of statement:
>
> (eval-when-compile
>    (require 'something)
>    (require 'something-else))
>
> The problem goes away when the lines are changed to:
>
> (eval-when-compile (require 'something))
> (eval-when-compile (require 'something-else))
>
> This is a regression and needs to be fixed in upstream Emacs.

-- 
|       Michael Olson  |  FSF Associate Member #652     |
| http://mwolson.org/  |  Hobbies: Lisp, HCoop          |
| Projects: Emacs, Muse, ERC, EMMS, ErBot, DVC, Planner |
`-------------------------------------------------------'

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: Please fix before the release of 23.1

[Glenn Morris]

Michael Olson wrote:

> This is a pretty serious regression, so I want to bring it to the
> attention of the list again, in hopes it gets fixed before the release
> of 23.1.  I think the problem is in the byte-compiler.

If this were true, we wouldn't be able to build Emacs, since there are
numerous instances of such constructs in the source. Since we can, the
problem either does not exist, or cannot be as general as you suggest.
So please try to investigate further.

>> This is a bug of emacs-snapshot, not semantic.  The above compilation
>> error is triggered by the following kind of statement:
>>
>> (eval-when-compile
>>    (require 'something)
>>    (require 'something-else))
>>
>> The problem goes away when the lines are changed to:
>>
>> (eval-when-compile (require 'something))
>> (eval-when-compile (require 'something-else))
>>
>> This is a regression and needs to be fixed in upstream Emacs.

Re: Please fix before the release of 23.1

[Ulrich Mueller]

>>>>> On Sat, 09 Aug 2008, Glenn Morris wrote:

>>> This is a bug of emacs-snapshot, not semantic.  The above
>>> compilation error is triggered by the following kind of statement:
>>> 
>>> (eval-when-compile
>>> (require 'something)
>>> (require 'something-else))
>>> 
>>> The problem goes away when the lines are changed to:
>>> 
>>> (eval-when-compile (require 'something))
>>> (eval-when-compile (require 'something-else))
>>> 
>>> This is a regression and needs to be fixed in upstream Emacs.

>> This is a pretty serious regression, so I want to bring it to the
>> attention of the list again, in hopes it gets fixed before the
>> release of 23.1.  I think the problem is in the byte-compiler.

> If this were true, we wouldn't be able to build Emacs, since there
> are numerous instances of such constructs in the source. Since we
> can, the problem either does not exist, or cannot be as general as
> you suggest.

The problem definitely exists and is not a problem of CEDET/Semantic.

> So please try to investigate further.

Here is a minimal example to reproduce the failure:

,----[ test.el ]
| (progn
|   (require 'cc-mode))
`----

$ emacs -Q test.el
M-x eval-buffer

This will result in an error: Invalid read syntax: ")"


The error disappears if the expression is in one line:

,----[ test.el ]
| (progn (require 'cc-mode))
`----

Ulrich

Re: Please fix before the release of 23.1

[Ulrich Mueller]

>>>>> On Sun, 10 Aug 2008, I wrote:

> ,----[ test.el ]
> | (progn
> |   (require 'cc-mode))
> `----

> $ emacs -Q test.el
> M-x eval-buffer

> This will result in an error: Invalid read syntax: ")"

The problem goes away if I revert the following change:

2008-06-27  Alan Mackenzie  <acm@muc.de>

	* progmodes/cc-defs.el (c-emacs-features): New feature
	'argumentative-bod-function.

--- progmodes/cc-defs.el	6 May 2008 07:18:12 -0000	1.57
+++ progmodes/cc-defs.el	27 Jun 2008 21:12:46 -0000	1.58
@@ -1440,6 +1440,14 @@
 			 '1-bit)
 		       list)))
 
+    ;; In Emacs >= 23, beginning-of-defun will passes its parameter to
+    ;; beginning-of-defun-function.  Assume end-of-defun does the same.
+    (let ((beginning-of-defun-function
+	   (lambda (&optional arg)
+	     (not (eq arg nil)))))
+      (if (beginning-of-defun 1)
+	  (setq list (cons 'argumentative-bod-function list))))
+
     (let ((buf (generate-new-buffer " test"))
 	  parse-sexp-lookup-properties
 	  parse-sexp-ignore-comments
@@ -1539,6 +1547,9 @@
 
 '8-bit              8 bit syntax entry flags (XEmacs style).
 '1-bit              1 bit syntax entry flags (Emacs style).
+'argumentative-bod-function         beginning-of-defun passes ARG through
+                    to a non-null beginning-of-defun-function.  It is assumed
+		    the end-of-defun does the same thing.
 'syntax-properties  It works to override the syntax for specific characters
 		    in the buffer with the 'syntax-table property.  It's
 		    always set - CC Mode no longer works in emacsen without

Re: Please fix before the release of 23.1

[Chong Yidong]

Ulrich Mueller <ulm@gentoo.org> writes:

>>>>>> On Sun, 10 Aug 2008, I wrote:
>
>> ,----[ test.el ]
>> | (progn
>> |   (require 'cc-mode))
>> `----
>
>> $ emacs -Q test.el
>> M-x eval-buffer
>
>> This will result in an error: Invalid read syntax: ")"
>
> The problem goes away if I revert the following change:
>
> 2008-06-27  Alan Mackenzie  <acm@muc.de>
>
> 	* progmodes/cc-defs.el (c-emacs-features): New feature
> 	'argumentative-bod-function.

Thanks for pinpointing this.

In fact, it's apparently not a problem with cc-mode, but with Emacs.
Here's a stripped-down recipe:

1. Create a file foo.el in your lisp path with the following contents:

(defconst foo
  (let ((beginning-of-defun-function
	 (lambda (&optional arg)
	   (not (eq arg nil)))))
    (progn
      (beginning-of-defun 1))))

(provide 'foo)

2. Byte-compile it (M-x byte-compile-file RET).

3. Create a file test.el with the following contents:

(progn
  (require 'foo))

4. emacs test.el

5. M-x eval-buffer

Debugger entered--Lisp error: (invalid-read-syntax ")")
  eval-buffer()  ; Reading at buffer position 25
  call-interactively(eval-buffer t nil)
  execute-extended-command(nil)
  call-interactively(execute-extended-command nil nil)


I don't know what causes this bug, though.  Anyone?

Re: Please fix before the release of 23.1

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

> (defconst foo
>   (let ((beginning-of-defun-function
> 	 (lambda (&optional arg)
> 	   (not (eq arg nil)))))
>     (progn
>       (beginning-of-defun 1))))

By the way, replacing `progn' with `save-excursion' makes the bug go
away (this makes cc-def.el work too).  Why?  I don't know.

Re: bug#636: Please fix before the release of 23.1

[Ulrich Mueller]

>>>>> On Sun, 10 Aug 2008, Chong Yidong wrote:

> Thanks for pinpointing this.

> In fact, it's apparently not a problem with cc-mode, but with Emacs.
> Here's a stripped-down recipe:

> 1. Create a file foo.el in your lisp path with the following contents:

> (defconst foo
>   (let ((beginning-of-defun-function
> 	 (lambda (&optional arg)
> 	   (not (eq arg nil)))))
>     (progn
>       (beginning-of-defun 1))))

> (provide 'foo)

Calling beginning-of-defun will move point, right?

And since beginning-of-defun-function is essentially defined as a
no-op, point will just be moved to the beginning-of-line. (In fact,
eval-buffer on foo.el results in an infinite loop.)

> By the way, replacing `progn' with `save-excursion' makes the bug go
> away (this makes cc-def.el work too). Why? I don't know.

Indeed. If we agree that the bug is in cc-defs.el, then the following
patch should fix it:

--- progmodes/cc-defs.el	27 Jun 2008 21:12:46 -0000	1.58
+++ progmodes/cc-defs.el	11 Aug 2008 07:11:04 -0000
@@ -1445,7 +1445,7 @@
     (let ((beginning-of-defun-function
 	   (lambda (&optional arg)
 	     (not (eq arg nil)))))
-      (if (beginning-of-defun 1)
+      (if (save-excursion (beginning-of-defun 1))
 	  (setq list (cons 'argumentative-bod-function list))))
 
     (let ((buf (generate-new-buffer " test"))

Ulrich

Re: bug#636: Please fix before the release of 23.1

[Alan Mackenzie]

Hi, Ulrich and Yidong!

Thanks for CC'ing me in on this one.

On Mon, Aug 11, 2008 at 09:18:55AM +0200, Ulrich Mueller wrote:
> >>>>> On Sun, 10 Aug 2008, Chong Yidong wrote:

> > In fact, it's apparently not a problem with cc-mode, but with Emacs.
> > Here's a stripped-down recipe:

> > 1. Create a file foo.el in your lisp path with the following contents:

> > (defconst foo
> >   (let ((beginning-of-defun-function
> > 	 (lambda (&optional arg)
> > 	   (not (eq arg nil)))))
> >     (progn
> >       (beginning-of-defun 1))))

> > (provide 'foo)

> Calling beginning-of-defun will move point, right?

> And since beginning-of-defun-function is essentially defined as a
> no-op, point will just be moved to the beginning-of-line. (In fact,
> eval-buffer on foo.el results in an infinite loop.)

> > By the way, replacing `progn' with `save-excursion' makes the bug go
> > away (this makes cc-def.el work too). Why? I don't know.

> Indeed. If we agree that the bug is in cc-defs.el, then the following
> patch should fix it:

> --- progmodes/cc-defs.el	27 Jun 2008 21:12:46 -0000	1.58
> +++ progmodes/cc-defs.el	11 Aug 2008 07:11:04 -0000
> @@ -1445,7 +1445,7 @@
>      (let ((beginning-of-defun-function
>  	   (lambda (&optional arg)
>  	     (not (eq arg nil)))))
> -      (if (beginning-of-defun 1)
> +      (if (save-excursion (beginning-of-defun 1))
>  	  (setq list (cons 'argumentative-bod-function list))))

>      (let ((buf (generate-new-buffer " test"))

I protest against this "fix".  :-)  Not against the fix as such (though
it is somewhat ugly, and would need some comments), just that the bug it
fixes is not understood.  Therefore, it is NOT a fix.  It is still a bug,
which will cause pain somewhere else.

> Ulrich

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: bug#636: Please fix before the release of 23.1

[Andreas Schwab]

Alan Mackenzie <acm@muc.de> writes:

> it is somewhat ugly, and would need some comments), just that the bug it
> fixes is not understood.

It's pretty simple.  You are moving point while load/eval-buffer/etc. is
reading the buffer, where reading the buffer always means reading from
point.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: bug#636: Please fix before the release of 23.1

[Alan Mackenzie]

Hi, Andreas

On Mon, Aug 11, 2008 at 10:54:59AM +0200, Andreas Schwab wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > it is somewhat ugly, and would need some comments), just that the bug
> > it fixes is not understood.

> It's pretty simple.  You are moving point while load/eval-buffer/etc.
> is reading the buffer, where reading the buffer always means reading
> from point.

Ah!  I missed that, because it wasn't me (in the capacity of CC Mode
maintainer) that moved point.

Thanks!

> Andreas.

> -- 
> Andreas Schwab, SuSE Labs, schwab@suse.de
> SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
> PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
> "And now for something completely different."

Re: bug#636: Please fix before the release of 23.1

[Michael Olson]

Ulrich Mueller <ulm@gentoo.org> writes:

> Indeed. If we agree that the bug is in cc-defs.el, then the following
> patch should fix it:
>
> --- progmodes/cc-defs.el	27 Jun 2008 21:12:46 -0000	1.58
> +++ progmodes/cc-defs.el	11 Aug 2008 07:11:04 -0000
> @@ -1445,7 +1445,7 @@
>      (let ((beginning-of-defun-function
>  	   (lambda (&optional arg)
>  	     (not (eq arg nil)))))
> -      (if (beginning-of-defun 1)
> +      (if (save-excursion (beginning-of-defun 1))
>  	  (setq list (cons 'argumentative-bod-function list))))
>
>      (let ((buf (generate-new-buffer " test"))

This patch fixes the problem for me as well.  It's interesting that
evaluating a top-level defconst definition can screw up the
byte-compiler though ... would it make sense for the byte-compiler to
automatically do a save-excursion around the processing of each
top-level form in a given file?

-- 
|       Michael Olson  |  FSF Associate Member #652     |
| http://mwolson.org/  |  Hobbies: Lisp, HCoop          |
| Projects: Emacs, Muse, ERC, EMMS, ErBot, DVC, Planner |
`-------------------------------------------------------'

Re: bug#636: Please fix before the release of 23.1

[Chong Yidong]

Ulrich Mueller <ulm@gentoo.org> writes:

>> (defconst foo
>>   (let ((beginning-of-defun-function
>> 	 (lambda (&optional arg)
>> 	   (not (eq arg nil)))))
>>     (progn
>>       (beginning-of-defun 1))))
>
>> (provide 'foo)
>
> Calling beginning-of-defun will move point, right?
>
> And since beginning-of-defun-function is essentially defined as a
> no-op, point will just be moved to the beginning-of-line. (In fact,
> eval-buffer on foo.el results in an infinite loop.)

Aha, I see.

So the question is, should we make the change to cc-defs.el, or (as
suggested by Michael Olson) make the byte-compiler wrap around each
variable definition with `save-excursion'?

Even with the latter solution, incorrectly written elisp in variable
definitions can still create problems.  For example, `save-excursion'
won't save us if the elisp runs `kill-buffer'.

This indicates that we fix cc-defs.el, and leave the rest of Emacs
alone.

Re: bug#636: Please fix before the release of 23.1

[Alan Mackenzie]

Hi, everybody!

On Mon, Aug 11, 2008 at 12:30:04PM -0400, Chong Yidong wrote:
> Ulrich Mueller <ulm@gentoo.org> writes:

> >> (defconst foo
> >>   (let ((beginning-of-defun-function
> >> 	 (lambda (&optional arg)
> >> 	   (not (eq arg nil)))))
> >>     (progn
> >>       (beginning-of-defun 1))))

> >> (provide 'foo)

> > Calling beginning-of-defun will move point, right?

> > And since beginning-of-defun-function is essentially defined as a
> > no-op, point will just be moved to the beginning-of-line. (In fact,
> > eval-buffer on foo.el results in an infinite loop.)

> Aha, I see.

> So the question is, should we make the change to cc-defs.el, or (as
> suggested by Michael Olson) make the byte-compiler wrap around each
> variable definition with `save-excursion'?

The way I see it, it's a bug in `beginning-of-defun'.  The docstring
says that a non-nil `beginning-of-defun-function' finds the pertinent
place, but beginning-of-defun then moves point somewhere else.  I HATE
things that are ostensibly "helpful", but in reality are dumbing down,
and mainly just foul things up.

I would be in favour of NOT moving point after a BOD-function has done
its work.  However, I wouldn't campaign too hard about it, because ....

CC Mode should remain compatible with lots of Emacsen, including 21.n.
That means I'll have to modify CC Mode anyway.  So I will.

> Even with the latter solution, incorrectly written elisp in variable
> definitions can still create problems.  For example, `save-excursion'
> won't save us if the elisp runs `kill-buffer'.

Hey, that's an idea!  ;-)

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: bug#636: Please fix before the release of 23.1

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

> The way I see it, it's a bug in `beginning-of-defun'.  The docstring
> says that a non-nil `beginning-of-defun-function' finds the pertinent
> place, but beginning-of-defun then moves point somewhere else.  I HATE
> things that are ostensibly "helpful", but in reality are dumbing down,
> and mainly just foul things up.

This is part of the baggage in beginning-of-defun regarding whether
column zero is the beginning of a defun.  Changing this now might be
unwise, so for the moment I've simply revised the docstring of
beginning-of-defun for extra clarity.

I've also changed cc-defs.el to use beginning-of-defun-raw, which is a
more direct and side-effect free test of beginning-of-defun-function,
and added a save-excursion for extra plus safety ;-)

[Also, closing bug#636.]

Re: bug#636: Please fix before the release of 23.1

[Alan Mackenzie]

Hi, Yidong!

On Mon, Aug 11, 2008 at 05:42:22PM -0400, Chong Yidong wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > The way I see it, it's a bug in `beginning-of-defun'.  The docstring
> > says that a non-nil `beginning-of-defun-function' finds the pertinent
> > place, but beginning-of-defun then moves point somewhere else.  I
> > HATE things that are ostensibly "helpful", but in reality are dumbing
> > down, and mainly just foul things up.

> This is part of the baggage in beginning-of-defun regarding whether
> column zero is the beginning of a defun.  Changing this now might be
> unwise, so for the moment I've simply revised the docstring of
> beginning-of-defun for extra clarity.

Thanks!

> I've also changed cc-defs.el to use beginning-of-defun-raw, which is a
> more direct and side-effect free test of beginning-of-defun-function,
> and added a save-excursion for extra plus safety ;-)

I've just added a bit to bind mark-ring, because BOD adds a mark.

I'm not actually convinced that using bod-raw is a good idea, since it
feels like using an internal interface (which we might want to refactor
away later).  But it's not that big a deal, so I've left it.

> [Also, closing bug#636.]

:-)

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: bug#636: Please fix before the release of 23.1

[Johan Bockgård]

Ulrich Mueller <ulm@gentoo.org> writes:

> -      (if (beginning-of-defun 1)
> +      (if (save-excursion (beginning-of-defun 1))

(if (beginning-of-defun-raw 1)

Re: Please fix before the release of 23.1

[Stefan Monnier]

>> ,----[ test.el ]
>> | (progn
>> |   (require 'cc-mode))
>> `----
>> $ emacs -Q test.el
>> M-x eval-buffer
>> This will result in an error: Invalid read syntax: ")"
> The problem goes away if I revert the following change:

> 	* progmodes/cc-defs.el (c-emacs-features): New feature
> 	'argumentative-bod-function.
[...] 
> +    ;; In Emacs >= 23, beginning-of-defun will passes its parameter to
> +    ;; beginning-of-defun-function.  Assume end-of-defun does the same.

I don't understand the code, but end-of-defun does not pass its argument
to end-of-defun function (instead, it passes it to
beginning-of-defun-function to find the beginning of the Nth function
and then calls end-of-defun to jump to its end).


        Stefan

Re: Please fix before the release of 23.1

[Glenn Morris]

Ulrich Mueller wrote:

> | (progn
> |   (require 'cc-mode))
> `----
[...]
> This will result in an error: Invalid read syntax: ")"
[...]
> The error disappears if the expression is in one line:
>
> ,----[ test.el ]
> | (progn (require 'cc-mode))
> `----

Already filed as bug #636.