bug#59630: 29.0.50; treesitter-buffer-root-node doesn't change when changing buffer restriction

bug#59630: 29.0.50; treesitter-buffer-root-node doesn't change when changing buffer restriction

[miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Treesitter doesn't seem to change its parse tree when changing
restriction. It gets it correct the first time when we query the root
node, but then, after changing restriction, it doesn't seem to get
updated.

The following is an M-x ielm session to demonstrate the problem

  ELISP> (set-buffer (get-buffer-create "test"))
  ELISP> (insert "echo '123'")
  ELISP> (narrow-to-region 1 4)
  ELISP> (buffer-string)
  "echo"
  ELISP> (treesit-buffer-root-node 'bash)
  #<treesit-node
  (program)
  in 1-4>  ;; This is expected

  ELISP> (widen)
  ELISP> (treesit-buffer-root-node 'bash)
  #<treesit-node
  (program)
  in 1-4>  ;; <---- This is not expected, the root node should span 1-9

  ELISP> (buffer-string)
  "echo '123'"

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

bug#59630: 29.0.50; treesitter-buffer-root-node doesn't change when changing buffer restriction

[Yuan Fu]

> On Nov 27, 2022, at 4:49 AM, miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors <bug-gnu-emacs@gnu.org> wrote:
> 
> Treesitter doesn't seem to change its parse tree when changing
> restriction. It gets it correct the first time when we query the root
> node, but then, after changing restriction, it doesn't seem to get
> updated.
> 
> The following is an M-x ielm session to demonstrate the problem
> 
>  ELISP> (set-buffer (get-buffer-create "test"))
>  ELISP> (insert "echo '123'")
>  ELISP> (narrow-to-region 1 4)
>  ELISP> (buffer-string)
>  "echo"
>  ELISP> (treesit-buffer-root-node 'bash)
>  #<treesit-node
>  (program)
>  in 1-4>  ;; This is expected
> 
>  ELISP> (widen)
>  ELISP> (treesit-buffer-root-node 'bash)
>  #<treesit-node
>  (program)
>  in 1-4>  ;; <---- This is not expected, the root node should span 1-9
> 
>  ELISP> (buffer-string)
>  "echo '123'"

Thanks. We didn’t edit the buffer after widening, so tree-sitter didn’t reparse and used the old tree, which sees the narrowed buffer. Eli, what would be a good and reliable way to know that narrowing has changed? I see current_buffer->clip_changed set to 1 in narrow-to-region and widen, but when are they set to 0?

Yuan

bug#59630: 29.0.50; treesitter-buffer-root-node doesn't change when changing buffer restriction

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sun, 27 Nov 2022 14:40:41 -0800
> Cc: 59630@debbugs.gnu.org,
>  Eli Zaretskii <eliz@gnu.org>
> 
> >  ELISP> (widen)
> >  ELISP> (treesit-buffer-root-node 'bash)
> >  #<treesit-node
> >  (program)
> >  in 1-4>  ;; <---- This is not expected, the root node should span 1-9
> > 
> >  ELISP> (buffer-string)
> >  "echo '123'"
> 
> Thanks. We didn’t edit the buffer after widening, so tree-sitter didn’t reparse and used the old tree, which sees the narrowed buffer. Eli, what would be a good and reliable way to know that narrowing has changed? I see current_buffer->clip_changed set to 1 in narrow-to-region and widen, but when are they set to 0?

Not sure what exactly are you after.  If you want to catch the moment when
we change the buffer restriction, you will have to add something to
Fnarrow_to_region and Fwiden.  However, why does tree-sitter need to know
about changes in the narrowing, unless it is asked to update something or
produce a tree?  I thought we decided to update this stuff lazily, only when
actually needed?  Being sensitive to these changes would require you to have
some logic, because a buffer can be narrowed and widened several times in a
sequence without any consequences for tree-sitter, and asking the parser to
update itself will just burn CPU cycles.  So if this is really needed, let's
discuss for which purposes and under what conditions.

I actually don't think why we should be worried by the above scenario; can
you explain?

To answer your question: the clip_changed flag is reset in
reconsider_clip_changes, which is called at the beginning of redisplay, and
when a window's redisplay is successfully completed (see
mark_window_display_accurate_1).  However, this flag is not meaningful
outside of redisplay, in particular when redisplay cycle is NOT running.  So
I'm not sure it is what you want.

bug#59630: 29.0.50; treesitter-buffer-root-node doesn't change when changing buffer restriction

[Yuan Fu]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Yuan Fu <casouri@gmail.com>
>> Date: Sun, 27 Nov 2022 14:40:41 -0800
>> Cc: 59630@debbugs.gnu.org,
>>  Eli Zaretskii <eliz@gnu.org>
>> 
>> >  ELISP> (widen)
>> >  ELISP> (treesit-buffer-root-node 'bash)
>> >  #<treesit-node
>> >  (program)
>> >  in 1-4>  ;; <---- This is not expected, the root node should span 1-9
>> > 
>> >  ELISP> (buffer-string)
>> >  "echo '123'"
>> 
>> Thanks. We didn’t edit the buffer after widening, so tree-sitter
>> didn’t reparse and used the old tree, which sees the narrowed
>> buffer. Eli, what would be a good and reliable way to know that
>> narrowing has changed? I see current_buffer->clip_changed set to 1
>> in narrow-to-region and widen, but when are they set to 0?
>
> Not sure what exactly are you after.  If you want to catch the moment when
> we change the buffer restriction, you will have to add something to
> Fnarrow_to_region and Fwiden.  However, why does tree-sitter need to know
> about changes in the narrowing, unless it is asked to update something or
> produce a tree?  I thought we decided to update this stuff lazily, only when
> actually needed?  Being sensitive to these changes would require you to have
> some logic, because a buffer can be narrowed and widened several times in a
> sequence without any consequences for tree-sitter, and asking the parser to
> update itself will just burn CPU cycles.  So if this is really needed, let's
> discuss for which purposes and under what conditions.
>
> I actually don't think why we should be worried by the above scenario; can
> you explain?
>

We still parse lazily, and narrow/widen wouldn’t affect the parse tree,
until user requests for a node when the restriction is different from
last time we parsed the buffer.  Basically:

request-node <-- last time we parsed
narrow
widen
narrow
widen
request-node <-- no need to reparse (1)

request-node <-- last time we parsed
edits-buffer
request-node <-- need to reparse (2)

request-node <-- last time we parsed
narrow
request-node <-- need to reparse (3)

Right now in case (3) we don’t reparse the buffer. I have a reasonable
fix in f794263da20.

> To answer your question: the clip_changed flag is reset in
> reconsider_clip_changes, which is called at the beginning of redisplay, and
> when a window's redisplay is successfully completed (see
> mark_window_display_accurate_1).  However, this flag is not meaningful
> outside of redisplay, in particular when redisplay cycle is NOT running.  So
> I'm not sure it is what you want.

Thanks. Yeah that’s not something we want to use.

Yuan

bug#59630: 29.0.50; treesitter-buffer-root-node doesn't change when changing buffer restriction

[Yuan Fu]

Yuan Fu <casouri@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Yuan Fu <casouri@gmail.com>
>>> Date: Sun, 27 Nov 2022 14:40:41 -0800
>>> Cc: 59630@debbugs.gnu.org,
>>>  Eli Zaretskii <eliz@gnu.org>
>>> 
>>> >  ELISP> (widen)
>>> >  ELISP> (treesit-buffer-root-node 'bash)
>>> >  #<treesit-node
>>> >  (program)
>>> >  in 1-4>  ;; <---- This is not expected, the root node should span 1-9
>>> > 
>>> >  ELISP> (buffer-string)
>>> >  "echo '123'"
>>> 
>>> Thanks. We didn’t edit the buffer after widening, so tree-sitter
>>> didn’t reparse and used the old tree, which sees the narrowed
>>> buffer. Eli, what would be a good and reliable way to know that
>>> narrowing has changed? I see current_buffer->clip_changed set to 1
>>> in narrow-to-region and widen, but when are they set to 0?
>>
>> Not sure what exactly are you after.  If you want to catch the moment when
>> we change the buffer restriction, you will have to add something to
>> Fnarrow_to_region and Fwiden.  However, why does tree-sitter need to know
>> about changes in the narrowing, unless it is asked to update something or
>> produce a tree?  I thought we decided to update this stuff lazily, only when
>> actually needed?  Being sensitive to these changes would require you to have
>> some logic, because a buffer can be narrowed and widened several times in a
>> sequence without any consequences for tree-sitter, and asking the parser to
>> update itself will just burn CPU cycles.  So if this is really needed, let's
>> discuss for which purposes and under what conditions.
>>
>> I actually don't think why we should be worried by the above scenario; can
>> you explain?
>>
>
> We still parse lazily, and narrow/widen wouldn’t affect the parse tree,
> until user requests for a node when the restriction is different from
> last time we parsed the buffer.  Basically:
>
> request-node <-- last time we parsed
> narrow
> widen
> narrow
> widen
> request-node <-- no need to reparse (1)
>
> request-node <-- last time we parsed
> edits-buffer
> request-node <-- need to reparse (2)
>
> request-node <-- last time we parsed
> narrow
> request-node <-- need to reparse (3)
>
> Right now in case (3) we don’t reparse the buffer. I have a reasonable
> fix in f794263da20.

Closing since I believe this is fixed.

Yuan