GNU bug report logs -
#40562
[patch] Treat records as arrays in ert object comparisons and add support for cl-structs
Previous Next
Reported by: Clément Pit-Claudel <cpitclaudel <at> gmail.com>
Date: Sat, 11 Apr 2020 20:36:01 UTC
Severity: normal
Tags: fixed, patch
Fixed in version 28.1
Done: Lars Ingebrigtsen <larsi <at> gnus.org>
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 40562 in the body.
You can then email your comments to 40562 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
lars <at> nocrew.org, bug-gnu-emacs <at> gnu.org
:
bug#40562
; Package
emacs
.
(Sat, 11 Apr 2020 20:36:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Clément Pit-Claudel <cpitclaudel <at> gmail.com>
:
New bug report received and forwarded. Copy sent to
lars <at> nocrew.org, bug-gnu-emacs <at> gnu.org
.
(Sat, 11 Apr 2020 20:36:01 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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" "?") (4 "#x4" "?"))))
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" "?") (4 "#x4" "?"))))
Clément.
[0001-lisp-emacs-lisp-ert.el-ert-explain-equal-rec-Treat-r.patch (text/x-patch, attachment)]
[0002-lisp-emacs-lisp-ert.el-ert-explain-equal-rec-Add-sup.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40562
; Package
emacs
.
(Sat, 11 Apr 2020 23:27:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 40562 <at> debbugs.gnu.org (full text, mbox):
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
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40562
; Package
emacs
.
(Sun, 12 Apr 2020 03:07:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 40562 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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!
[0002-lisp-emacs-lisp-ert.el-ert-explain-equal-rec-Add-sup.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40562
; Package
emacs
.
(Sun, 12 Apr 2020 09:25:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 40562 <at> debbugs.gnu.org (full text, mbox):
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
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40562
; Package
emacs
.
(Sun, 12 Apr 2020 15:51:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 40562 <at> debbugs.gnu.org (full text, mbox):
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 :)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40562
; Package
emacs
.
(Sat, 25 Apr 2020 21:29:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 40562 <at> debbugs.gnu.org (full text, mbox):
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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40562
; Package
emacs
.
(Sat, 08 Aug 2020 13:03:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 40562 <at> debbugs.gnu.org (full text, mbox):
Clément Pit-Claudel <cpitclaudel <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40562
; Package
emacs
.
(Tue, 18 Aug 2020 14:08:01 GMT)
Full text and
rfc822 format available.
Message #26 received at 40562 <at> debbugs.gnu.org (full text, mbox):
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> Clément Pit-Claudel <cpitclaudel <at> 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
Added tag(s) fixed.
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org
.
(Tue, 18 Aug 2020 14:08:02 GMT)
Full text and
rfc822 format available.
bug marked as fixed in version 28.1, send any further explanations to
40562 <at> debbugs.gnu.org and Clément Pit-Claudel <cpitclaudel <at> gmail.com>
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org
.
(Tue, 18 Aug 2020 14:08: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
.
(Wed, 16 Sep 2020 11:24:08 GMT)
Full text and
rfc822 format available.
This bug report was last modified 3 years and 193 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.