unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal'
@ 2023-01-08 14:55 Mickey Petersen
  2023-01-09  3:59 ` Yuan Fu
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Mickey Petersen @ 2023-01-08 14:55 UTC (permalink / raw)
  To: 60659


In order to check for node equality, one must use `treesit-node-eq'.

But I see little reason why two identical nodes in in the same tree aren't `equal'?



In GNU Emacs 30.0.50 (build 6, x86_64-pc-linux-gnu, GTK+ Version
 3.24.20, cairo version 1.16.0) of 2023-01-02 built on mickey-work
Repository revision: c209802f7b3721a1b95113290934a23fee88f678
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'






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal'
  2023-01-08 14:55 bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal' Mickey Petersen
@ 2023-01-09  3:59 ` Yuan Fu
  2023-01-09  8:55   ` Mickey Petersen
  2023-01-10  5:56 ` Yuan Fu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Yuan Fu @ 2023-01-09  3:59 UTC (permalink / raw)
  To: Mickey Petersen; +Cc: 60659


Mickey Petersen <mickey@masteringemacs.org> writes:

> In order to check for node equality, one must use `treesit-node-eq'.
>
> But I see little reason why two identical nodes in in the same tree aren't `equal'?

Indeed. I’ll see if I can extend equal to handle tree-sitter nodes.

Yuan





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal'
  2023-01-09  3:59 ` Yuan Fu
@ 2023-01-09  8:55   ` Mickey Petersen
  0 siblings, 0 replies; 8+ messages in thread
From: Mickey Petersen @ 2023-01-09  8:55 UTC (permalink / raw)
  To: Yuan Fu; +Cc: 60659


Yuan Fu <casouri@gmail.com> writes:

> Mickey Petersen <mickey@masteringemacs.org> writes:
>
>> In order to check for node equality, one must use `treesit-node-eq'.
>>
>> But I see little reason why two identical nodes in in the same tree aren't `equal'?
>
> Indeed. I’ll see if I can extend equal to handle tree-sitter nodes.
>

Nice one. This will allow for membership checks with `member' and
such-like and it'll greatly cut down on custom code, particularly one
you start writing tests!

> Yuan






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal'
  2023-01-08 14:55 bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal' Mickey Petersen
  2023-01-09  3:59 ` Yuan Fu
@ 2023-01-10  5:56 ` Yuan Fu
  2023-01-10 13:11   ` Eli Zaretskii
  2023-01-11  2:57 ` Yuan Fu
  2023-01-13  1:08 ` Yuan Fu
  3 siblings, 1 reply; 8+ messages in thread
From: Yuan Fu @ 2023-01-10  5:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60659

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


Mickey Petersen <mickey@masteringemacs.org> writes:

> Yuan Fu <casouri@gmail.com> writes:
>
>> Mickey Petersen <mickey@masteringemacs.org> writes:
>>
>>> In order to check for node equality, one must use `treesit-node-eq'.
>>>
>>> But I see little reason why two identical nodes in in the same tree aren't `equal'?
>>
>> Indeed. I’ll see if I can extend equal to handle tree-sitter nodes.
>>
>
> Nice one. This will allow for membership checks with `member' and
> such-like and it'll greatly cut down on custom code, particularly one
> you start writing tests!
>
>> Yuan

Eli, does this look right to you? In particular, I’m not sure if it’s ok to call a lisp function (Ftreesit_node_eq) inside internal_equal, even though it never signals. 

Yuan


[-- Attachment #2: equal.diff --]
[-- Type: application/octet-stream, Size: 878 bytes --]

diff --git a/src/fns.c b/src/fns.c
index 1aaf17914a2..c0ecc735bc5 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -38,6 +38,10 @@ Copyright (C) 1985-2023 Free Software Foundation, Inc.
 #include "puresize.h"
 #include "gnutls.h"
 
+#ifdef HAVE_TREE_SITTER
+#include "treesit.h"
+#endif
+
 enum equal_kind { EQUAL_NO_QUIT, EQUAL_PLAIN, EQUAL_INCLUDING_PROPERTIES };
 static bool internal_equal (Lisp_Object, Lisp_Object,
 			    enum equal_kind, int, Lisp_Object);
@@ -2822,6 +2826,10 @@ internal_equal (Lisp_Object o1, Lisp_Object o2, enum equal_kind equal_kind,
 		    && !memcmp (bool_vector_data (o1), bool_vector_data (o2),
 			        bool_vector_bytes (size)));
 	  }
+	if (TS_NODEP (o1))
+	  {
+	    return !NILP (Ftreesit_node_eq (o1, o2));
+	  }
 
 	/* Aside from them, only true vectors, char-tables, compiled
 	   functions, and fonts (font-spec, font-entity, font-object)

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal'
  2023-01-10  5:56 ` Yuan Fu
