bug#64442: 29.0.92; treesit-beginning-of-defun fails in DEFUN functions in C

[Yuan Fu <casouri@gmail.com>]

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



> On Aug 10, 2023, at 2:18 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> Cc: 64442@debbugs.gnu.org
>> Date: Sun, 30 Jul 2023 10:10:35 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> 
>>> From: Yuan Fu <casouri@gmail.com>
>>> Date: Tue, 11 Jul 2023 19:10:01 -0700
>>> Cc: 64442@debbugs.gnu.org
>>> 
>>> 
>>> 
>>>> On Jul 6, 2023, at 11:40 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>>> 
>>>>> From: Yuan Fu <casouri@gmail.com>
>>>>> Date: Thu, 6 Jul 2023 23:15:00 -0700
>>>>> Cc: 64442@debbugs.gnu.org
>>>>> 
>>>>>> On Jul 4, 2023, at 4:39 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>>>>> 
>>>>>> Why cannot we look for a top-level expression_statement node which is
>>>>>> a call_expression whose function identifier is "DEFUN" and whose
>>>>>> position is between the place where C-M-a was invoked and the place
>>>>>> where it does find a defun?
>>>>> 
>>>>> It’s gonna be ugly, but I can take a jab at it this weekend. I’m thinking of a wrapper function that tries to detect DEFUN before falling back to the ordinary tree-sitter defun movement function.
>>>> 
>>>> Thanks.  let's see how ugly it is before deciding whether it's worth it.
>>>> 
>>>>>> DEFUN's cannot be nested, so we don't need to consider that.
>>>>> 
>>>>> Yeah, in general C sources don’t have nested defuns, only C++ ones do.
>>>> 
>>>> No, I meant the use of DEFUN macros in Emacs cannot be nested.
>>> 
>>> Just an update. I didn’t forget about this, but it’s more harder than I thought and I’m still working on it :-(
>> 
>> Any progress with this?  It would be good to have a solution in Emacs
>> 29.2, if possible.
> 
> Ping!

I still don’t have a good solution. But I just realized that we might be able to make a little compromise: what if Emacs recognizes DEFUN, but as two separate parts (the declaration and the body), rather than one? It’s hard to make it recognize DEFUN as a single defun, but making it recognize DEFUN as two parts is easy.

Try this patch and see if you like the behavior. Personally I find it quite alright.

Yuan


[-- Attachment #2: defun-nav.patch --]
[-- Type: application/octet-stream, Size: 5483 bytes --]

From b0e361b430650e95449c59b8643be58d598cde58 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Thu, 10 Aug 2023 14:27:29 -0700
Subject: [PATCH] Support defun navigation for DEFUN in c-ts-mode (bug#64442)

Before this change, beginning/end-of-defun just ignores DEFUN in
c-ts-mode. After this change, beginning/end-of-defun can recognize
DEFUN, but a DEFUN definition is considered two defuns. Eg,
beginning/end-of-defun will stop at (1) (2) and (3) in the following
snippet:

(1)DEFUN ("treesit-node-parser",
       Ftreesit_node_parser, Streesit_node_parser,
       1, 1, 0,
       doc: /* Return the parser to which NODE belongs.  */)
  (Lisp_Object node)
(2){
  CHECK_TS_NODE (node);
  return XTS_NODE (node)->parser;
}
(3)

Ideally we want point to only stop at (1) and (3), but that'll be a
lot harder to do.

* lisp/progmodes/c-ts-mode.el:
(c-ts-mode--defun-valid-p): Refactor to take in account of DEFUN body.
(c-ts-mode--emacs-defun-body-p): New function.
(c-ts-base-mode): Add DEFUN and DEFUN body to recognized types.
---
 lisp/progmodes/c-ts-mode.el | 64 +++++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 24 deletions(-)

diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 7f4f6f11387..0eb1f2a17a7 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -882,29 +882,36 @@ c-ts-mode--defun-name
 (defun c-ts-mode--defun-valid-p (node)
   "Return non-nil if NODE is a valid defun node.
 Ie, NODE is not nested."
-  (or (c-ts-mode--emacs-defun-p node)
-      (not (or (and (member (treesit-node-type node)
-                            '("struct_specifier"
-                              "enum_specifier"
-                              "union_specifier"
-                              "declaration"))
-                    ;; If NODE's type is one of the above, make sure it is
-                    ;; top-level.
-                    (treesit-node-top-level
-                     node (rx (or "function_definition"
-                                  "type_definition"
-                                  "struct_specifier"
-                                  "enum_specifier"
-                                  "union_specifier"
-                                  "declaration"))))
-
-               (and (equal (treesit-node-type node) "declaration")
-                    ;; If NODE is a declaration, make sure it is not a
-                    ;; function declaration.
-                    (equal (treesit-node-type
-                            (treesit-node-child-by-field-name
-                             node "declarator"))
-                           "function_declarator"))))))
+  (let ((top-level-p (lambda (node)
+                       (not (treesit-node-top-level
+                             node (rx (or "function_definition"
+                                          "type_definition"
+                                          "struct_specifier"
+                                          "enum_specifier"
+                                          "union_specifier"
+                                          "declaration")))))))
+    (pcase (treesit-node-type node)
+      ;; The declaration part of a DEFUN.
+      ("expression_statement" (c-ts-mode--emacs-defun-p node))
+      ;; The body of a DEFUN.
+      ("compound_statement" (c-ts-mode--emacs-defun-body-p node))
+      ;; If NODE's type is one of these three, make sure it is
+      ;; top-level.
+      ((or "struct_specifier"
+           "enum_specifier"
+           "union_specifier")
+       (funcall top-level-p node))
+      ;; If NODE is a declaration, make sure it's not a function
+      ;; declaration (we only want function_definition) and is a
+      ;; top-level declaration.
+      ("declaration"
+       (and (not (equal (treesit-node-type
+                         (treesit-node-child-by-field-name
+                          node "declarator"))
+                        "function_declarator"))
+            (funcall top-level-p node)))
+      ;; Other types don't need further verification.
+      (_ t))))
 
 (defun c-ts-mode--defun-for-class-in-imenu-p (node)
   "Check if NODE is a valid entry for the Class subindex.
@@ -957,6 +964,11 @@ c-ts-mode--emacs-defun-p
                t)
               "DEFUN")))
 
+(defun c-ts-mode--emacs-defun-body-p (node)
+  "Return non-nil if NODE is the function body of a DEFUN."
+  (and (equal (treesit-node-type node) "compound_statement")
+       (c-ts-mode--emacs-defun-p (treesit-node-prev-sibling node))))
+
 (defun c-ts-mode--emacs-defun-at-point (&optional range)
   "Return the defun node at point.
 
@@ -1119,7 +1131,11 @@ c-ts-base-mode
                                   "enum_specifier"
                                   "union_specifier"
                                   "class_specifier"
-                                  "namespace_definition"))
+                                  "namespace_definition"
+                                  ;; DEFUN.
+                                  "expression_statement"
+                                  ;; DEFUN body.
+                                  "compound_statement"))
                     #'c-ts-mode--defun-valid-p))
   (setq-local treesit-defun-skipper #'c-ts-mode--defun-skipper)
   (setq-local treesit-defun-name-function #'c-ts-mode--defun-name)
-- 
2.41.0