* 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-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