GNU bug report logs - #61235
30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

Previous Next

Package: emacs;

Reported by: Mickey Petersen <mickey <at> masteringemacs.org>

Date: Thu, 2 Feb 2023 19:47:02 UTC

Severity: normal

Found in version 30.0.50

Fixed in version 29.1

Done: Yuan Fu <casouri <at> gmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 61235 in the body.
You can then email your comments to 61235 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Thu, 02 Feb 2023 19:47:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Mickey Petersen <mickey <at> masteringemacs.org>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 02 Feb 2023 19:47:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mickey Petersen <mickey <at> masteringemacs.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a
 node belongs to a deleted parser
Date: Thu, 02 Feb 2023 19:46:41 +0000
There's no way to tell if a node belongs to a now-deleted
parser. Checking if it is `missing' or `outdated' returns nil; there
is no way to ascertain this state except by catching an error if you
try to get its node text, type, etc.




In GNU Emacs 30.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
 3.24.20, cairo version 1.16.0) of 2023-01-25 built on mickey-work
Repository revision: 8b87d095acfb23b527f955873a59dd9c13ffc9b4
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: Ubuntu 20.04.3 LTS

Configured using:
 'configure --with-native-compilation --with-json --with-mailutils
 --without-compress-install --with-imagemagick CC=gcc-10'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ
IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2
M17N_FLT MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP
SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER X11 XDBE
XIM XINPUT2 XPM GTK3 ZLIB




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Mon, 06 Feb 2023 04:25:01 GMT) Full text and rfc822 format available.

Message #8 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Mickey Petersen <mickey <at> masteringemacs.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 61235 <at> debbugs.gnu.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way 
 to tell if a node belongs to a deleted parser
Date: Sun, 5 Feb 2023 20:24:27 -0800
[Message part 1 (text/plain, inline)]
Mickey Petersen <mickey <at> masteringemacs.org> writes:

> There's no way to tell if a node belongs to a now-deleted
> parser. Checking if it is `missing' or `outdated' returns nil; there
> is no way to ascertain this state except by catching an error if you
> try to get its node text, type, etc.

That sounds reasonable. I can add treesit-parser-live-p, and add
a "live" PROPERTY to treesit-node-check.

treesit-parser-live-p only returns t when the parser is not deleted AND
its buffer is live. I assume that’s more useful than just checking
whether the parser is deleted, for whatever usecase you have?

Eli, should we add this in the release branch or Emacs 30? This
functionality is possible to implement in Elisp, albeit a bit clunky. I
attached a patch of how would it look like (without documentation/test).

Yuan

[livep.patch (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Mon, 06 Feb 2023 12:35:02 GMT) Full text and rfc822 format available.

Message #11 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 61235 <at> debbugs.gnu.org, mickey <at> masteringemacs.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way 
 to tell if a node belongs to a deleted parser
Date: Mon, 06 Feb 2023 14:34:44 +0200
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Sun, 5 Feb 2023 20:24:27 -0800
> Cc: 61235 <at> debbugs.gnu.org,
>  Eli Zaretskii <eliz <at> gnu.org>
> 
> Mickey Petersen <mickey <at> masteringemacs.org> writes:
> 
> > There's no way to tell if a node belongs to a now-deleted
> > parser. Checking if it is `missing' or `outdated' returns nil; there
> > is no way to ascertain this state except by catching an error if you
> > try to get its node text, type, etc.
> 
> That sounds reasonable. I can add treesit-parser-live-p, and add
> a "live" PROPERTY to treesit-node-check.

I'm not sure I understand the need.  AFAIU, a parser is deleted only
if we call treesit-parser-delete; are we saying that a Lisp program
doesn't know that it deleted a parser?  What exactly is the practical
situation where this problem happens, and why?

Frankly, I don't think we should at this stage add APIs without a very
good reason.  We should instead collect experience, both from users
and from Lisp programs, and analyze them before deciding whether more
APIs are necessary.

> +DEFUN ("treesit-parser-live-p",
> +       Ftreesit_parser_live_p, Streesit_parser_live_p, 1, 1, 0,
> +       doc: /* Check whether PARSER is not deleted and its buffer is live.  */)
> +  (Lisp_Object parser)
> +{
> +  treesit_check_parser (parser);
> +  if (XTS_PARSER (parser)->deleted)
> +    return Qnil;
> +  else if (NILP (Fbuffer_live_p (XTS_PARSER (parser)->buffer)))
> +    return Qnil;
> +  else
> +    return Qt;
> +}

Doesn't treesit_check_parser signal an error if the parser was
deleted?  If so, how will the above be useful, if someone wants to
avoid errors?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Mon, 06 Feb 2023 12:39:02 GMT) Full text and rfc822 format available.

