GNU bug report logs - #25623
CC Mode 5.32.99 (C/l); `c-defun-name' returns wrong result for filescope enums, structs and unions

Previous Next

Package: cc-mode;

Reported by: Mohammed Sadiq <sadiq <at> sadiqpk.org>

Date: Sun, 5 Feb 2017 02:52:02 UTC

Severity: normal

Done: Alan Mackenzie <acm <at> muc.de>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25623 in the body.
You can then email your comments to 25623 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-cc-mode <at> gnu.org:
bug#25623; Package cc-mode. (Sun, 05 Feb 2017 02:52:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Mohammed Sadiq <sadiq <at> sadiqpk.org>:
New bug report received and forwarded. Copy sent to bug-cc-mode <at> gnu.org. (Sun, 05 Feb 2017 02:52:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mohammed Sadiq <sadiq <at> sadiqpk.org>
To: submit <at> debbugs.gnu.org
Subject: CC Mode 5.32.99 (C/l);
 `c-defun-name' returns wrong result for filescope enums, structs and
 unions
Date: Sun, 05 Feb 2017 08:17:50 +0530
When the point is in struct, enum or union of file scope, `c-defun-name'
returns wrong result.

Eg: In the following example (where `|' is where the points are (ie, 3
cursors are 3 different cases),
case 1, in struct: the result is "t" ("struct test" may be a better
result, the same for union)
case 2, in enum: the result is "ONE" ("enum" may be better)
case 3, in main: the result is "main" (which is of course, right)

Also, simply return "struct", "union", etc if no tag name is present
(like in "typedef struct {...")

struct test
{
  int a;|
};

enum {
  ONE,
  TWO|
};

int
main (void)
{
  |
}


Thanks


Emacs  : GNU Emacs 26.0.50.2 (x86_64-pc-linux-gnu, GTK+ Version 3.22.6)
 of 2017-01-20
Package: CC Mode 5.32.99 (C/l)
Buffer Style: gnu
c-emacs-features: (pps-extended-state col-0-paren posix-char-classes gen-string-delim gen-comment-delim syntax-properties 1-bit)

