Some issues with the tree-sitter branch

Some issues with the tree-sitter branch

[Eli Zaretskii]

I noticed several minor issues with the branch while reading the code:

 . Several places assign EMACS_INT values to uint32_t variables with
   an explicit range check (and error signal in case of overflow).
 . Several functions produce Lisp_Object results by reference, and
   callers pass to them pointers to Lisp_Object variables.  Our style
   prefers returning a Lisp_Object value through the return value,
   like this:

      Lisp_Object some_var = some_func (...);

   When a function produces a single value, I think the above is
   preferable.
 . There's a call to malloc in Ftreesit_parser_set_included_ranges
   which doesn't check the return value of malloc, and doesn't signal
   memory-full error when malloc fails (that function should perhaps
   use SAFE_ALLOCA).

In addition, the style of treesit.c (indentation etc.) is not exactly
ours (but this can be fixed later).

Thank you for your work on this important feature.

Re: Some issues with the tree-sitter branch

[Eli Zaretskii]

> Date: Sun, 16 Oct 2022 16:32:10 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> CC: emacs-devel@gnu.org
> 
> I noticed several minor issues with the branch while reading the code:

Oh, and one more: the test/src/treesit-tests.el test suite assumes
without checking that certain language definitions are installed.  It
would be nice to skip the tests which need languages that aren't
installed.

Re: Some issues with the tree-sitter branch

[Eli Zaretskii]

> Date: Sun, 16 Oct 2022 16:32:10 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> CC: emacs-devel@gnu.org
> 
> In addition, the style of treesit.c (indentation etc.) is not exactly
> ours (but this can be fixed later).

I fixed some of those now.

tree-sitter: Paths used for loading of language definitions

[Jostein Kjønigsen]

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

Hey there, Yuan Fu.

First of all: Thank you for the tremendous effort you've put in to get a 
tree-sitter implementation mainlined into Emacs. For some of the 
major-modes I've been working on, tree-sitter has been a godsend and if 
it can enable some of them to become mainlined into Emacs... I mean 
nothing could be better!

That said, I've noticed something when building the feature/tree-sitter 
branch, and also while testing other things tree-sitter related... If a 
language-definition SO is missing... treesit.el in Emacs only reports 
looking for files and folders typically found within $HOME/.emacs.d/ 
(that is user-owned files).

Based on that, I'm assuming those are the only locations probed. Is that 
assumption correct?

If so, I think that is going to become a problem quite quickly if/when 
Linux distro starts providing Emacs-builds with tree-sitter enabled. 
Before that can be done, there needs to be support put in place to load 
these from a system/distro/package-manager owned location, or else it's 
going to be near-impossible for distros to ship a working Emacs with 
tree-sitter.

Should we consider adding another system-wide location for such files? 
/usr/lib/emacs? /usr/share/emacs?

What do you guys think?

-- 
Kind regards
*Jostein Kjønigsen*

jostein@kjonigsen.net 🍵 jostein@gmail.com
https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>

