GNU bug report logs - #40562
[patch] Treat records as arrays in ert object comparisons and add support for cl-structs

Previous Next

Package: emacs;

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.

View this report as an mbox folder, status mbox, maintainer mbox


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):

From: Clément Pit-Claudel <cpitclaudel <at> gmail.com>
To: bug-gnu-emacs <bug-gnu-emacs <at> gnu.org>
Subject: [patch] Treat records as arrays in ert object comparisons and add
 support for cl-structs
Date: Sat, 11 Apr 2020 16:34:57 -0400
[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):

From: Štěpán Němec <stepnem <at> gmail.com>
To: Clément Pit-Claudel <cpitclaudel <at> gmail.com>
Cc: lars <lars <at> nocrew.org>, 40562 <at> debbugs.gnu.org
Subject: Re: bug#40562: [patch] Treat records as arrays in ert object
 comparisons and add support for cl-structs
Date: Sun, 12 Apr 2020 01:26:41 +0200
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):

From: Clément Pit-Claudel <cpitclaudel <at> gmail.com>
To: Štěpán Němec <stepnem <at> gmail.com>
Cc: lars <lars <at> nocrew.org>, 40562 <at> debbugs.gnu.org
Subject: Re: bug#40562: [patch] Treat records as arrays in ert object
 comparisons and add support for cl-structs
Date: Sat, 11 Apr 2020 23:06:47 -0400
[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):

From: Štěpán Němec <stepnem <at> gmail.com>
To: Clément Pit-Claudel <cpitclaudel <at> gmail.com>
Cc: lars <lars <at> nocrew.org>, 40562 <at> debbugs.gnu.org
Subject: Re: bug#40562: [patch] Treat records as arrays in ert object
 comparisons and add support for cl-structs
Date: Sun, 12 Apr 2020 11:24:41 +0200
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):

From: Clément Pit-Claudel <cpitclaudel <at> gmail.com>
To: Štěpán Němec <stepnem <at> gmail.com>
Cc: lars <lars <at> nocrew.org>, 40562 <at> debbugs.gnu.org
Subject: Re: bug#40562: [patch] Treat records as arrays in ert object
 comparisons and add support for cl-structs
Date: Sun, 12 Apr 2020 11:50:44 -0400
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):

From: Clément Pit-Claudel <cpitclaudel <at> gmail.com>
To: Štěpán Němec <stepnem <at> gmail.com>
Cc: lars <lars <at> nocrew.org>, 40562 <at> debbugs.gnu.org
Subject: Re: bug#40562: [patch] Treat records as arrays in ert object
 comparisons and add support for cl-structs
Date: Sat, 25 Apr 2020 17:27:53 -0400
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):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Clément Pit-Claudel <cpitclaudel <at> gmail.com>
Cc: lars <lars <at> nocrew.org>, 40562 <at> debbugs.gnu.org,
 Štěpán Němec <stepnem <at> gmail.com>
Subject: Re: bug#40562: [patch] Treat records as arrays in ert object
 comparisons and add support for cl-structs
Date: Sat, 08 Aug 2020 15:02:01 +0200
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):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Clément Pit-Claudel <cpitclaudel <at> gmail.com>
Cc: lars <lars <at> nocrew.org>, 40562 <at> debbugs.gnu.org,
 Štěpán Němec <stepnem <at> gmail.com>
Subject: Re: bug#40562: [patch] Treat records as arrays in ert object
 comparisons and add support for cl-structs
Date: Tue, 18 Aug 2020 16:07:10 +0200
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.