current state:
==============
(setq
 c-basic-offset 2
 c-comment-only-line-offset '(0 . 0)
 c-indent-comment-alist '((anchored-comment column . 0) (end-block space . 1)
			  (cpp-end-block space . 2))
 c-indent-comments-syntactically-p nil
 c-block-comment-prefix ""
 c-comment-prefix-regexp '((pike-mode . "//+!?\\|\\**") (awk-mode . "#+")
			   (other . "//+\\|\\**"))
 c-doc-comment-style '((java-mode . javadoc) (pike-mode . autodoc)
		       (c-mode . gtkdoc))
 c-cleanup-list '(scope-operator)
 c-hanging-braces-alist '((substatement-open before after)
			  (arglist-cont-nonempty))
 c-hanging-colons-alist nil
 c-hanging-semi&comma-criteria '(c-semi&comma-inside-parenlist)
 c-backslash-column 48
 c-backslash-max-column 72
 c-special-indent-hook '(c-gnu-impose-minimum)
 c-label-minimum-indentation 1
 c-offsets-alist '((inexpr-class . +)
		   (inexpr-statement . +)
		   (lambda-intro-cont . +)
		   (inlambda . c-lineup-inexpr-block)
		   (template-args-cont c-lineup-template-args +)
		   (incomposition . +)
		   (inmodule . +)
		   (innamespace . +)
		   (inextern-lang . +)
		   (composition-close . 0)
		   (module-close . 0)
		   (namespace-close . 0)
		   (extern-lang-close . 0)
		   (composition-open . 0)
		   (module-open . 0)
		   (namespace-open . 0)
		   (extern-lang-open . 0)
		   (objc-method-call-cont
		    c-lineup-ObjC-method-call-colons
		    c-lineup-ObjC-method-call
		    +
		    )
		   (objc-method-args-cont . c-lineup-ObjC-method-args)
		   (objc-method-intro . [0])
		   (friend . 0)
		   (cpp-define-intro c-lineup-cpp-define +)
		   (cpp-macro-cont . +)
		   (cpp-macro . [0])
		   (inclass . +)
		   (stream-op . c-lineup-streamop)
		   (arglist-cont-nonempty
		    c-lineup-gcc-asm-reg
		    c-lineup-arglist
		    )
		   (arglist-cont c-lineup-gcc-asm-reg 0)
		   (comment-intro c-lineup-knr-region-comment c-lineup-comment)
		   (catch-clause . 0)
		   (else-clause . 0)
		   (do-while-closure . 0)
		   (access-label . -)
		   (case-label . 0)
		   (substatement . +)
		   (statement-case-intro . +)
		   (statement . 0)
		   (brace-entry-open . 0)
		   (brace-list-entry . 0)
		   (brace-list-intro . +)
		   (brace-list-close . 0)
		   (block-close . 0)
		   (block-open . 0)
		   (inher-cont . c-lineup-multi-inher)
		   (inher-intro . +)
		   (member-init-cont . c-lineup-multi-inher)
		   (member-init-intro . +)
		   (annotation-var-cont . +)
		   (annotation-top-cont . 0)
		   (topmost-intro . 0)
		   (knr-argdecl . 0)
		   (func-decl-cont . +)
		   (inline-close . 0)
		   (class-close . 0)
		   (class-open . 0)
		   (defun-block-intro . +)
		   (defun-close . 0)
		   (defun-open . 0)
		   (c . c-lineup-C-comments)
		   (string . c-lineup-dont-change)
		   (topmost-intro-cont
		    first
		    c-lineup-topmost-intro-cont
		    c-lineup-gnu-DEFUN-intro-cont
		    )
		   (brace-list-open . +)
		   (inline-open . 0)
		   (arglist-close . c-lineup-arglist)
		   (arglist-intro . c-lineup-arglist-intro-after-paren)
		   (statement-cont . +)
		   (statement-case-open . +)
		   (label . 0)
		   (substatement-label . 0)
		   (substatement-open . +)
		   (knr-argdecl-intro . 5)
		   (statement-block-intro . +)
		   )
 c-buffer-is-cc-mode 'c-mode
 c-tab-always-indent t
 c-syntactic-indentation t
 c-syntactic-indentation-in-macros t
 c-ignore-auto-fill '(string cpp code)
 c-auto-align-backslashes t
 c-backspace-function 'backward-delete-char-untabify
 c-delete-function 'delete-char
 c-electric-pound-behavior nil
 c-default-style '((java-mode . "java") (awk-mode . "awk") (other . "gnu"))
 c-enable-xemacs-performance-kludge-p nil
 c-old-style-variable-behavior nil
 defun-prompt-regexp nil
 tab-width 8
 comment-column 32
 parse-sexp-ignore-comments t
 parse-sexp-lookup-properties t
 auto-fill-function nil
 comment-multi-line t
 comment-start-skip "\\(//+\\|/\\*+\\)\\s *"
 fill-prefix nil
 fill-column 70
 paragraph-start "[ 	]*\\(//+\\|\\**\\)[ 	]*$\\|^\f"
 adaptive-fill-mode t
 adaptive-fill-regexp "[ 	]*\\(//+\\|\\**\\)[ 	]*\\([ 	]*\\([-–!|#%;>*·•‣⁃◦]+[ 	]*\\)*\\)"
 )




Information forwarded to bug-cc-mode <at> gnu.org:
bug#25623; Package cc-mode. (Sun, 23 Apr 2017 12:19:02 GMT) Full text and rfc822 format available.

Message #8 received at 25623 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Mohammed Sadiq <sadiq <at> sadiqpk.org>
Cc: 25623 <at> debbugs.gnu.org
Subject: Re: bug#25623: CC Mode 5.32.99 (C/l); `c-defun-name' returns wrong
 result for filescope enums, structs and unions
Date: Sun, 23 Apr 2017 12:18:05 +0000
Hello, Mohammed.

On Sun, Feb 05, 2017 at 08:17:50 +0530, Mohammed Sadiq wrote:

> When the point is in struct, enum or union of file scope, `c-defun-name'
> returns wrong result.

> Eg: In the following example (where `|' is where the points are (ie, 3
> cursors are 3 different cases),
> case 1, in struct: the result is "t" ("struct test" may be a better
> result, the same for union)
> case 2, in enum: the result is "ONE" ("enum" may be better)
> case 3, in main: the result is "main" (which is of course, right)

> Also, simply return "struct", "union", etc if no tag name is present
> (like in "typedef struct {...")

> struct test
> {
>   int a;|
> };

> enum {
>   ONE,
>   TWO|
> };

> int
> main (void)
> {
>   |
> }

Thanks for the bug report.  This bug is simply a matter of buggy code in
CC Mode.  There follows a patch.

Would you please apply this patch, compile cc-cmds.el, try it out, then
either confirm to me that the bug is fixed, or tell me what still needs
looking at.  Thanks in advance!