[-- Attachment #2: Type: text/html, Size: 1977 bytes --]

Re: tree-sitter: Paths used for loading of language definitions

[Daniel Martín]

Jostein Kjønigsen <jostein@secure.kjonigsen.net> writes:

> Hey there, Yuan Fu.
>
> First of all: Thank you for the tremendous effort you've put in to get
> a tree-sitter implementation mainlined into Emacs. For some of the
> major-modes I've been working on, tree-sitter has been a godsend and
> if it can enable some of them to become mainlined into Emacs... I mean
> nothing could be better!
>
> That said, I've noticed something when building the
> feature/tree-sitter branch, and also while testing other things
> tree-sitter related... If a language-definition SO is
> missing... treesit.el in Emacs only reports looking for files and
> folders typically found within $HOME/.emacs.d/ (that is user-owned
> files).
>
> Based on that, I'm assuming those are the only locations probed. Is
> that assumption correct?

I think the variable treesit-extra-load-path is supposed to contain
those extra locations.

Re: tree-sitter: Paths used for loading of language definitions

[Yuan Fu]

> On Oct 16, 2022, at 12:36 PM, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:
> 
> Hey there, Yuan Fu.
> 
> First of all: Thank you for the tremendous effort you've put in to get a tree-sitter implementation mainlined into Emacs. For some of the major-modes I've been working on, tree-sitter has been a godsend and if it can enable some of them to become mainlined into Emacs... I mean nothing could be better!

Thank you!

> 
> That said, I've noticed something when building the feature/tree-sitter branch, and also while testing other things tree-sitter related... If a language-definition SO is missing... treesit.el in Emacs only reports looking for files and folders typically found within $HOME/.emacs.d/ (that is user-owned files).
> 
> Based on that, I'm assuming those are the only locations probed. Is that assumption correct?

It shouldn’t be. Emacs should first look in treesit-extra-load-path, then ~/.emacs.d/tree-sitter, then standard system library locations. The exact locations depend on dlopen. Could you share the error message reported when loading a non-exist language? It should print all the locations it tries (as you observed).

Yuan

Re: Some issues with the tree-sitter branch

[Yuan Fu]

> On Oct 16, 2022, at 6:32 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> I noticed several minor issues with the branch while reading the code:
> 
> . Several places assign EMACS_INT values to uint32_t variables with
>   an explicit range check (and error signal in case of overflow).

I think you meant the lack of a range check? Like mentioned here:

+  /* FIXME: We should signal an error below if START_BYTE
+     etc. overflow the 32-bit unsigned data type.  */

I added buffer size check at parser creation time, and used casts to uint32_t liberally, assuming the values never overflows and, so we don’t need to handle the error at a million places. But I should have added checks in ts_after_chang and other places where buffer size could change. I’ll add checks in ts_after_change and other places, and if the argumetns overflows uint32, it will set a flag (say, buffer_too_large) in the parser object, and next time any lisp function tries to use that parser, an buffer-too-large error will be signaled. WDYT?

> . Several functions produce Lisp_Object results by reference, and
>   callers pass to them pointers to Lisp_Object variables.  Our style
>   prefers returning a Lisp_Object value through the return value,
>   like this:
> 
>      Lisp_Object some_var = some_func (...);
> 
>   When a function produces a single value, I think the above is
>   preferable.

Got it.

> . There's a call to malloc in Ftreesit_parser_set_included_ranges
>   which doesn't check the return value of malloc, and doesn't signal
>   memory-full error when malloc fails (that function should perhaps
>   use SAFE_ALLOCA).

I’ll fix that.

> 
> In addition, the style of treesit.c (indentation etc.) is not exactly
> ours (but this can be fixed later).
> 
> Thank you for your work on this important feature.

Thank you!

I see that you fixed them, I’ll keep those in mind in the future. That’s a lot of lines you need to change, sorry about that :-(

Yuan

Re: Some issues with the tree-sitter branch

[Po Lu]

Yuan Fu <casouri@gmail.com> writes:

> I think you meant the lack of a range check? Like mentioned here:
>
> +  /* FIXME: We should signal an error below if START_BYTE
> +     etc. overflow the 32-bit unsigned data type.  */
>
> I added buffer size check at parser creation time, and used casts to
> uint32_t liberally, assuming the values never overflows and, so we
> don’t need to handle the error at a million places. But I should have
> added checks in ts_after_chang and other places where buffer size
> could change. I’ll add checks in ts_after_change and other places, and
> if the argumetns overflows uint32, it will set a flag (say,
> buffer_too_large) in the parser object, and next time any lisp
> function tries to use that parser, an buffer-too-large error will be
> signaled. WDYT?

Isn't there a way to make tree-sitter parse buffers larger than
uint32_t?  AFAIK, our buffers can be EMACS_INT large (especially with
long log files), and tree-sitter should be fast enough to parse them
incrementally?

Re: Some issues with the tree-sitter branch

[Yuan Fu]

> On Oct 16, 2022, at 10:37 PM, Po Lu <luangruo@yahoo.com> wrote:
> 
> Yuan Fu <casouri@gmail.com> writes:
> 
>> I think you meant the lack of a range check? Like mentioned here:
>> 
>> +  /* FIXME: We should signal an error below if START_BYTE
>> +     etc. overflow the 32-bit unsigned data type.  */
>> 
>> I added buffer size check at parser creation time, and used casts to
>> uint32_t liberally, assuming the values never overflows and, so we
>> don’t need to handle the error at a million places. But I should have
>> added checks in ts_after_chang and other places where buffer size
>> could change. I’ll add checks in ts_after_change and other places, and
>> if the argumetns overflows uint32, it will set a flag (say,
>> buffer_too_large) in the parser object, and next time any lisp
>> function tries to use that parser, an buffer-too-large error will be
>> signaled. WDYT?
> 
> Isn't there a way to make tree-sitter parse buffers larger than
> uint32_t?  AFAIK, our buffers can be EMACS_INT large (especially with
> long log files), and tree-sitter should be fast enough to parse them
> incrementally?

I don’t think we want to get even near that size, since tree-sitter uses ~10x the size of the buffer to store the parse tree. That’s probably why tree-sitter’s author didn’t bother to use uint64.

Yuan

Re: Some issues with the tree-sitter branch

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sun, 16 Oct 2022 21:53:06 -0700
> Cc: emacs-devel@gnu.org
> 
> > . Several places assign EMACS_INT values to uint32_t variables with
> >   an explicit range check (and error signal in case of overflow).
> 
> I think you meant the lack of a range check? Like mentioned here:
> 
> +  /* FIXME: We should signal an error below if START_BYTE
> +     etc. overflow the 32-bit unsigned data type.  */

Yes, I added FIXME comments to make it easier to find those places.

> I added buffer size check at parser creation time, and used casts to uint32_t liberally, assuming the values never overflows and, so we don’t need to handle the error at a million places. But I should have added checks in ts_after_chang and other places where buffer size could change. I’ll add checks in ts_after_change and other places, and if the argumetns overflows uint32, it will set a flag (say, buffer_too_large) in the parser object, and next time any lisp function tries to use that parser, an buffer-too-large error will be signaled. WDYT?

That sounds fine to me, but I think we also should do something when
some value that can be larger than UINT_MAX is passed to tree-sitter
functions, because doing so might cause tree-sitter do something for a
completely unrelated portion of the buffer.  At the very least add
eassert there, so that at least a build with --enable-checking will
detect such cases.

> I see that you fixed them, I’ll keep those in mind in the future. That’s a lot of lines you need to change, sorry about that :-(

That was a low-hanging fruit that was easy to pluck ;-)

Re: tree-sitter: Paths used for loading of language definitions

[Jostein Kjønigsen]

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


On 17.10.2022 06:27, Yuan Fu wrote:
>
>> That said, I've noticed something when building the feature/tree-sitter branch, and also while testing other things tree-sitter related... If a language-definition SO is missing... treesit.el in Emacs only reports looking for files and folders typically found within $HOME/.emacs.d/ (that is user-owned files).
>>
>> Based on that, I'm assuming those are the only locations probed. Is that assumption correct?
> It shouldn’t be. Emacs should first look in treesit-extra-load-path, then ~/.emacs.d/tree-sitter, then standard system library locations. The exact locations depend on dlopen. Could you share the error message reported when loading a non-exist language? It should print all the locations it tries (as you observed).
>
> Yuan

textmodes/mhtml-mode.el:29:2: Error: Cannot load language definition: 
"javascript", 
("/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript: cannot 
open shared object file: No such file or directory" 
"/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript.so: cannot 
open shared object file: No such file or directory" 
"libtree-sitter-javascript: cannot open shared object file: No such file 
or directory" "libtree-sitter-javascript.so: cannot open shared object 
file: No such file or directory")

Does that impliy it checks in the standard system library localtion? If 
so, I guess there's no problem.

In that case, sorry for the "noise", but I just wanted to make sure this 
was packagable and thought one check too much is better than one check 
too little :)


-- 
Vennlig hilsen
*Jostein Kjønigsen*

jostein@kjonigsen.net 🍵 jostein@gmail.com
https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>

[-- Attachment #2: Type: text/html, Size: 2547 bytes --]

Re: tree-sitter: Paths used for loading of language definitions

[Yuan Fu]

> On Oct 17, 2022, at 12:14 AM, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:
> 
> 
> 
> On 17.10.2022 06:27, Yuan Fu wrote:
>> 
>>> That said, I've noticed something when building the feature/tree-sitter branch, and also while testing other things tree-sitter related... If a language-definition SO is missing... treesit.el in Emacs only reports looking for files and folders typically found within $HOME/.emacs.d/ (that is user-owned files).
>>> 
>>> Based on that, I'm assuming those are the only locations probed. Is that assumption correct?
>>> 
>> It shouldn’t be. Emacs should first look in treesit-extra-load-path, then ~/.emacs.d/tree-sitter, then standard system library locations. The exact locations depend on dlopen. Could you share the error message reported when loading a non-exist language? It should print all the locations it tries (as you observed).
>> 
>> Yuan
>> 
> textmodes/mhtml-mode.el:29:2: Error: Cannot load language definition: "javascript", ("/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript: cannot open shared object file: No such file or directory" "/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript.so: cannot open shared object file: No such file or directory" "libtree-sitter-javascript: cannot open shared object file: No such file or directory" "libtree-sitter-javascript.so: cannot open shared object file: No such file or directory")

Ah, I guess your system doesn’t explicitly say what path has been checked. The last two messages in the list is where dlopen searches the system paths.

> 
> Does that impliy it checks in the standard system library localtion? If so, I guess there's no problem.
> 
> In that case, sorry for the "noise", but I just wanted to make sure this was packagable and thought one check too much is better than one check too little :)

Yes, false positives are always better than false negative ;-)

Yuan

Re: tree-sitter: Paths used for loading of language definitions

[Eli Zaretskii]

> Date: Mon, 17 Oct 2022 09:14:58 +0200
> Cc: emacs-devel@gnu.org
> From: Jostein Kjønigsen <jostein@secure.kjonigsen.net>
> 
> textmodes/mhtml-mode.el:29:2: Error: Cannot load language definition: "javascript",
> ("/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript: cannot open shared object file: No such file or
> directory" "/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript.so: cannot open shared object file: No
> such file or directory" "libtree-sitter-javascript: cannot open shared object file: No such file or directory"
> "libtree-sitter-javascript.so: cannot open shared object file: No such file or directory")

The error data should be improved to provide a more informative error
message.  It would be best to display just the name of the language
definition library, without leading directories.

Re: tree-sitter: Paths used for loading of language definitions

[Yuan Fu]

> On Oct 17, 2022, at 1:02 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> Date: Mon, 17 Oct 2022 09:14:58 +0200
>> Cc: emacs-devel@gnu.org
>> From: Jostein Kjønigsen <jostein@secure.kjonigsen.net>
>> 
>> textmodes/mhtml-mode.el:29:2: Error: Cannot load language definition: "javascript",
>> ("/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript: cannot open shared object file: No such file or
>> directory" "/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript.so: cannot open shared object file: No
>> such file or directory" "libtree-sitter-javascript: cannot open shared object file: No such file or directory"
>> "libtree-sitter-javascript.so: cannot open shared object file: No such file or directory")
> 
> The error data should be improved to provide a more informative error
> message.  It would be best to display just the name of the language
> definition library, without leading directories.

You mean something like 

Error: Cannot load language definition: "javascript”, “libtree-sitter-javascript.so”

?

Yuan

Re: tree-sitter: Paths used for loading of language definitions

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Mon, 17 Oct 2022 02:02:43 -0700
> Cc: jostein@kjonigsen.net,
>  emacs-devel@gnu.org
> 
> > On Oct 17, 2022, at 1:02 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > 
> >> Date: Mon, 17 Oct 2022 09:14:58 +0200
> >> Cc: emacs-devel@gnu.org
> >> From: Jostein Kjønigsen <jostein@secure.kjonigsen.net>
> >> 
> >> textmodes/mhtml-mode.el:29:2: Error: Cannot load language definition: "javascript",
> >> ("/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript: cannot open shared object file: No such file or
> >> directory" "/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript.so: cannot open shared object file: No
> >> such file or directory" "libtree-sitter-javascript: cannot open shared object file: No such file or directory"
> >> "libtree-sitter-javascript.so: cannot open shared object file: No such file or directory")
> > 
> > The error data should be improved to provide a more informative error
> > message.  It would be best to display just the name of the language
> > definition library, without leading directories.
> 
> You mean something like 
> 
> Error: Cannot load language definition: "javascript”, “libtree-sitter-javascript.so”
> 
> ?

Better yet

  Error: Cannot load language definition: "javascript”, “libtree-sitter-javascript.so”: No such file or directory

Re: Some issues with the tree-sitter branch

[Yuan Fu]

> 
>> I added buffer size check at parser creation time, and used casts to uint32_t liberally, assuming the values never overflows and, so we don’t need to handle the error at a million places. But I should have added checks in ts_after_chang and other places where buffer size could change. I’ll add checks in ts_after_change and other places, and if the argumetns overflows uint32, it will set a flag (say, buffer_too_large) in the parser object, and next time any lisp function tries to use that parser, an buffer-too-large error will be signaled. WDYT?
> 
> That sounds fine to me, but I think we also should do something when
> some value that can be larger than UINT_MAX is passed to tree-sitter
> functions, because doing so might cause tree-sitter do something for a
> completely unrelated portion of the buffer.  At the very least add
> eassert there, so that at least a build with --enable-checking will
> detect such cases.

IIUC you don’t want to hide and delay the error, right? That makes sense. I can add assertions, those assertions shouldn’t interfere with normal usage.

Yuan

Re: Some issues with the tree-sitter branch

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Mon, 17 Oct 2022 02:12:13 -0700
> Cc: emacs-devel@gnu.org
> 
> > That sounds fine to me, but I think we also should do something when
> > some value that can be larger than UINT_MAX is passed to tree-sitter
> > functions, because doing so might cause tree-sitter do something for a
> > completely unrelated portion of the buffer.  At the very least add
> > eassert there, so that at least a build with --enable-checking will
> > detect such cases.
> 
> IIUC you don’t want to hide and delay the error, right? That makes sense. I can add assertions, those assertions shouldn’t interfere with normal usage.

Yes, please.

Re: tree-sitter: Paths used for loading of language definitions

[Stephen Leake]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Yuan Fu <casouri@gmail.com>
>> Date: Mon, 17 Oct 2022 02:02:43 -0700
>> Cc: jostein@kjonigsen.net,
>>  emacs-devel@gnu.org
>> 
>> > On Oct 17, 2022, at 1:02 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> > 
>> >> Date: Mon, 17 Oct 2022 09:14:58 +0200
>> >> Cc: emacs-devel@gnu.org
>> >> From: Jostein Kjønigsen <jostein@secure.kjonigsen.net>
>> >> 
>> >> textmodes/mhtml-mode.el:29:2: Error: Cannot load language definition: "javascript",
>> >> ("/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript:
>> >> cannot open shared object file: No such file or
>> >> directory"
>> >> "/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript.so:
>> >> cannot open shared object file: No
>> >> such file or directory" "libtree-sitter-javascript: cannot open
>> >> shared object file: No such file or directory"
>> >> "libtree-sitter-javascript.so: cannot open shared object file: No
>> >> such file or directory")
>> > 
>> > The error data should be improved to provide a more informative error
>> > message.  It would be best to display just the name of the language
>> > definition library, without leading directories.
>> 
>> You mean something like 
>> 
>> Error: Cannot load language definition: "javascript”, “libtree-sitter-javascript.so”
>> 
>> ?
>
> Better yet
>
>   Error: Cannot load language definition: "javascript”,
> “libtree-sitter-javascript.so”: No such file or directory

I would find it helpful if the error message told me what variable
contains the search path, so I can append the correct directory to it.

-- 
-- Stephe

Re: tree-sitter: Paths used for loading of language definitions

[Eli Zaretskii]

> From: Stephen Leake <stephen_leake@stephe-leake.org>
> Cc: Yuan Fu <casouri@gmail.com>,  jostein@kjonigsen.net,  emacs-devel@gnu.org
> Date: Mon, 17 Oct 2022 04:56:38 -0700
> 
> >   Error: Cannot load language definition: "javascript”,
> > “libtree-sitter-javascript.so”: No such file or directory
> 
> I would find it helpful if the error message told me what variable
> contains the search path, so I can append the correct directory to it.

We don't say that when we fail to load other kinds of files.  For
example:

  (load "nonexistent")
   => error: (file-missing "Cannot open load file" "No such file or directory" "nonexistent")

So I see no reason to mention the variable in this case.  There's no
limit to relevant information to supply for an error, but we cannot
possibly give all of that when we signal an error.  The failure to
load a file should prompt the user to look for the information how
these files are looked up, and that should lead to the variable,
among other things.

Re: tree-sitter: Paths used for loading of language definitions

[Yuan Fu]

> On Oct 17, 2022, at 2:08 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Mon, 17 Oct 2022 02:02:43 -0700
>> Cc: jostein@kjonigsen.net,
>> emacs-devel@gnu.org
>> 
>>> On Oct 17, 2022, at 1:02 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> 
>>>> Date: Mon, 17 Oct 2022 09:14:58 +0200
>>>> Cc: emacs-devel@gnu.org
>>>> From: Jostein Kjønigsen <jostein@secure.kjonigsen.net>
>>>> 
>>>> textmodes/mhtml-mode.el:29:2: Error: Cannot load language definition: "javascript",
>>>> ("/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript: cannot open shared object file: No such file or
>>>> directory" "/home/jostein/.emacs.d/tree-sitter/libtree-sitter-javascript.so: cannot open shared object file: No
>>>> such file or directory" "libtree-sitter-javascript: cannot open shared object file: No such file or directory"
>>>> "libtree-sitter-javascript.so: cannot open shared object file: No such file or directory")
>>> 
>>> The error data should be improved to provide a more informative error
>>> message.  It would be best to display just the name of the language
>>> definition library, without leading directories.
>> 
>> You mean something like 
>> 
>> Error: Cannot load language definition: "javascript”, “libtree-sitter-javascript.so”
>> 
>> ?
> 
> Better yet
> 
>  Error: Cannot load language definition: "javascript”, “libtree-sitter-javascript.so”: No such file or directory

Done.

Re: Some issues with the tree-sitter branch

[Yuan Fu]

> 
>> . There's a call to malloc in Ftreesit_parser_set_included_ranges
>>  which doesn't check the return value of malloc, and doesn't signal
>>  memory-full error when malloc fails (that function should perhaps
>>  use SAFE_ALLOCA).
> 
> I’ll fix that.

Done. IIUC I just need to change malloc to xmalloc, right?

Yuan

Re: Some issues with the tree-sitter branch

[Yuan Fu]

> On Oct 17, 2022, at 3:14 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Mon, 17 Oct 2022 02:12:13 -0700
>> Cc: emacs-devel@gnu.org
>> 
>>> That sounds fine to me, but I think we also should do something when
>>> some value that can be larger than UINT_MAX is passed to tree-sitter
>>> functions, because doing so might cause tree-sitter do something for a
>>> completely unrelated portion of the buffer.  At the very least add
>>> eassert there, so that at least a build with --enable-checking will
>>> detect such cases.
>> 
>> IIUC you don’t want to hide and delay the error, right? That makes sense. I can add assertions, those assertions shouldn’t interfere with normal usage.
> 
> Yes, please.

Now done.

Re: Some issues with the tree-sitter branch

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Mon, 17 Oct 2022 17:14:17 -0700
> Cc: emacs-devel@gnu.org
> 
> 
> > 
> >> . There's a call to malloc in Ftreesit_parser_set_included_ranges
> >>  which doesn't check the return value of malloc, and doesn't signal
> >>  memory-full error when malloc fails (that function should perhaps
> >>  use SAFE_ALLOCA).
> > 
> > I’ll fix that.
> 
> Done. IIUC I just need to change malloc to xmalloc, right?

Yes, that's one way.