Message #14 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Mickey Petersen <mickey <at> masteringemacs.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Yuan Fu <casouri <at> gmail.com>, 61235 <at> debbugs.gnu.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way
 to tell if a node belongs to a deleted parser
Date: Mon, 06 Feb 2023 12:35:20 +0000
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Yuan Fu <casouri <at> gmail.com>
>> Date: Sun, 5 Feb 2023 20:24:27 -0800
>> Cc: 61235 <at> debbugs.gnu.org,
>>  Eli Zaretskii <eliz <at> gnu.org>
>>
>> Mickey Petersen <mickey <at> masteringemacs.org> writes:
>>
>> > There's no way to tell if a node belongs to a now-deleted
>> > parser. Checking if it is `missing' or `outdated' returns nil; there
>> > is no way to ascertain this state except by catching an error if you
>> > try to get its node text, type, etc.
>>
>> That sounds reasonable. I can add treesit-parser-live-p, and add
>> a "live" PROPERTY to treesit-node-check.
>
> I'm not sure I understand the need.  AFAIU, a parser is deleted only
> if we call treesit-parser-delete; are we saying that a Lisp program
> doesn't know that it deleted a parser?  What exactly is the practical
> situation where this problem happens, and why?
>
> Frankly, I don't think we should at this stage add APIs without a very
> good reason.  We should instead collect experience, both from users
> and from Lisp programs, and analyze them before deciding whether more
> APIs are necessary.
>

Because node references are retained even after a parser is deleted.

Retrieving a node; somehow deleting the parser (maybe you closed the
buffer, or you were doing some off-hand parsing); and then doing
_anything_ with the aforementioned node yields an error for which
there is no way to test for.

This is particularly the case when you mix and match parsers in the
same buffer.

>> +DEFUN ("treesit-parser-live-p",
>> +       Ftreesit_parser_live_p, Streesit_parser_live_p, 1, 1, 0,
>> +       doc: /* Check whether PARSER is not deleted and its buffer is live.  */)
>> +  (Lisp_Object parser)
>> +{
>> +  treesit_check_parser (parser);
>> +  if (XTS_PARSER (parser)->deleted)
>> +    return Qnil;
>> +  else if (NILP (Fbuffer_live_p (XTS_PARSER (parser)->buffer)))
>> +    return Qnil;
>> +  else
>> +    return Qt;
>> +}
>
> Doesn't treesit_check_parser signal an error if the parser was
> deleted?  If so, how will the above be useful, if someone wants to
> avoid errors?





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Mon, 06 Feb 2023 13:20:02 GMT) Full text and rfc822 format available.

Message #17 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mickey Petersen <mickey <at> masteringemacs.org>
Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way
 to tell if a node belongs to a deleted parser
Date: Mon, 06 Feb 2023 15:19:23 +0200
> From: Mickey Petersen <mickey <at> masteringemacs.org>
> Cc: Yuan Fu <casouri <at> gmail.com>, 61235 <at> debbugs.gnu.org
> Date: Mon, 06 Feb 2023 12:35:20 +0000
> 
> > I'm not sure I understand the need.  AFAIU, a parser is deleted only
> > if we call treesit-parser-delete; are we saying that a Lisp program
> > doesn't know that it deleted a parser?  What exactly is the practical
> > situation where this problem happens, and why?
> >
> > Frankly, I don't think we should at this stage add APIs without a very
> > good reason.  We should instead collect experience, both from users
> > and from Lisp programs, and analyze them before deciding whether more
> > APIs are necessary.
> >
> 
> Because node references are retained even after a parser is deleted.
> 
> Retrieving a node; somehow deleting the parser (maybe you closed the
> buffer, or you were doing some off-hand parsing); and then doing
> _anything_ with the aforementioned node yields an error for which
> there is no way to test for.
> 
> This is particularly the case when you mix and match parsers in the
> same buffer.

I'm asking why the Lisp program cannot track the parsers its uses and
deletes, and instead expects the core to do the janitor's job for it.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Mon, 06 Feb 2023 13:29:02 GMT) Full text and rfc822 format available.