@ 2023-01-10 13:11   ` Eli Zaretskii
  0 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2023-01-10 13:11 UTC (permalink / raw)
  To: Yuan Fu, Lars Ingebrigtsen, Stefan Monnier; +Cc: 60659

> From: Yuan Fu <casouri@gmail.com>
> Date: Mon, 9 Jan 2023 21:56:19 -0800
> Cc: 60659@debbugs.gnu.org
> 
> Eli, does this look right to you? In particular, I’m not sure if it’s ok to call a lisp function (Ftreesit_node_eq) inside internal_equal, even though it never signals. 

It is better to make a C function from most of the body of
Ftreesit_node_eq, but without the CHECK_NODE parts, and make both
Fequal and Ftreesit_node_eq call that.  Because CHECK_NODE can signal,
right?  Also, you should make sure up front that _both_ o1 and o2 are
treesit nodes, otherwise they cannot be 'equal'.

Lars, Stefan: any comments/suggestions?





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal'
  2023-01-08 14:55 bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal' Mickey Petersen
  2023-01-09  3:59 ` Yuan Fu
  2023-01-10  5:56 ` Yuan Fu
@ 2023-01-11  2:57 ` Yuan Fu
  2023-01-11 12:26   ` Eli Zaretskii
  2023-01-13  1:08 ` Yuan Fu
  3 siblings, 1 reply; 8+ messages in thread
From: Yuan Fu @ 2023-01-11  2:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60659, larsi, monnier

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


Eli Zaretskii <eliz@gnu.org> writes:

>> From: Yuan Fu <casouri@gmail.com>
>> Date: Mon, 9 Jan 2023 21:56:19 -0800
>> Cc: 60659@debbugs.gnu.org
>> 
>> Eli, does this look right to you? In particular, I’m not sure if
>> it’s ok to call a lisp function (Ftreesit_node_eq) inside
>> internal_equal, even though it never signals.
>
> It is better to make a C function from most of the body of
> Ftreesit_node_eq, but without the CHECK_NODE parts, and make both
> Fequal and Ftreesit_node_eq call that.  Because CHECK_NODE can signal,
> right?  Also, you should make sure up front that _both_ o1 and o2 are
> treesit nodes, otherwise they cannot be 'equal'.

I should’ve included some context. There is code that checks whether o1
and o2 have the same (pseudo vector) type and size[1], and returns early
if not, so I only need to check the type of o1. And since I checked the
type of o1, CHECK_NODE will always succeed.

But still, using a C functions is more correct. How about this?


[-- Attachment #2: equal.diff --]
[-- Type: application/octet-stream, Size: 2391 bytes --]

diff --git a/src/fns.c b/src/fns.c
index 1aaf17914a2..d5f7565d3d7 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -38,6 +38,10 @@ Copyright (C) 1985-2023 Free Software Foundation, Inc.
 #include "puresize.h"
 #include "gnutls.h"
 
+#ifdef HAVE_TREE_SITTER
+#include "treesit.h"
+#endif
+
 enum equal_kind { EQUAL_NO_QUIT, EQUAL_PLAIN, EQUAL_INCLUDING_PROPERTIES };
 static bool internal_equal (Lisp_Object, Lisp_Object,
 			    enum equal_kind, int, Lisp_Object);
@@ -2822,6 +2826,10 @@ internal_equal (Lisp_Object o1, Lisp_Object o2, enum equal_kind equal_kind,
 		    && !memcmp (bool_vector_data (o1), bool_vector_data (o2),
 			        bool_vector_bytes (size)));
 	  }