diff -r f78dfc813575 cc-cmds.el
--- a/cc-cmds.el	Sat Apr 22 14:18:55 2017 +0000
+++ b/cc-cmds.el	Sun Apr 23 12:09:06 2017 +0000
@@ -1769,19 +1769,25 @@
 	  (unless (eq where 'at-header)
 	    (c-backward-to-nth-BOF-{ 1 where)
 	    (c-beginning-of-decl-1))
+	  (when (looking-at c-typedef-key)
+	    (goto-char (match-end 0))
+	    (c-forward-syntactic-ws))
 
 	  ;; Pick out the defun name, according to the type of defun.
 	  (cond
 	   ;; struct, union, enum, or similar:
-	   ((and (looking-at c-type-prefix-key)
-		 (progn (c-forward-token-2 2) ; over "struct foo "
-			(or (eq (char-after) ?\{)
-			    (looking-at c-symbol-key)))) ; "struct foo bar ..."
-	    (save-match-data (c-forward-token-2))
-	    (when (eq (char-after) ?\{)
-	      (c-backward-token-2)
-	      (looking-at c-symbol-key))
-	    (match-string-no-properties 0))
+	   ((looking-at c-type-prefix-key)
+	    (let ((key-pos (point)))
+	      (c-forward-token-2 1)	; over "struct ".
+	      (cond
+	       ((looking-at c-symbol-key)	; "struct foo { ..."
+		(buffer-substring-no-properties key-pos (match-end 0)))
+	       ((eq (char-after) ?{)	; "struct { ... } foo"
+		(when (c-go-list-forward)
+		  (c-forward-syntactic-ws)
+		  (when (looking-at c-symbol-key) ; a bit bogus - there might
+						  ; be several identifiers.
+		    (match-string-no-properties 0)))))))
 
 	   ((looking-at "DEFUN\\s-*(") ;"DEFUN\\_>") think of XEmacs!
 	    ;; DEFUN ("file-name-directory", Ffile_name_directory, Sfile_name_directory, ...) ==> Ffile_name_directory



> Thanks

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).




Information forwarded to bug-cc-mode <at> gnu.org:
bug#25623; Package cc-mode. (Sun, 23 Apr 2017 18:31:02 GMT) Full text and rfc822 format available.

Message #11 received at 25623 <at> debbugs.gnu.org (full text, mbox):

From: Mohammed Sadiq <sadiq <at> sadiqpk.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 25623 <at> debbugs.gnu.org
Subject: Re: bug#25623: CC Mode 5.32.99 (C/l); `c-defun-name' returns wrong
 result for filescope enums, structs and unions
Date: Mon, 24 Apr 2017 00:00:07 +0530 (IST)
Hi,

> On April 23, 2017 at 5:48 PM Alan Mackenzie <acm <at> muc.de> wrote:
> Thanks for the bug report.  This bug is simply a matter of buggy code in
> CC Mode.  There follows a patch.
> 
> Would you please apply this patch, compile cc-cmds.el, try it out, then
> either confirm to me that the bug is fixed, or tell me what still needs
> looking at.  Thanks in advance!
> [code snipped]

Hi. I have a few test cases that this fails (not quiet bad, but may be better not 
to break)



enum
  {
   A,
   B,|
   C,
  };

gives 'nil', which would better return 'enum' as it is common to have enums without
names associated.

int a[] = {'h', 'e', 'l', 'l', 'o', '\0'|};

gives "[" which I believe, should actually return 'nil' when defined at file scope.

typedef struct
{
  int a;
  int b;|
} nice;

returns "nice". It may be better to return "struct nice" as in "struct nice {...};" definition.

Thanks
 
> Alan Mackenzie (Nuremberg, Germany).




Information forwarded to bug-cc-mode <at> gnu.org:
bug#25623; Package cc-mode. (Sun, 23 Apr 2017 21:03:01 GMT) Full text and rfc822 format available.

Message #14 received at 25623 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Mohammed Sadiq <sadiq <at> sadiqpk.org>
Cc: 25623 <at> debbugs.gnu.org
Subject: Re: bug#25623: CC Mode 5.32.99 (C/l); `c-defun-name' returns wrong
 result for filescope enums, structs and unions
Date: Sun, 23 Apr 2017 21:01:56 +0000
Hello, Mohammed.

Thanks for the fast reply.

On Mon, Apr 24, 2017 at 00:00:07 +0530, Mohammed Sadiq wrote:
> Hi,

> > On April 23, 2017 at 5:48 PM Alan Mackenzie <acm <at> muc.de> wrote:
> > Thanks for the bug report.  This bug is simply a matter of buggy code in
> > CC Mode.  There follows a patch.

> > Would you please apply this patch, compile cc-cmds.el, try it out, then
> > either confirm to me that the bug is fixed, or tell me what still needs
> > looking at.  Thanks in advance!
> > [code snipped]

> Hi. I have a few test cases that this fails (not quiet bad, but may be better not 
> to break)



> enum
>   {
>    A,
>    B,|
>    C,
>   };

> gives 'nil', which would better return 'enum' as it is common to have enums without
> names associated.

I've thought about this.  The problem is that "enum" is not a name - it
is a keyword.

> int a[] = {'h', 'e', 'l', 'l', 'o', '\0'|};

> gives "[" which I believe, should actually return 'nil' when defined at file scope.

It should indeed return nil.  Please see the amended patch (below).

> typedef struct
> {
>   int a;
>   int b;|
> } nice;

> returns "nice". It may be better to return "struct nice" as in "struct nice {...};" definition.

I considered that, too.  Even returning "nice" is a bit bogus, here.  For
example, if we had "typedef struct { ... } nice, easy;" why should we
return "nice" (which we do) rather than "easy"?  I think here, strictly
speaking, we should return nil, since "nice" is an identifier _after_ the
main type definition, not part of it.  But, then again, maybe here is not
the place to be strict.  ;-)

I think returning "struct nice" would be totally wrong, here.  That is
not the name of the struct, the way it is in "struct nice { ... }".

Here's the amended patch:



diff -r f78dfc813575 cc-cmds.el
--- a/cc-cmds.el	Sat Apr 22 14:18:55 2017 +0000
+++ b/cc-cmds.el	Sun Apr 23 20:38:14 2017 +0000
@@ -1769,19 +1769,25 @@
 	  (unless (eq where 'at-header)
 	    (c-backward-to-nth-BOF-{ 1 where)
 	    (c-beginning-of-decl-1))
+	  (when (looking-at c-typedef-key)
+	    (goto-char (match-end 0))
+	    (c-forward-syntactic-ws))
 
 	  ;; Pick out the defun name, according to the type of defun.
 	  (cond
 	   ;; struct, union, enum, or similar:
-	   ((and (looking-at c-type-prefix-key)
-		 (progn (c-forward-token-2 2) ; over "struct foo "
-			(or (eq (char-after) ?\{)
-			    (looking-at c-symbol-key)))) ; "struct foo bar ..."
-	    (save-match-data (c-forward-token-2))
-	    (when (eq (char-after) ?\{)
-	      (c-backward-token-2)
-	      (looking-at c-symbol-key))
-	    (match-string-no-properties 0))
+	   ((looking-at c-type-prefix-key)
+	    (let ((key-pos (point)))
+	      (c-forward-token-2 1)	; over "struct ".
+	      (cond
+	       ((looking-at c-symbol-key)	; "struct foo { ..."
+		(buffer-substring-no-properties key-pos (match-end 0)))
+	       ((eq (char-after) ?{)	; "struct { ... } foo"
+		(when (c-go-list-forward)
+		  (c-forward-syntactic-ws)
+		  (when (looking-at c-symbol-key) ; a bit bogus - there might
+						  ; be several identifiers.
+		    (match-string-no-properties 0)))))))
 
 	   ((looking-at "DEFUN\\s-*(") ;"DEFUN\\_>") think of XEmacs!
 	    ;; DEFUN ("file-name-directory", Ffile_name_directory, Sfile_name_directory, ...) ==> Ffile_name_directory
@@ -1808,7 +1814,8 @@
 		(c-backward-syntactic-ws))
 	      (setq name-end (point))
 	      (c-backward-token-2)
-	      (buffer-substring-no-properties (point) name-end)))))))))
+	      (and (looking-at c-symbol-start)
+		   (buffer-substring-no-properties (point) name-end))))))))))
 
 (defun c-declaration-limits (near)
   ;; Return a cons of the beginning and end positions of the current


> Thanks
 
-- 
Alan Mackenzie (Nuremberg, Germany).




Information forwarded to bug-cc-mode <at> gnu.org:
bug#25623; Package cc-mode. (Mon, 24 Apr 2017 03:45:02 GMT) Full text and rfc822 format available.

Message #17 received at 25623 <at> debbugs.gnu.org (full text, mbox):

From: Mohammed Sadiq <sadiq <at> sadiqpk.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 25623 <at> debbugs.gnu.org
Subject: Re: bug#25623: CC Mode 5.32.99 (C/l); `c-defun-name' returns wrong
 result for filescope enums, structs and unions
Date: Mon, 24 Apr 2017 09:14:27 +0530 (IST)
> On April 24, 2017 at 2:31 AM Alan Mackenzie <acm <at> muc.de> wrote:
> > 
> > enum
> >  {
> >  A,
> >  B,|
> >  C,
> >  };
> > 
> > gives 'nil', which would better return 'enum' as it is common to have enums without
> > names associated.
> 
> I've thought about this. The problem is that "enum" is not a name - it
> is a keyword.

Yeah, right, but as "enum" and "struct" can't be used function names, users can use
c-defun-name to check if the point is in struct or enum too.

> 
> > int a[] = {'h', 'e', 'l', 'l', 'o', '\0'|};
> > 
> > gives "[" which I believe, should actually return 'nil' when defined at file scope.
> 
> It should indeed return nil. Please see the amended patch (below).

Your patch fixed the issue.

> 
> > typedef struct
> > {
> >  int a;
> >  int b;|
> > } nice;
> > 
> > returns "nice". It may be better to return "struct nice" as in "struct nice {...};" definition.
> 
> I considered that, too. Even returning "nice" is a bit bogus, here. For
> example, if we had "typedef struct { ... } nice, easy;" why should we
> return "nice" (which we do) rather than "easy"? I think here, strictly
> speaking, we should return nil, since "nice" is an identifier _after_ the
> main type definition, not part of it. But, then again, maybe here is not
> the place to be strict. ;-)
> 