Message #20 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Mickey Petersen <mickey <at> masteringemacs.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way
 to tell if a node belongs to a deleted parser
Date: Mon, 06 Feb 2023 13:19:57 +0000
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Mickey Petersen <mickey <at> masteringemacs.org>
>> Cc: Yuan Fu <casouri <at> gmail.com>, 61235 <at> debbugs.gnu.org
>> Date: Mon, 06 Feb 2023 12:35:20 +0000
>>
>> > I'm not sure I understand the need.  AFAIU, a parser is deleted only
>> > if we call treesit-parser-delete; are we saying that a Lisp program
>> > doesn't know that it deleted a parser?  What exactly is the practical
>> > situation where this problem happens, and why?
>> >
>> > Frankly, I don't think we should at this stage add APIs without a very
>> > good reason.  We should instead collect experience, both from users
>> > and from Lisp programs, and analyze them before deciding whether more
>> > APIs are necessary.
>> >
>>
>> Because node references are retained even after a parser is deleted.
>>
>> Retrieving a node; somehow deleting the parser (maybe you closed the
>> buffer, or you were doing some off-hand parsing); and then doing
>> _anything_ with the aforementioned node yields an error for which
>> there is no way to test for.
>>
>> This is particularly the case when you mix and match parsers in the
>> same buffer.
>
> I'm asking why the Lisp program cannot track the parsers its uses and
> deletes, and instead expects the core to do the janitor's job for it.

Because I have a proxy-like object of a real node because they're
invalidated if a buffer is edited, even if the parcel of code I hold a
node reference to is untouched. That's just how tree-sitter works, so
I deal with it like this. That part works fine for I can of course use
`treesit-node-check' to determine if it's outdated and thus needs
refreshing (or not.)

The problems begin when the parser is also, for one reason or another,
destroyed.

Now I can no longer determine if it is safe to use the existing (real)
node reference. Any attempt to use a "dangling" node reference
that points to a dead parser throws a nasty error.

Thus, the extension to `treesit-node-check' that can tell if a node
belongs to a dead parser --- something that tree-sitter and the C core
clearly knows about internally.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Mon, 06 Feb 2023 14:06:01 GMT) Full text and rfc822 format available.

Message #23 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mickey Petersen <mickey <at> masteringemacs.org>
Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way
 to tell if a node belongs to a deleted parser
Date: Mon, 06 Feb 2023 16:05:30 +0200
> From: Mickey Petersen <mickey <at> masteringemacs.org>
> Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
> Date: Mon, 06 Feb 2023 13:19:57 +0000
> 
> > I'm asking why the Lisp program cannot track the parsers its uses and
> > deletes, and instead expects the core to do the janitor's job for it.
> 
> Because I have a proxy-like object of a real node because they're
> invalidated if a buffer is edited, even if the parcel of code I hold a
> node reference to is untouched. That's just how tree-sitter works, so
> I deal with it like this. That part works fine for I can of course use
> `treesit-node-check' to determine if it's outdated and thus needs
> refreshing (or not.)
> 
> The problems begin when the parser is also, for one reason or another,
> destroyed.

But it is only destroyed if your program calls treesit-parser-delete,
no?

Anyway, I'm okay with exposing treesit_check_parser to Lisp, if you
really insist.  But please be sure you want to insist, because I'm not
really convinced.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Mon, 06 Feb 2023 14:14:01 GMT) Full text and rfc822 format available.

Message #26 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Mickey Petersen <mickey <at> masteringemacs.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way
 to tell if a node belongs to a deleted parser
Date: Mon, 06 Feb 2023 14:08:46 +0000
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Mickey Petersen <mickey <at> masteringemacs.org>
>> Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
>> Date: Mon, 06 Feb 2023 13:19:57 +0000
>>
>> > I'm asking why the Lisp program cannot track the parsers its uses and
>> > deletes, and instead expects the core to do the janitor's job for it.
>>
>> Because I have a proxy-like object of a real node because they're
>> invalidated if a buffer is edited, even if the parcel of code I hold a
>> node reference to is untouched. That's just how tree-sitter works, so
>> I deal with it like this. That part works fine for I can of course use
>> `treesit-node-check' to determine if it's outdated and thus needs
>> refreshing (or not.)
>>
>> The problems begin when the parser is also, for one reason or another,
>> destroyed.
>
> But it is only destroyed if your program calls treesit-parser-delete,
> no?
>
> Anyway, I'm okay with exposing treesit_check_parser to Lisp, if you
> really insist.  But please be sure you want to insist, because I'm not
> really convinced.

