* bug#40562: [patch] Treat records as arrays in ert object comparisons and add support for cl-structs
@ 2020-04-11 20:34 Clément Pit-Claudel
2020-04-11 23:26 ` Štěpán Němec
0 siblings, 1 reply; 8+ messages in thread
From: Clément Pit-Claudel @ 2020-04-11 20:34 UTC (permalink / raw)
To: 40562; +Cc: lars
[-- Attachment #1: Type: text/plain, Size: 1016 bytes --]
Hi all,
With the introduction of native records ert lost the ability to peek inside structures when comparing unequal values:
(require 'ert)
(cl-defstruct xyz a b c)
(defvar xyz123 (make-xyz :a 1 :b 2 :c 3))
(defvar xyz143 (make-xyz :a 1 :b 4 :c 3))
(should (equal xyz123 xyz143))
Emacs 25 said this:
Test failed: ((should (equal xyz123 xyz143))
:form (equal [cl-struct-xyz 1 2 3 nil] [cl-struct-xyz 1 4 3 nil])
:value nil
:explanation (array-elt 2 (different-atoms (2 "#x2" "?\x02") (4 "#x4" "?\x04"))))
Emacs 26 says this:
Test failed: ((should (equal xyz123 xyz143))
:form (equal #s(xyz 1 2 3) #s(xyz 1 4 3))
:value nil
:explanation (different-atoms #s(xyz 1 2 3) #s(xyz 1 4 3)))
The first attached patch fixes this for all records. The second patch adds special support for cl-structs, to get this:
Test failed: ((should (equal xyz123 xyz143))
:form (equal #s(xyz 1 2 3) #s(xyz 1 4 3))
:value nil
:explanation (struct-field c (different-atoms (2 "#x2" "?\x02") (4 "#x4" "?\x04"))))
Clément.
[-- Attachment #2: 0001-lisp-emacs-lisp-ert.el-ert-explain-equal-rec-Treat-r.patch --]
[-- Type: text/x-patch, Size: 942 bytes --]
From 1f9936f1f1f8428b9ddebec04a4a865719543cf4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pitclaudel@live.com>
Date: Sat, 11 Apr 2020 16:22:56 -0400
Subject: [PATCH 1/2] * lisp/emacs-lisp/ert.el (ert--explain-equal-rec): Treat
records as arrays
---
lisp/emacs-lisp/ert.el | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 764354b03b..74202183cc 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -515,7 +515,7 @@ ert--explain-equal-rec
`(cdr ,cdr-x)
(cl-assert (equal a b) t)
nil))))))))
- ((pred arrayp)
+ ((or (pred arrayp) (pred recordp))
;; For mixed unibyte/multibyte string comparisons, make both multibyte.
(when (and (stringp a)
(xor (multibyte-string-p a) (multibyte-string-p b)))
--
2.17.1
[-- Attachment #3: 0002-lisp-emacs-lisp-ert.el-ert-explain-equal-rec-Add-sup.patch --]
[-- Type: text/x-patch, Size: 1189 bytes --]
From 47eb414118ab41b32c613fcdbe86369b2efd4c56 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pitclaudel@live.com>
Date: Sat, 11 Apr 2020 16:23:47 -0400
Subject: [PATCH 2/2] * lisp/emacs-lisp/ert.el (ert--explain-equal-rec): Add
support for cl-structs
---
lisp/emacs-lisp/ert.el | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 74202183cc..e3666a84c0 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -515,6 +515,13 @@ ert--explain-equal-rec
`(cdr ,cdr-x)
(cl-assert (equal a b) t)
nil))))))))
+ ((pred cl-struct-p)
+ (cl-loop for slot in (cdr (cl-struct-slot-info (type-of a)))
+ for ai across a
+ for bi across b
+ for xf = (ert--explain-equal-rec ai bi)
+ do (when xf (cl-return `(struct-field ,(car slot) ,xf)))
+ finally (cl-assert (equal a b) t)))
((or (pred arrayp) (pred recordp))
;; For mixed unibyte/multibyte string comparisons, make both multibyte.
(when (and (stringp a)
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#40562: [patch] Treat records as arrays in ert object comparisons and add support for cl-structs
2020-04-11 20:34 bug#40562: [patch] Treat records as arrays in ert object comparisons and add support for cl-structs Clément Pit-Claudel
@ 2020-04-11 23:26 ` Štěpán Němec
2020-04-12 3:06 ` Clément Pit-Claudel
0 siblings, 1 reply; 8+ messages in thread
From: Štěpán Němec @ 2020-04-11 23:26 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: lars, 40562
On Sat, 11 Apr 2020 16:34:57 -0400
Clément Pit-Claudel wrote:
[...]
> (defvar xyz123 (make-xyz :a 1 :b 2 :c 3))
> (defvar xyz143 (make-xyz :a 1 :b 4 :c 3))
[...]
> The first attached patch fixes this for all records.
Seems like a nice addition.
> Test failed: ((should (equal xyz123 xyz143))
> :form (equal #s(xyz 1 2 3) #s(xyz 1 4 3))
> :value nil
> :explanation (struct-field c (different-atoms (2 "#x2" "?") (4 "#x4" "?"))))
^gotcha
> lisp/emacs-lisp/ert.el | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
> index 74202183cc..e3666a84c0 100644
> --- a/lisp/emacs-lisp/ert.el
> +++ b/lisp/emacs-lisp/ert.el
> @@ -515,6 +515,13 @@ ert--explain-equal-rec
> `(cdr ,cdr-x)
> (cl-assert (equal a b) t)
> nil))))))))
> + ((pred cl-struct-p)
> + (cl-loop for slot in (cdr (cl-struct-slot-info (type-of a)))
> + for ai across a
^^^^^^^^^^^^^^^
This is incorrect, as witnessed by your very example (`c' instead of
`b'). Records are accessible with `aref', but the first slot is the type
descriptor, so you're making an off-by-one error here.
> + for bi across b
> + for xf = (ert--explain-equal-rec ai bi)
> + do (when xf (cl-return `(struct-field ,(car slot) ,xf)))
> + finally (cl-assert (equal a b) t)))
> ((or (pred arrayp) (pred recordp))
> ;; For mixed unibyte/multibyte string comparisons, make both multibyte.
> (when (and (stringp a)
Thanks,
Štěpán
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#40562: [patch] Treat records as arrays in ert object comparisons and add support for cl-structs
2020-04-11 23:26 ` Štěpán Němec
@ 2020-04-12 3:06 ` Clément Pit-Claudel
2020-04-12 9:24 ` Štěpán Němec
0 siblings, 1 reply; 8+ messages in thread
From: Clément Pit-Claudel @ 2020-04-12 3:06 UTC (permalink / raw)
To: Štěpán Němec; +Cc: lars, 40562
[-- Attachment #1: Type: text/plain, Size: 429 bytes --]
On 11/04/2020 19.26, Štěpán Němec wrote:
> This is incorrect, as witnessed by your very example (`c' instead of
> `b'). Records are accessible with `aref', but the first slot is the type
> descriptor, so you're making an off-by-one error here.
Of course, it should be `for slot in (cl-struct-slot-info (type-of a))` not `for slot in (cdr (cl-struct-slot-info (type-of a)))`. Updated patch attached.
Thanks for the review!
[-- Attachment #2: 0002-lisp-emacs-lisp-ert.el-ert-explain-equal-rec-Add-sup.patch --]
[-- Type: text/x-patch, Size: 1183 bytes --]
From 129ba8aeaa91b6ea44a325ec237f3df7e9400c9f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pitclaudel@live.com>
Date: Sat, 11 Apr 2020 16:23:47 -0400
Subject: [PATCH 2/2] * lisp/emacs-lisp/ert.el (ert--explain-equal-rec): Add
support for cl-structs
---
lisp/emacs-lisp/ert.el | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 74202183cc..241eece05b 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -515,6 +515,13 @@ ert--explain-equal-rec
`(cdr ,cdr-x)
(cl-assert (equal a b) t)
nil))))))))
+ ((pred cl-struct-p)
+ (cl-loop for slot in (cl-struct-slot-info (type-of a))
+ for ai across a
+ for bi across b
+ for xf = (ert--explain-equal-rec ai bi)
+ do (when xf (cl-return `(struct-field ,(car slot) ,xf)))
+ finally (cl-assert (equal a b) t)))
((or (pred arrayp) (pred recordp))
;; For mixed unibyte/multibyte string comparisons, make both multibyte.
(when (and (stringp a)
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#40562: [patch] Treat records as arrays in ert object comparisons and add support for cl-structs
2020-04-12 3:06 ` Clément Pit-Claudel
@ 2020-04-12 9:24 ` Štěpán Němec
2020-04-12 15:50 ` Clément Pit-Claudel
0 siblings, 1 reply; 8+ messages in thread
From: Štěpán Němec @ 2020-04-12 9:24 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: lars, 40562
On Sat, 11 Apr 2020 23:06:47 -0400
Clément Pit-Claudel wrote:
> On 11/04/2020 19.26, Štěpán Němec wrote:
>> This is incorrect, as witnessed by your very example (`c' instead of
>> `b'). Records are accessible with `aref', but the first slot is the type
>> descriptor, so you're making an off-by-one error here.
>
> Of course, it should be `for slot in (cl-struct-slot-info (type-of
> a))` not `for slot in (cdr (cl-struct-slot-info (type-of a)))`.
> Updated patch attached.
Hm, except now you're duplicating the (eq (type-of a) (type-of b))
check. I'm not quite sure it's worth complicating the code to avoid
that, though, given how clumsy records currently seem to be to work with
(most sequence functions don't support them).
--
Štěpán
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#40562: [patch] Treat records as arrays in ert object comparisons and add support for cl-structs
2020-04-12 9:24 ` Štěpán Němec
@ 2020-04-12 15:50 ` Clément Pit-Claudel
2020-04-25 21:27 ` Clément Pit-Claudel
0 siblings, 1 reply; 8+ messages in thread
From: Clément Pit-Claudel @ 2020-04-12 15:50 UTC (permalink / raw)
To: Štěpán Němec; +Cc: lars, 40562
On 12/04/2020 05.24, Štěpán Němec wrote:
> On Sat, 11 Apr 2020 23:06:47 -0400
> Clément Pit-Claudel wrote:
>
>> On 11/04/2020 19.26, Štěpán Němec wrote:
>>> This is incorrect, as witnessed by your very example (`c' instead of
>>> `b'). Records are accessible with `aref', but the first slot is the type
>>> descriptor, so you're making an off-by-one error here.
>>
>> Of course, it should be `for slot in (cl-struct-slot-info (type-of
>> a))` not `for slot in (cdr (cl-struct-slot-info (type-of a)))`.
>> Updated patch attached.
>
> Hm, except now you're duplicating the (eq (type-of a) (type-of b))
> check.
Yup; I don't think that's a problem :)
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#40562: [patch] Treat records as arrays in ert object comparisons and add support for cl-structs
2020-04-12 15:50 ` Clément Pit-Claudel
@ 2020-04-25 21:27 ` Clément Pit-Claudel
2020-08-08 13:02 ` Lars Ingebrigtsen
0 siblings, 1 reply; 8+ messages in thread
From: Clément Pit-Claudel @ 2020-04-25 21:27 UTC (permalink / raw)
To: Štěpán Němec; +Cc: lars, 40562
On 12/04/2020 11.50, Clément Pit-Claudel wrote:
> On 12/04/2020 05.24, Štěpán Němec wrote:
>> On Sat, 11 Apr 2020 23:06:47 -0400
>> Clément Pit-Claudel wrote:
>>
>>> On 11/04/2020 19.26, Štěpán Němec wrote:
>>>> This is incorrect, as witnessed by your very example (`c' instead of
>>>> `b'). Records are accessible with `aref', but the first slot is the type
>>>> descriptor, so you're making an off-by-one error here.
>>>
>>> Of course, it should be `for slot in (cl-struct-slot-info (type-of
>>> a))` not `for slot in (cdr (cl-struct-slot-info (type-of a)))`.
>>> Updated patch attached.
>>
>> Hm, except now you're duplicating the (eq (type-of a) (type-of b))
>> check.
>
> Yup; I don't think that's a problem :)
Is there anything else I should do before pushing this patch?
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#40562: [patch] Treat records as arrays in ert object comparisons and add support for cl-structs
2020-04-25 21:27 ` Clément Pit-Claudel
@ 2020-08-08 13:02 ` Lars Ingebrigtsen
2020-08-18 14:07 ` Lars Ingebrigtsen
0 siblings, 1 reply; 8+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-08 13:02 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: lars, 40562, Štěpán Němec
Clément Pit-Claudel <cpitclaudel@gmail.com> writes:
> Is there anything else I should do before pushing this patch?
This was four months ago, so I guess you should go ahead and push the
patch.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#40562: [patch] Treat records as arrays in ert object comparisons and add support for cl-structs
2020-08-08 13:02 ` Lars Ingebrigtsen
@ 2020-08-18 14:07 ` Lars Ingebrigtsen
0 siblings, 0 replies; 8+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-18 14:07 UTC (permalink / raw)
To: Clément Pit-Claudel; +Cc: lars, 40562, Štěpán Němec
Lars Ingebrigtsen <larsi@gnus.org> writes:
> Clément Pit-Claudel <cpitclaudel@gmail.com> writes:
>
>> Is there anything else I should do before pushing this patch?
>
> This was four months ago, so I guess you should go ahead and push the
> patch.
That was a week ago, so I went ahead and pushed the patch to Emacs 28.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-08-18 14:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-11 20:34 bug#40562: [patch] Treat records as arrays in ert object comparisons and add support for cl-structs Clément Pit-Claudel
2020-04-11 23:26 ` Štěpán Němec
2020-04-12 3:06 ` Clément Pit-Claudel
2020-04-12 9:24 ` Štěpán Němec
2020-04-12 15:50 ` Clément Pit-Claudel
2020-04-25 21:27 ` Clément Pit-Claudel
2020-08-08 13:02 ` Lars Ingebrigtsen
2020-08-18 14:07 ` Lars Ingebrigtsen
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).