If in both cases, returning simply "struct" would be more helpful to the users
than nil, imho. the same for enum

Thanks

> [code snipped]
> --
> Alan Mackenzie (Nuremberg, Germany).




Reply sent to Alan Mackenzie <acm <at> muc.de>:
You have taken responsibility. (Wed, 12 Jul 2017 20:27:01 GMT) Full text and rfc822 format available.

Notification sent to Mohammed Sadiq <sadiq <at> sadiqpk.org>:
bug acknowledged by developer. (Wed, 12 Jul 2017 20:27:02 GMT) Full text and rfc822 format available.

Message #22 received at 25623-done <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Mohammed Sadiq <sadiq <at> sadiqpk.org>
Cc: 25623-done <at> debbugs.gnu.org
Subject: Re: bug#25623: CC Mode 5.32.99 (C/l); `c-defun-name' returns wrong
 result for filescope enums, structs and unions
Date: Wed, 12 Jul 2017 20:24:53 +0000
Hello, Mohammed.

On Mon, Apr 24, 2017 at 09:14:27 +0530, Mohammed Sadiq wrote:

> > On April 24, 2017 at 2:31 AM Alan Mackenzie <acm <at> muc.de> wrote:

> > > enum
> > >  {
> > >  A,
> > >  B,|
> > >  C,
> > >  };