All I want is a way for treesit-node-check to tell me if the node
belongs to a dead or alive parser. It is already capable of telling me
if a node is outdated, for instance, another rather important feature.
Knowing its status is pertinent if you do any sort of light
refactoring or if you end up destroying a block of code that has its
own nested parser.

Whether I destroy a parser explicitly (which is how I found the issue)
or indirectly (through some other mechanism) is, I think, orthogonal
to the problem of determining liveness (of a node, of a process, or
any of the `xxxxxx-live-p' functions we presently have)

Thanks,

Mickey




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Mon, 06 Feb 2023 15:22:01 GMT) Full text and rfc822 format available.

Message #29 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mickey Petersen <mickey <at> masteringemacs.org>
Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way
 to tell if a node belongs to a deleted parser
Date: Mon, 06 Feb 2023 17:21:18 +0200
> From: Mickey Petersen <mickey <at> masteringemacs.org>
> Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
> Date: Mon, 06 Feb 2023 14:08:46 +0000
> 
> All I want is a way for treesit-node-check to tell me if the node
> belongs to a dead or alive parser.

That'd be fine by me, but the patch posted by Yuan was a different
one.

Yuan, any reason not to extend treesit-node-check instead?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Tue, 07 Feb 2023 03:31:02 GMT) Full text and rfc822 format available.

Message #32 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 61235 <at> debbugs.gnu.org, Mickey Petersen <mickey <at> masteringemacs.org>
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to
 tell if a node belongs to a deleted parser
Date: Mon, 6 Feb 2023 19:00:30 -0800

> On Feb 6, 2023, at 7:21 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>> From: Mickey Petersen <mickey <at> masteringemacs.org>
>> Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
>> Date: Mon, 06 Feb 2023 14:08:46 +0000
>> 
>> All I want is a way for treesit-node-check to tell me if the node
>> belongs to a dead or alive parser.
> 
> That'd be fine by me, but the patch posted by Yuan was a different
> one.
> 
> Yuan, any reason not to extend treesit-node-check instead?

I did extend treesit-node-check in the patch. But I also added a function treesit-parser-live-p, which makes the same check but directly on a parser. It just made sense to me that if we let treesit-node-check check the nodes’ parser’s status, we’d also add a function to allow directly checking the status of a parser.

Micky, the function I added (and the extension to treesit-node-check) checks that the parser is not deleted AND its buffer is live. That makes the most sense to me, but would it cause any problem for your use case?

Yuan



Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Tue, 07 Feb 2023 03:32:01 GMT) Full text and rfc822 format available.

Message #35 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 61235 <at> debbugs.gnu.org, mickey <at> masteringemacs.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to
 tell if a node belongs to a deleted parser
Date: Tue, 07 Feb 2023 05:31:11 +0200
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Mon, 6 Feb 2023 19:00:30 -0800
> Cc: Mickey Petersen <mickey <at> masteringemacs.org>,
>  61235 <at> debbugs.gnu.org
> 
> > Yuan, any reason not to extend treesit-node-check instead?
> 
> I did extend treesit-node-check in the patch. But I also added a function treesit-parser-live-p, which makes the same check but directly on a parser. It just made sense to me that if we let treesit-node-check check the nodes’ parser’s status, we’d also add a function to allow directly checking the status of a parser.

That additional function would signal an error in the case discussed
here, so I'm not sure we should add it in that shape, or at all.  Why
isn't treesit-node-check enough?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Tue, 07 Feb 2023 04:56:02 GMT) Full text and rfc822 format available.

Message #38 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 61235 <at> debbugs.gnu.org, Mickey Petersen <mickey <at> masteringemacs.org>
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to
 tell if a node belongs to a deleted parser
Date: Mon, 6 Feb 2023 20:55:38 -0800
[Message part 1 (text/plain, inline)]

> On Feb 6, 2023, at 7:31 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>> From: Yuan Fu <casouri <at> gmail.com>
>> Date: Mon, 6 Feb 2023 19:00:30 -0800
>> Cc: Mickey Petersen <mickey <at> masteringemacs.org>,
>> 61235 <at> debbugs.gnu.org
>> 
>>> Yuan, any reason not to extend treesit-node-check instead?
>> 
>> I did extend treesit-node-check in the patch. But I also added a function treesit-parser-live-p, which makes the same check but directly on a parser. It just made sense to me that if we let treesit-node-check check the nodes’ parser’s status, we’d also add a function to allow directly checking the status of a parser.
> 
> That additional function would signal an error in the case discussed
> here, so I'm not sure we should add it in that shape, or at all.  Why
> isn't treesit-node-check enough?

Oops, it shouldn’t have. The updated patch fixes that. Treesit-node-check is enough, it just made more sense implentattion-wise, to implement that function that checks a parser, and let treesit-node-check use that function to check the node’s parser. We can choose to not expose that function, and only expose this feature through treesit-node-check, if you prefer so.

Yuan

[livep.patch (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Tue, 07 Feb 2023 08:06:02 GMT) Full text and rfc822 format available.

Message #41 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Mickey Petersen <mickey <at> masteringemacs.org>
To: Yuan Fu <casouri <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 61235 <at> debbugs.gnu.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way
 to tell if a node belongs to a deleted parser
Date: Tue, 07 Feb 2023 08:03:56 +0000
Yuan Fu <casouri <at> gmail.com> writes:

>> On Feb 6, 2023, at 7:21 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>
>>> From: Mickey Petersen <mickey <at> masteringemacs.org>
>>> Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
>>> Date: Mon, 06 Feb 2023 14:08:46 +0000
>>>
>>> All I want is a way for treesit-node-check to tell me if the node
>>> belongs to a dead or alive parser.
>>
>> That'd be fine by me, but the patch posted by Yuan was a different
>> one.
>>
>> Yuan, any reason not to extend treesit-node-check instead?
>
> I did extend treesit-node-check in the patch. But I also added a
> function treesit-parser-live-p, which makes the same check but
> directly on a parser. It just made sense to me that if we let
> treesit-node-check check the nodes’ parser’s status, we’d also add a
> function to allow directly checking the status of a parser.
>
> Micky, the function I added (and the extension to treesit-node-check)
> checks that the parser is not deleted AND its buffer is live. That
> makes the most sense to me, but would it cause any problem for your
> use case?

Thanks for turning around the features so fast.

I can use `treesit-node-buffer' and `buffer-live-p' to accomplish
that, so perhaps leaving out that check makes sense?

> Yuan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Tue, 07 Feb 2023 12:25:02 GMT) Full text and rfc822 format available.

