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: 1639 bytes --]



> On Aug 14, 2023, at 4:59 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Sun, 13 Aug 2023 22:20:56 -0700
>> Cc: 64442@debbugs.gnu.org
>> 
>>> I like this much better than what we have now, thanks.  But I have a
>>> question: can we perhaps recognize the "function" of the body as such,
>>> and then automatically move to the previous defun, which is the right
>>> place?  The "defun" that is the body has no name, so maybe that could
>>> be used as a sign?  
>> 
>> We can easily tell the body from the declaration, but we can’t easily tell whether we should automatically move forward or backward. When point arrives at the point between the declaration and the body, should it move to the beginning of the next defun or the beginning of the declaration? This, plus it’s not straightforward to know whether we are in between a body and a declaration. I really don’t want to add even more cursed hacks into c-ts-mode.el :-)
> 
> Too bad, but okay.
> 
>>> That would allow "C-x 4 a" to work inside a DEFUN,
>>> something that still works less reliably with this patch: you must be
>>> in the "first defun" to get it to find the name of the function.
>> 
>> C-x 4 a should’ve been fixed already. And it shouldn’t rely on this fix to work. Do you have a recipe for when it doesn’t work?
> 
> Just try it with your patch.  If point is inside the body, the
> function's name is not captured by "C-x 4 a”.

My bad, I must’ve been trying C-x 4 a in a different Emacs session, which worked. Anyway, I updated the patch and C-x 4 a should now work.

Yuan


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

From 779092b28e472d17adfff92c1b9d7403aad7a106 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.
(c-ts-mode--emacs-defun-at-point): Now that we recognize both parts of
a DEFUN as defun, c-ts-mode--emacs-defun-at-point needs to be updated
to adapt to it.
---
 lisp/progmodes/c-ts-mode.el | 115 +++++++++++++++++++-----------------
 1 file changed, 60 insertions(+), 55 deletions(-)

diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 98797bf3ce7..34a89b4dcad 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -880,29 +880,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.
@@ -955,6 +962,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.
 
@@ -969,31 +981,18 @@ c-ts-mode--emacs-defun-at-point
 If RANGE is non-nil, return (BEG . END) where BEG end END
 encloses the whole defun.  This is for when the entire defun
 is required, not just the declaration part for DEFUN."
-  (or (when-let ((node (treesit-defun-at-point)))
-        (if range
-            (cons (treesit-node-start node)
-                (treesit-node-end node))
-            node))
-      (and c-ts-mode-emacs-sources-support
-           (let ((candidate-1 ; For when point is in the DEFUN statement.
-                  (treesit-node-prev-sibling
-                   (treesit-node-top-level
-                    (treesit-node-at (point))
-                    "compound_statement")))
-                 (candidate-2 ; For when point is in the body.
-                  (treesit-node-top-level
-                   (treesit-node-at (point))
-                   "expression_statement")))
-             (when-let
-                 ((node (or (and (c-ts-mode--emacs-defun-p candidate-1)
-                                 candidate-1)
-                            (and (c-ts-mode--emacs-defun-p candidate-2)
-                                 candidate-2))))
-               (if range
-                   (cons (treesit-node-start node)
-                       (treesit-node-end
-                        (treesit-node-next-sibling node)))
-                   node))))))
+  (when-let* ((node (treesit-defun-at-point))
+              (defun-range (cons (treesit-node-start node)
+                                 (treesit-node-end node))))
+    ;; Make some adjustment for DEFUN.
+    (when c-ts-mode-emacs-sources-support
+      (cond ((c-ts-mode--emacs-defun-body-p node)
+             (setq node (treesit-node-prev-sibling node))
+             (setcar defun-range (treesit-node-start node)))
+            ((c-ts-mode--emacs-defun-p node)
+             (setcdr defun-range (treesit-node-end
+                                  (treesit-node-next-sibling node))))))
+    (if range defun-range node)))
 
 (defun c-ts-mode-indent-defun ()
   "Indent the current top-level declaration syntactically.
@@ -1111,13 +1110,19 @@ c-ts-base-mode
 
   ;; Navigation.
   (setq-local treesit-defun-type-regexp
-              (cons (regexp-opt '("function_definition"
-                                  "type_definition"
-                                  "struct_specifier"
-                                  "enum_specifier"
-                                  "union_specifier"
-                                  "class_specifier"
-                                  "namespace_definition"))
+              (cons (regexp-opt (append
+                                 '("function_definition"
+                                   "type_definition"
+                                   "struct_specifier"
+                                   "enum_specifier"
+                                   "union_specifier"
+                                   "class_specifier"
+                                   "namespace_definition")
+                                 (and c-ts-mode-emacs-sources-support
+                                      '(;; 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


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