CC Mode 5.31.4 (C/l); c-end-of-defun broken

CC Mode 5.31.4 (C/l); c-end-of-defun broken

[Andreas Schwab]

c-end-of-defun no longer always goes to the end of the function.  This
breaks add-log-current-defun, for example.  It doesn't always happen, but
you can reproduce it with
<http://sourceware.org/cgi-bin/cvsweb.cgi/~checkout~/src/gdb/symtab.c?rev=1.157&content-type=text/plain&cvsroot=src>.
Just load the file and type M-C-e.  You end up in the middle of
lookup_partial_symtab instead of at the end of lookup_symtab.

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: CC Mode 5.31.4 (C/l); c-end-of-defun broken

[Glenn Morris]

Andreas Schwab wrote:

> c-end-of-defun no longer always goes to the end of the function.

The problem seems to have been introduced in rev 1.47 of cc-cmds.el.
It's only a 400 line change...

Re: CC Mode 5.31.4 (C/l); c-end-of-defun broken

[Alan Mackenzie]

'Evening, Glenn!

On Wed, Apr 18, 2007 at 01:32:15PM -0400, Glenn Morris wrote:
> Andreas Schwab wrote:
 
> > c-end-of-defun no longer always goes to the end of the function.
 
> The problem seems to have been introduced in rev 1.47 of cc-cmds.el.
> It's only a 400 line change...

;-)  It was indeed a ~400 line change.  It was, however, largely of the
nature of a refactoring.  Even though it introduced the current bug, the
improved structure of the code made the bug's diagnosis very easy.

Had a similar bug been found in the previous code, tracking it down
would have taken many days rather than a few minutes.  Indeed, just such
a similar bug was the spur for my refactoring the code in the first
place.

-- 
Alan Mackenzie (Ittersbach, Germany).

Re: CC Mode 5.31.4 (C/l); c-end-of-defun broken

[Chong Yidong]

Andreas Schwab <schwab@suse.de> writes:

> c-end-of-defun no longer always goes to the end of the function.  This
> breaks add-log-current-defun, for example.  It doesn't always happen, but
> you can reproduce it with
> <http://sourceware.org/cgi-bin/cvsweb.cgi/~checkout~/src/gdb/symtab.c?rev=1.157&content-type=text/plain&cvsroot=src>.
> Just load the file and type M-C-e.  You end up in the middle of
> lookup_partial_symtab instead of at the end of lookup_symtab.