Message #44 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 61235 <at> debbugs.gnu.org, mickey <at> masteringemacs.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to
 tell if a node belongs to a deleted parser
Date: Tue, 07 Feb 2023 14:24:06 +0200
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Mon, 6 Feb 2023 20:55:38 -0800
> Cc: Mickey Petersen <mickey <at> masteringemacs.org>,
>  61235 <at> debbugs.gnu.org
> 
> >>> Yuan, any reason not to extend treesit-node-check instead?
> >> 
> >> I did extend treesit-node-check in the patch. But I also added a function treesit-parser-live-p, which makes the same check but directly on a parser. It just made sense to me that if we let treesit-node-check check the nodes’ parser’s status, we’d also add a function to allow directly checking the status of a parser.
> > 
> > That additional function would signal an error in the case discussed
> > here, so I'm not sure we should add it in that shape, or at all.  Why
> > isn't treesit-node-check enough?
> 
> Oops, it shouldn’t have. The updated patch fixes that. Treesit-node-check is enough, it just made more sense implentattion-wise, to implement that function that checks a parser, and let treesit-node-check use that function to check the node’s parser. We can choose to not expose that function, and only expose this feature through treesit-node-check, if you prefer so.

I think treesit-node-check alone should be enough.

One comment:

> @@ -1943,9 +1959,11 @@ DEFUN ("treesit-node-check",
>      result = ts_node_is_extra (treesit_node);
>    else if (EQ (property, Qhas_error))
>      result = ts_node_has_error (treesit_node);
> +  else if (EQ (property, Qlive))
> +    result = Ftreesit_parser_live_p (XTS_NODE (node)->parser);

Ftreesit_parser_live_p returns a Lisp object, whereas 'result' is a C
'bool' type.  This won't compile if you configure with
"--enable-check-lisp-object-type".




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Wed, 08 Feb 2023 03:54:02 GMT) Full text and rfc822 format available.

Message #47 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Mickey Petersen <mickey <at> masteringemacs.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 61235 <at> debbugs.gnu.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to
 tell if a node belongs to a deleted parser