+	if (TS_NODEP (o1))
+	  {
+	    return treesit_node_eq (o1, o2);
+	  }
 
 	/* Aside from them, only true vectors, char-tables, compiled
 	   functions, and fonts (font-spec, font-entity, font-object)
diff --git a/src/treesit.c b/src/treesit.c
index 55463122d14..6a1a5c5ea2a 100644
--- a/src/treesit.c
+++ b/src/treesit.c
@@ -2154,6 +2154,16 @@ DEFUN ("treesit-node-descendant-for-range",
   return make_treesit_node (XTS_NODE (node)->parser, child);
 }
 
+/* Return true if NODE1 and NODE2 are the same node.  Assumes they are
+   TS_NODE type.  */
+bool treesit_node_eq (Lisp_Object node1, Lisp_Object node2)
+{
+  treesit_initialize ();
+  TSNode treesit_node_1 = XTS_NODE (node1)->node;
+  TSNode treesit_node_2 = XTS_NODE (node2)->node;
+  return ts_node_eq (treesit_node_1, treesit_node_2);
+}
+
 DEFUN ("treesit-node-eq",
        Ftreesit_node_eq,
        Streesit_node_eq, 2, 2, 0,
@@ -2166,12 +2176,7 @@ DEFUN ("treesit-node-eq",
   CHECK_TS_NODE (node1);
   CHECK_TS_NODE (node2);
 
-  treesit_initialize ();
-
-  TSNode treesit_node_1 = XTS_NODE (node1)->node;
-  TSNode treesit_node_2 = XTS_NODE (node2)->node;
-
-  bool same_node = ts_node_eq (treesit_node_1, treesit_node_2);
+  bool same_node = treesit_node_eq (node1, node2);
   return same_node ? Qt : Qnil;
 }
 
diff --git a/src/treesit.h b/src/treesit.h
index 909609737d3..5382bc58817 100644
--- a/src/treesit.h
+++ b/src/treesit.h
@@ -191,6 +191,7 @@ CHECK_TS_COMPILED_QUERY (Lisp_Object query)
 extern void treesit_delete_parser (struct Lisp_TS_Parser *);
 extern void treesit_delete_query (struct Lisp_TS_Query *);
 extern bool treesit_named_node_p (TSNode);
+extern bool treesit_node_eq (Lisp_Object, Lisp_Object);
 
 #endif	/* HAVE_TREE_SITTER */
 

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



Yuan

[1]

	ptrdiff_t size = ASIZE (o1);
	/* Pseudovectors have the type encoded in the size field, so this test
	   actually checks that the objects have the same type as well as the
	   same size.  */
	if (ASIZE (o2) != size)
	  return false;

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal'
  2023-01-11  2:57 ` Yuan Fu
@ 2023-01-11 12:26   ` Eli Zaretskii
  0 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2023-01-11 12:26 UTC (permalink / raw)
  To: Yuan Fu; +Cc: 60659, larsi, monnier

> From: Yuan Fu <casouri@gmail.com>
> Date: Tue, 10 Jan 2023 18:57:02 -0800
> Cc: larsi@gnus.org,
>  monnier@iro.umontreal.ca,
>  60659@debbugs.gnu.org
> 
> But still, using a C functions is more correct. How about this?

Yes, this is what I had in mind.  Thanks.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal'
  2023-01-08 14:55 bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal' Mickey Petersen
                   ` (2 preceding siblings ...)
  2023-01-11  2:57 ` Yuan Fu
@ 2023-01-13  1:08 ` Yuan Fu
  3 siblings, 0 replies; 8+ messages in thread
From: Yuan Fu @ 2023-01-13  1:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, 60659-done, monnier


Eli Zaretskii <eliz@gnu.org> writes:

>> From: Yuan Fu <casouri@gmail.com>
>> Date: Tue, 10 Jan 2023 18:57:02 -0800
>> Cc: larsi@gnus.org,
>>  monnier@iro.umontreal.ca,
>>  60659@debbugs.gnu.org
>> 
>> But still, using a C functions is more correct. How about this?
>
> Yes, this is what I had in mind.  Thanks.

Cool. I applied it.

Yuan





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-01-13  1:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-08 14:55 bug#60659: 30.0.50; tree-sitter: identical nodes are not `equal' Mickey Petersen
2023-01-09  3:59 ` Yuan Fu
2023-01-09  8:55   ` Mickey Petersen
2023-01-10  5:56 ` Yuan Fu
2023-01-10 13:11   ` Eli Zaretskii
2023-01-11  2:57 ` Yuan Fu
2023-01-11 12:26   ` Eli Zaretskii
2023-01-13  1:08 ` Yuan Fu

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).