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

[Mohammed Sadiq]

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 "[ ]*\\(//+\\|\\**\\)[ ]*\\([ ]*\\([-–!|#%;>*·•‣⁃◦]+[ ]*\\)*\\)"
)

[Alan Mackenzie]

Hello, Mohammed.

Post by Mohammed Sadiq
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

Post by Mohammed Sadiq
Thanks

[ .... ]

[Alan Mackenzie]

Hello, Mohammed.

Thanks for the fast reply.

Hi,

Post by Alan Mackenzie
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

[Mohammed Sadiq]

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).

[Alan Mackenzie]

Hello, Mohammed.

Post by Mohammed Sadiq

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.

Post by Mohammed Sadiq

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.

Post by Mohammed Sadiq

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.

Post by Mohammed Sadiq
Thanks