Date: Tue, 7 Feb 2023 19:52:56 -0800

> On Feb 7, 2023, at 12:03 AM, Mickey Petersen <mickey <at> masteringemacs.org> wrote:
> 
> 
> Yuan Fu <casouri <at> gmail.com> writes:
> 
>>> On Feb 6, 2023, at 7:21 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>> 
>>>> From: Mickey Petersen <mickey <at> masteringemacs.org>
>>>> Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
>>>> Date: Mon, 06 Feb 2023 14:08:46 +0000
>>>> 
>>>> All I want is a way for treesit-node-check to tell me if the node
>>>> belongs to a dead or alive parser.
>>> 
>>> That'd be fine by me, but the patch posted by Yuan was a different
>>> one.
>>> 
>>> Yuan, any reason not to extend treesit-node-check instead?
>> 
>> I did extend treesit-node-check in the patch. But I also added a
>> function treesit-parser-live-p, which makes the same check but
>> directly on a parser. It just made sense to me that if we let
>> treesit-node-check check the nodes’ parser’s status, we’d also add a
>> function to allow directly checking the status of a parser.
>> 
>> Micky, the function I added (and the extension to treesit-node-check)
>> checks that the parser is not deleted AND its buffer is live. That
>> makes the most sense to me, but would it cause any problem for your
>> use case?
> 
> Thanks for turning around the features so fast.
> 
> I can use `treesit-node-buffer' and `buffer-live-p' to accomplish
> that, so perhaps leaving out that check makes sense?

I’m hoping to write the function as I described, ie, return t only if the parser is not deleted and its buffer is live. So I wonder if this definition of “live” would work for you?

Yuan



Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Wed, 08 Feb 2023 03:55:02 GMT) Full text and rfc822 format available.

Message #50 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 61235 <at> debbugs.gnu.org, mickey <at> masteringemacs.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to
 tell if a node belongs to a deleted parser
Date: Tue, 7 Feb 2023 19:54:03 -0800

> On Feb 7, 2023, at 4:24 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>> From: Yuan Fu <casouri <at> gmail.com>
>> Date: Mon, 6 Feb 2023 20:55:38 -0800
>> Cc: Mickey Petersen <mickey <at> masteringemacs.org>,
>> 61235 <at> debbugs.gnu.org
>> 
>>>>> Yuan, any reason not to extend treesit-node-check instead?
>>>> 
>>>> I did extend treesit-node-check in the patch. But I also added a function treesit-parser-live-p, which makes the same check but directly on a parser. It just made sense to me that if we let treesit-node-check check the nodes’ parser’s status, we’d also add a function to allow directly checking the status of a parser.
>>> 
>>> That additional function would signal an error in the case discussed
>>> here, so I'm not sure we should add it in that shape, or at all.  Why
>>> isn't treesit-node-check enough?
>> 
>> Oops, it shouldn’t have. The updated patch fixes that. Treesit-node-check is enough, it just made more sense implentattion-wise, to implement that function that checks a parser, and let treesit-node-check use that function to check the node’s parser. We can choose to not expose that function, and only expose this feature through treesit-node-check, if you prefer so.
> 
> I think treesit-node-check alone should be enough.

Cool, I’ll only modify treesit-node-check, then.