> > > gives 'nil', which would better return 'enum' as it is common to have enums without
> > > names associated.

> > I've thought about this. The problem is that "enum" is not a name - it
> > is a keyword.

> Yeah, right, but as "enum" and "struct" can't be used function names, users can use
> c-defun-name to check if the point is in struct or enum too.

I put on my (hopefully benevolent) dictator's hat and decided to stick
with my reasoning, here.

> > > int a[] = {'h', 'e', 'l', 'l', 'o', '\0'|};

> > > gives "[" which I believe, should actually return 'nil' when defined at file scope.

> > It should indeed return nil. Please see the amended patch (below).

> Your patch fixed the issue.

Thanks.  I've committed the patch and closed the bug.

> > > typedef struct
> > > {
> > >  int a;
> > >  int b;|
> > > } nice;

> > > returns "nice". It may be better to return "struct nice" as in "struct nice {...};" definition.

> > I considered that, too. Even returning "nice" is a bit bogus, here. For
> > example, if we had "typedef struct { ... } nice, easy;" why should we
> > return "nice" (which we do) rather than "easy"? I think here, strictly
> > speaking, we should return nil, since "nice" is an identifier _after_ the
> > main type definition, not part of it. But, then again, maybe here is not
> > the place to be strict. ;-)


> If in both cases, returning simply "struct" would be more helpful to the users
> than nil, imho. the same for enum

I'm afraid I used my dictator's hat here, too.

Anyway, the bug is, at long last, closed.

> Thanks

-- 
Alan Mackenzie (Nuremberg, Germany).




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 10 Aug 2017 11:24:03 GMT) Full text and rfc822 format available.

This bug report was last modified 8 years and 30 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.