bug#61356: 29.0.60; wrong-type-argument in treesit-indent-region when indenting a C buffer

bug#61356: 29.0.60; wrong-type-argument in treesit-indent-region when indenting a C buffer

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Steps to reproduce the problem:

emacs -Q
C-x C-f emacs/src/xdisp.c RET
M-x c-ts-mode RET
C-x h
TAB

Expected result:

The buffer is reindented.

Actual results:

The operation fails with this error:

Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
  treesit-indent-region(1 1214226)
  indent-region(1 1214226)
  indent-for-tab-command(nil)
  funcall-interactively(indent-for-tab-command nil)
  call-interactively(indent-for-tab-command nil nil)
  command-execute(indent-for-tab-command)

bug#61356: 29.0.60; wrong-type-argument in treesit-indent-region when indenting a C buffer

[Eli Zaretskii]

> Date: Tue, 07 Feb 2023 23:20:09 +0100
> From:  Daniel Martín via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> 
> Steps to reproduce the problem:
> 
> emacs -Q
> C-x C-f emacs/src/xdisp.c RET
> M-x c-ts-mode RET
> C-x h
> TAB
> 
> Expected result:
> 
> The buffer is reindented.
> 
> Actual results:
> 
> The operation fails with this error:
> 
> Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
>   treesit-indent-region(1 1214226)
>   indent-region(1 1214226)
>   indent-for-tab-command(nil)
>   funcall-interactively(indent-for-tab-command nil)
>   call-interactively(indent-for-tab-command nil nil)
>   command-execute(indent-for-tab-command)

Thank you for your report.

Yuan and Theo, could you please look into this?  I briefly stepped
through the code involved in the problem, and it sounds like it makes
some assumptions regarding the nodes which are not necessarily true in
Real Life.

bug#61356: 29.0.60; wrong-type-argument in treesit-indent-region when indenting a C buffer

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Tue, 07 Feb 2023 23:20:09 +0100
>> From:  Daniel Martín via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> 
>> Steps to reproduce the problem:
>> 
>> emacs -Q
>> C-x C-f emacs/src/xdisp.c RET
>> M-x c-ts-mode RET
>> C-x h
>> TAB
>> 
>> Expected result:
>> 
>> The buffer is reindented.
>> 
>> Actual results:
>> 
>> The operation fails with this error:
>> 
>> Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
>>   treesit-indent-region(1 1214226)
>>   indent-region(1 1214226)
>>   indent-for-tab-command(nil)
>>   funcall-interactively(indent-for-tab-command nil)
>>   call-interactively(indent-for-tab-command nil nil)
>>   command-execute(indent-for-tab-command)
>
> Thank you for your report.
>
> Yuan and Theo, could you please look into this?  I briefly stepped
> through the code involved in the problem, and it sounds like it makes
> some assumptions regarding the nodes which are not necessarily true in
> Real Life.

c-ts-mode from the tip of the emacs-29 branch can indent xdisp.c without
failures if I apply the patch from bug#61691 and the patch attached to
this email message.

It is very slow, though, compared to c-mode.  I have a very fast machine
and it took around 10 minutes to indent the whole file, while c-mode
indented the file in 30-40 seconds.  I guess the algorithm in
treesit-indent-region doesn't scale very well for very big files like
xdisp.c.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Check-the-anchor-along-with-the-offset-in-treesit-in.patch --]
[-- Type: text/x-patch, Size: 1022 bytes --]

From d082bfb92a17f3968910182009939300ed51a3ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Sat, 25 Feb 2023 03:07:55 +0100
Subject: [PATCH] Check the anchor along with the offset in
 treesit-indent-region

* lisp/treesit.el (treesit-indent-region): To compute a valid column,
having a non-nil offset is not enough, we need a non-nil anchor as
well.
---
 lisp/treesit.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/treesit.el b/lisp/treesit.el
index 045fdf21cba..1decfc3d7cf 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -1533,7 +1533,7 @@ treesit-indent-region
         (dotimes (jdx idx)
           (let ((anchor (aref meta-vec (* jdx meta-len)))
                 (offset (aref meta-vec (+ 1 (* jdx meta-len)))))
-            (when offset
+            (when (and anchor offset)
               (let ((col (save-excursion
                            (goto-char anchor)
                            (+ offset (current-column)))))
-- 
2.34.1

bug#61356: 29.0.60; wrong-type-argument in treesit-indent-region when indenting a C buffer

[Yuan Fu]

> On Feb 24, 2023, at 6:24 PM, Daniel Martín <mardani29@yahoo.es> wrote:
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
>>> Date: Tue, 07 Feb 2023 23:20:09 +0100
>>> From:  Daniel Martín via "Bug reports for GNU Emacs,
>>> the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>>> 
>>> 
>>> Steps to reproduce the problem:
>>> 
>>> emacs -Q
>>> C-x C-f emacs/src/xdisp.c RET
>>> M-x c-ts-mode RET
>>> C-x h
>>> TAB
>>> 
>>> Expected result:
>>> 
>>> The buffer is reindented.
>>> 
>>> Actual results:
>>> 
>>> The operation fails with this error:
>>> 
>>> Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
>>>  treesit-indent-region(1 1214226)
>>>  indent-region(1 1214226)
>>>  indent-for-tab-command(nil)
>>>  funcall-interactively(indent-for-tab-command nil)
>>>  call-interactively(indent-for-tab-command nil nil)
>>>  command-execute(indent-for-tab-command)
>> 
>> Thank you for your report.
>> 
>> Yuan and Theo, could you please look into this?  I briefly stepped
>> through the code involved in the problem, and it sounds like it makes
>> some assumptions regarding the nodes which are not necessarily true in
>> Real Life.
> 
> c-ts-mode from the tip of the emacs-29 branch can indent xdisp.c without
> failures if I apply the patch from bug#61691 and the patch attached to
> this email message.
> 
> It is very slow, though, compared to c-mode.  I have a very fast machine
> and it took around 10 minutes to indent the whole file, while c-mode
> indented the file in 30-40 seconds.  I guess the algorithm in
> treesit-indent-region doesn't scale very well for very big files like
> xdisp.c.
> 

Thanks, I applied the patch. What you observed is certainly a regression due to, from what I can see from profiling, c-ts-common-statement-offset. It is added for more accurate indenting, by going up the parse tree and “count brackets”, more or less. This is probably a pretty inefficient when calculating indentation for every line. A change I just pushed helps a bit (in a block, make the first child calculate indentation as before, and children after than uses the previous sibling’s indentation), but still not enough.

I have some ideas to improve this, and get back once I have something.

Yuan

bug#61356: 29.0.60; wrong-type-argument in treesit-indent-region when indenting a C buffer

[stefankangas]

Yuan Fu <casouri@gmail.com> writes:

> Thanks, I applied the patch. What you observed is certainly a regression due to,
> from what I can see from profiling, c-ts-common-statement-offset. It is added
> for more accurate indenting, by going up the parse tree and “count brackets”,
> more or less. This is probably a pretty inefficient when calculating indentation
> for every line. A change I just pushed helps a bit (in a block, make the first
> child calculate indentation as before, and children after than uses the previous
> sibling’s indentation), but still not enough.
>
> I have some ideas to improve this, and get back once I have something.

It seems like the patch here was applied.  Can this bug therefore be
closed, or is there more to do here?