> 
> One comment:
> 
>> @@ -1943,9 +1959,11 @@ DEFUN ("treesit-node-check",
>>     result = ts_node_is_extra (treesit_node);
>>   else if (EQ (property, Qhas_error))
>>     result = ts_node_has_error (treesit_node);
>> +  else if (EQ (property, Qlive))
>> +    result = Ftreesit_parser_live_p (XTS_NODE (node)->parser);
> 
> Ftreesit_parser_live_p returns a Lisp object, whereas 'result' is a C
> 'bool' type.  This won't compile if you configure with
> "--enable-check-lisp-object-type”.

Right, sorry :-(

Yuan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Wed, 08 Feb 2023 08:43:02 GMT) Full text and rfc822 format available.

Message #53 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Mickey Petersen <mickey <at> masteringemacs.org>
To: Yuan Fu <casouri <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 61235 <at> debbugs.gnu.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way
 to tell if a node belongs to a deleted parser
Date: Wed, 08 Feb 2023 08:41:35 +0000
Yuan Fu <casouri <at> gmail.com> writes:

>> On Feb 7, 2023, at 12:03 AM, Mickey Petersen <mickey <at> masteringemacs.org> wrote:
>>
>>
>> Yuan Fu <casouri <at> gmail.com> writes:
>>
>>>> On Feb 6, 2023, at 7:21 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>>>
>>>>> From: Mickey Petersen <mickey <at> masteringemacs.org>
>>>>> Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
>>>>> Date: Mon, 06 Feb 2023 14:08:46 +0000
>>>>>
>>>>> All I want is a way for treesit-node-check to tell me if the node
>>>>> belongs to a dead or alive parser.
>>>>
>>>> That'd be fine by me, but the patch posted by Yuan was a different
>>>> one.
>>>>
>>>> Yuan, any reason not to extend treesit-node-check instead?
>>>
>>> I did extend treesit-node-check in the patch. But I also added a
>>> function treesit-parser-live-p, which makes the same check but
>>> directly on a parser. It just made sense to me that if we let
>>> treesit-node-check check the nodes’ parser’s status, we’d also add a
>>> function to allow directly checking the status of a parser.
>>>
>>> Micky, the function I added (and the extension to treesit-node-check)
>>> checks that the parser is not deleted AND its buffer is live. That
>>> makes the most sense to me, but would it cause any problem for your
>>> use case?
>>
>> Thanks for turning around the features so fast.
>>
>> I can use `treesit-node-buffer' and `buffer-live-p' to accomplish
>> that, so perhaps leaving out that check makes sense?
>
> I’m hoping to write the function as I described, ie, return t only if
> the parser is not deleted and its buffer is live. So I wonder if this
> definition of “live” would work for you?

Sounds good to me, and I think others will find it useful as well!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#61235; Package emacs. (Fri, 10 Feb 2023 01:29:01 GMT) Full text and rfc822 format available.

Message #56 received at 61235 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Mickey Petersen <mickey <at> masteringemacs.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 61235 <at> debbugs.gnu.org
Subject: Re: bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way 
 to tell if a node belongs to a deleted parser
Date: Thu, 9 Feb 2023 17:28:30 -0800
Mickey Petersen <mickey <at> masteringemacs.org> writes:

> Yuan Fu <casouri <at> gmail.com> writes:
>
>>> On Feb 7, 2023, at 12:03 AM, Mickey Petersen <mickey <at> masteringemacs.org> wrote:
>>>
>>>
>>> Yuan Fu <casouri <at> gmail.com> writes:
>>>
>>>>> On Feb 6, 2023, at 7:21 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>>>>
>>>>>> From: Mickey Petersen <mickey <at> masteringemacs.org>
>>>>>> Cc: casouri <at> gmail.com, 61235 <at> debbugs.gnu.org
>>>>>> Date: Mon, 06 Feb 2023 14:08:46 +0000
>>>>>>
>>>>>> All I want is a way for treesit-node-check to tell me if the node
>>>>>> belongs to a dead or alive parser.
>>>>>
>>>>> That'd be fine by me, but the patch posted by Yuan was a different
>>>>> one.
>>>>>
>>>>> Yuan, any reason not to extend treesit-node-check instead?
>>>>
>>>> I did extend treesit-node-check in the patch. But I also added a
>>>> function treesit-parser-live-p, which makes the same check but
>>>> directly on a parser. It just made sense to me that if we let
>>>> treesit-node-check check the nodes’ parser’s status, we’d also add a
>>>> function to allow directly checking the status of a parser.
>>>>
>>>> Micky, the function I added (and the extension to treesit-node-check)
>>>> checks that the parser is not deleted AND its buffer is live. That
>>>> makes the most sense to me, but would it cause any problem for your
>>>> use case?
>>>
>>> Thanks for turning around the features so fast.
>>>
>>> I can use `treesit-node-buffer' and `buffer-live-p' to accomplish
>>> that, so perhaps leaving out that check makes sense?
>>
>> I’m hoping to write the function as I described, ie, return t only if
>> the parser is not deleted and its buffer is live. So I wonder if this
>> definition of “live” would work for you?
>
> Sounds good to me, and I think others will find it useful as well!

Done.

Yuan




bug marked as fixed in version 29.1, send any further explanations to 61235 <at> debbugs.gnu.org and Mickey Petersen <mickey <at> masteringemacs.org> Request was from Yuan Fu <casouri <at> gmail.com> to control <at> debbugs.gnu.org. (Sun, 19 Feb 2023 20:59:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 20 Mar 2023 11:24:07 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 9 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.