The trouble is the following code in c-end-of-defun (cc-cmds.el:1645):

      ;; Move forward to the } of a function
      (if (> arg 0)
	  (setq arg (c-forward-to-nth-EOF-} arg where))))

    ;; Do we need to move forward from the brace to the semicolon?
    (when (eq arg 0)
      (if (c-in-function-trailer-p)	; after "}" of struct/enum, etc.
	  (c-syntactic-re-search-forward ";"))

The function c-in-function-trailer-p thinks we are between the closing
brace and the semicolon of struct construct, because this function
returns a struct *:

  struct symtab *
  lookup_symtab (const char *name)
  {

Alan, can you come up with a quick fix?

Re: CC Mode 5.31.4 (C/l); c-end-of-defun broken

[Alan Mackenzie]

'Evening, Chong!

On Wed, Apr 18, 2007 at 02:10:05PM -0400, Chong Yidong wrote:
> Andreas Schwab <schwab@suse.de> writes:

> > c-end-of-defun no longer always goes to the end of the function.  This
> > breaks add-log-current-defun, for example.  It doesn't always happen, but
> > you can reproduce it with
> > <http://sourceware.org/cgi-bin/cvsweb.cgi/~checkout~/src/gdb/symtab.c?rev=1.157&content-type=text/plain&cvsroot=src>.
> > Just load the file and type M-C-e.  You end up in the middle of
> > lookup_partial_symtab instead of at the end of lookup_symtab.

> The trouble is the following code in c-end-of-defun (cc-cmds.el:1645):
> 
>       ;; Move forward to the } of a function
>       (if (> arg 0)
> 	  (setq arg (c-forward-to-nth-EOF-} arg where))))
> 
>     ;; Do we need to move forward from the brace to the semicolon?
>     (when (eq arg 0)
>       (if (c-in-function-trailer-p)	; after "}" of struct/enum, etc.
> 	  (c-syntactic-re-search-forward ";"))

> The function c-in-function-trailer-p thinks we are between the closing
> brace and the semicolon of struct construct, because this function
> returns a struct *:

>   struct symtab *
>   lookup_symtab (const char *name)
>   {

> Alan, can you come up with a quick fix?

Yes.

There is code which analyses `struct' headers properly in the function
`c-end-of-decl-1' in cc-engine.el.  I can copy this into
`c-in-function-trailer-p'.

I might be able to post a patch tonight.  If not, tomorrow.

-- 
Alan.

Re: CC Mode 5.31.4 (C/l); c-end-of-defun broken

[Alan Mackenzie]

Hi, everybody!

On Wed, Apr 18, 2007 at 02:10:05PM -0400, Chong Yidong wrote:
> Andreas Schwab <schwab@suse.de> writes:

> > c-end-of-defun no longer always goes to the end of the function.  This
> > breaks add-log-current-defun, for example.  It doesn't always happen, but
> > you can reproduce it with
> > <http://sourceware.org/cgi-bin/cvsweb.cgi/~checkout~/src/gdb/symtab.c?rev=1.157&content-type=text/plain&cvsroot=src>.
> > Just load the file and type M-C-e.  You end up in the middle of
> > lookup_partial_symtab instead of at the end of lookup_symtab.

> Alan, can you come up with a quick fix?

Could people please test this fix (which I haven't yet committed):


2007-04-18  acm  <acm@localhost.localdomain>

	* progmodes/cc-cmds.el (c-in-function-trailer-p): Fix this: when a
	function type contains "struct", "union", etc. c-end-of-defun goes
	too far forward.


Index: cc-cmds.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/progmodes/cc-cmds.el,v
retrieving revision 1.59
diff -c -r1.59 cc-cmds.el
*** cc-cmds.el	17 Apr 2007 20:48:43 -0000	1.59
--- cc-cmds.el	18 Apr 2007 20:02:47 -0000
***************
*** 1358,1369 ****
    ;;
    ;; This function might do hidden buffer changes.
    (and c-opt-block-decls-with-vars-key
!        (save-excursion
! 	 (c-syntactic-skip-backward "^;}" lim)
! 	 (and (eq (char-before) ?\})
! 	      (eq (car (c-beginning-of-decl-1 lim)) 'previous)
! 	      (looking-at c-opt-block-decls-with-vars-key)
! 	      (point)))))
  
  (defun c-where-wrt-brace-construct ()
    ;; Determine where we are with respect to functions (or other brace
--- 1358,1382 ----
    ;;
    ;; This function might do hidden buffer changes.
    (and c-opt-block-decls-with-vars-key
!        (let (bod)
! 	 (save-excursion
! 	   (c-syntactic-skip-backward "^;}" lim)
! 	   (and (eq (char-before) ?\})
! 		(eq (car (c-beginning-of-decl-1 lim)) 'previous)
! 		(setq bod (point))
! 		;; Look for struct or union or ...  If we find one, it might
! 		;; be the return type of a function, or the like.  Exclude
! 		;; this case.
! 		(c-syntactic-re-search-forward
! 		 (concat "[;=\(\[{]\\|\\("
! 			 c-opt-block-decls-with-vars-key
! 			 "\\)")
! 		 lim t t t)
! 		(match-beginning 1)	; Is there a "struct" etc., somewhere?
! 		(not (eq (char-before) ?_))
! 		(c-syntactic-re-search-forward "[;=\(\[{]" lim t t t)
! 		(eq (char-before) ?\{)
! 		bod)))))
  
  (defun c-where-wrt-brace-construct ()
    ;; Determine where we are with respect to functions (or other brace


-- 
Alan.

Re: CC Mode 5.31.4 (C/l); c-end-of-defun broken

[Chong Yidong]

Hi Alan,

> Could people please test this fix (which I haven't yet committed):

The behavior seems to be correct, as far as I can tell.  However:

> !             ;; Look for struct or union or ...  If we find one, it might
> !             ;; be the return type of a function, or the like.  Exclude
> !             ;; this case.
> !             (c-syntactic-re-search-forward
> !              (concat "[;=\(\[{]\\|\\("
> !                      c-opt-block-decls-with-vars-key
> !                      "\\)")
> !              lim t t t)

Do you really want to use `lim' here?  That argument, if non-nil, is
supposed to give a lower bound for the search, starting from point.  I
think the correct limit you are looking for is the original point
where we started.

(In fact, c-in-function-trailer-p is not called with a non-nil
argument anywhere in Emacs, so this bug wouldn't show up, but it's
better to be safe than sorry.)

Similarly here:

> !             (c-syntactic-re-search-forward "[;=\(\[{]" lim t t t)

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: CC Mode 5.31.4 (C/l); c-end-of-defun broken

[Alan Mackenzie]

Hi, Chong,

On Wed, Apr 18, 2007 at 04:43:15PM -0400, Chong Yidong wrote:
> Hi Alan,

> The behavior seems to be correct, as far as I can tell.  However:

> > !             ;; Look for struct or union or ...  If we find one, it might
> > !             ;; be the return type of a function, or the like.  Exclude
> > !             ;; this case.
> > !             (c-syntactic-re-search-forward
> > !              (concat "[;=\(\[{]\\|\\("
> > !                      c-opt-block-decls-with-vars-key
> > !                      "\\)")
> > !              lim t t t)

> Do you really want to use `lim' here?  That argument, if non-nil, is
> supposed to give a lower bound for the search, starting from point.  I
> think the correct limit you are looking for is the original point
> where we started.

You're right.  I did some careless copying and yanking from somewhere
else in the code.  Sorry.

> (In fact, c-in-function-trailer-p is not called with a non-nil
> argument anywhere in Emacs, so this bug wouldn't show up, but it's
> better to be safe than sorry.)

Code should (i.e. must) do what its comments say.  ;-)

> Similarly here:

> > !             (c-syntactic-re-search-forward "[;=\(\[{]" lim t t t)

Yes.

This should be better:


2007-04-18  acm  <acm@localhost.localdomain>

	* progmodes/cc-cmds.el (c-in-function-trailer-p): Fix this: when a
	function type contains "struct", "union", etc. c-end-of-defun goes
	too far forward.


Index: cc-cmds.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/progmodes/cc-cmds.el,v
retrieving revision 1.59
diff -c -r1.59 cc-cmds.el
*** cc-cmds.el	17 Apr 2007 20:48:43 -0000	1.59
--- cc-cmds.el	18 Apr 2007 21:23:26 -0000
***************
*** 1360,1369 ****
    (and c-opt-block-decls-with-vars-key
         (save-excursion
  	 (c-syntactic-skip-backward "^;}" lim)
! 	 (and (eq (char-before) ?\})
! 	      (eq (car (c-beginning-of-decl-1 lim)) 'previous)
! 	      (looking-at c-opt-block-decls-with-vars-key)
! 	      (point)))))
  
  (defun c-where-wrt-brace-construct ()
    ;; Determine where we are with respect to functions (or other brace
--- 1360,1383 ----
    (and c-opt-block-decls-with-vars-key
         (save-excursion
  	 (c-syntactic-skip-backward "^;}" lim)
! 	 (let ((eo-block (point))
! 	       bod)
! 	   (and (eq (char-before) ?\})
! 		(eq (car (c-beginning-of-decl-1 lim)) 'previous)
! 		(setq bod (point))
! 		;; Look for struct or union or ...  If we find one, it might
! 		;; be the return type of a function, or the like.  Exclude
! 		;; this case.
! 		(c-syntactic-re-search-forward
! 		 (concat "[;=\(\[{]\\|\\("
! 			 c-opt-block-decls-with-vars-key
! 			 "\\)")
! 		 eo-block t t t)
! 		(match-beginning 1)	; Is there a "struct" etc., somewhere?
! 		(not (eq (char-before) ?_))
! 		(c-syntactic-re-search-forward "[;=\(\[{]" eo-block t t t)
! 		(eq (char-before) ?\{)
! 		bod)))))
  
  (defun c-where-wrt-brace-construct ()
    ;; Determine where we are with respect to functions (or other brace


I'm off to bed, now.  If everything's OK, I'll commit the patch tomorrow.

Thanks for checking my patch.

-- 
Alan.

Re: CC Mode 5.31.4 (C/l); c-end-of-defun broken

[Alan Mackenzie]

Hi, again!

> If everything's OK, I'll commit the patch tomorrow.

I've just committed it.

-- 
Alan.

Re: CC Mode 5.31.4 (C/l); c-end-of-defun broken

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

> Hi, again!
>
>> If everything's OK, I'll commit the patch tomorrow.
>
> I've just committed it.

Thanks.

Re: CC Mode 5.31.4 (C/l); c-end-of-defun broken

[Richard Stallman]

Does this mean you have dealt with the FOR-RELEASE item about this?

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: CC Mode 5.31.4 (C/l); c-end-of-defun broken

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

> Does this mean you have dealt with the FOR-RELEASE item about this?

ACM did.