GNU bug report logs - #6581
24.0.50; `equal-including-properties' uses `eq' to compare property values

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 6581 in the body.
You can then email your comments to 6581 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christian Ohler <ohler+emacs <at> fastmail.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.0.50; `equal-including-properties' uses `eq' to compare property
	values
Date: Thu, 08 Jul 2010 00:19:49 +1000

I just noticed that, in my current Emacs (24.0.50.1, built from trunk a 
few weeks ago):

(equal-including-properties #("a" 0 1 (k "v")) #("a" 0 1 (k "v")))
=> nil

This is because these two strings both have a property with key `k', but 
the values are two different strings "v", and 
`equal-including-properties' appears to use `eq' to compare property 
values (I didn't verify this in the code).  Is this a bug, or is it 
intentional?

For comparison,

(equal-including-properties #("a" 0 1 (k v)) #("a" 0 1 (k v)))
=> t

since `v' is an interned symbol.

My expectation was that, since `equal' is recursively defined in terms 
of itself, `equal-including-properties' should also call itself 
recursively on property values.

Christian.

Message #8 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Christian Ohler <ohler+emacs <at> fastmail.net>
Cc: 6581 <at> debbugs.gnu.org
Subject: Re: bug#6581: 24.0.50;
 `equal-including-properties' uses `eq' to compare property values
Date: Tue, 13 Jan 2015 05:01:33 +0300

I've just hit this bug today.  Half an hour wasted.

Message #11 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>, Christian Ohler <ohler+emacs <at> fastmail.net>
Cc: 6581 <at> debbugs.gnu.org
Subject: RE: bug#6581: 24.0.50; `equal-including-properties' uses `eq' to
 compare property values
Date: Mon, 12 Jan 2015 18:23:04 -0800 (PST)

> I've just hit this bug today.  Half an hour wasted.

There certainly is *at least* a doc bug.  Users of such
a function at least need to be told how the text property
values are compared.  That's a minimum for the doc of any
equality predicate: say just what equality means in this
case.

Both the doc string and the Elisp manual (node `Equality
Predicates') are deficient.

In addition, a reasonable enhancement might be (now) to
accept an optional comparison (equality) predicate,
especially to be able to work around the designed behavior.

But I'm guessing that the default behavior could/should
not be changed at this point, because existing code
could break.

Just call out the *actual behavior*, clearly and completely.
Even LOUDLY, as this is truly a gotcha.  (And one needs to
check C code to see what the function really does.)

This predicate was added after Emacs 20 (after 21 perhaps?).
It's a bit surprising that no one has thought of completing
the doc - at least.

Message #14 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Drew Adams <drew.adams <at> oracle.com>,
 Christian Ohler <ohler+emacs <at> fastmail.net>
Cc: 6581 <at> debbugs.gnu.org
Subject: Re: bug#6581: 24.0.50; `equal-including-properties' uses `eq'
 to compare property values
Date: Tue, 13 Jan 2015 05:27:01 +0300

On 01/13/2015 05:23 AM, Drew Adams wrote:

> There certainly is *at least* a doc bug.

Yup.

Message #17 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 6581 <at> debbugs.gnu.org, Christian Ohler <ohler+emacs <at> fastmail.net>
Subject: Re: bug#6581: 24.0.50;
 `equal-including-properties' uses `eq' to compare property values
Date: Tue, 13 Jan 2015 10:23:19 -0500

> I've just hit this bug today.  Half an hour wasted.

The intention is to use `equal-including-properties' when
comparing the property values.


        Stefan

Message #20 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 6581 <at> debbugs.gnu.org, Christian Ohler <ohler+emacs <at> fastmail.net>
Subject: RE: bug#6581: 24.0.50; `equal-including-properties' uses `eq' to
 compare property values
Date: Tue, 13 Jan 2015 07:33:51 -0800 (PST)

> > I've just hit this bug today.  Half an hour wasted.
> 
> The intention is to use `equal-including-properties' when
> comparing the property values.

Whatever the intended use is, the behavior should be specified
in the predicate's doc.

Message #23 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 6581 <at> debbugs.gnu.org, Christian Ohler <ohler+emacs <at> fastmail.net>
Subject: Re: bug#6581: 24.0.50; `equal-including-properties' uses `eq'
 to compare property values
Date: Tue, 13 Jan 2015 18:44:43 +0300

On 01/13/2015 06:23 PM, Stefan Monnier wrote:

> The intention is to use `equal-including-properties' when
> comparing the property values.

So you agree the behavior should be fixed?

Message #26 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 6581 <at> debbugs.gnu.org, Christian Ohler <ohler+emacs <at> fastmail.net>
Subject: Re: bug#6581: 24.0.50;
 `equal-including-properties' uses `eq' to compare property values
Date: Tue, 13 Jan 2015 14:53:22 -0500

>> The intention is to use `equal-including-properties' when
>> comparing the property values.
> So you agree the behavior should be fixed?

Yes.


        Stefan

Message #39 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 6581 <at> debbugs.gnu.org, Christian Ohler <ohler+emacs <at> fastmail.net>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#6581: 24.0.50; `equal-including-properties' uses `eq' to
 compare property values
Date: Wed, 20 Oct 2021 07:49:59 -0700

[Message part 1 (text/plain, inline)]

tags 6581 + patch
thanks

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>> The intention is to use `equal-including-properties' when
>>> comparing the property values.
>> So you agree the behavior should be fixed?
>
> Yes.

The attached patch should fix it.

[0001-Fix-bug-with-string-values-in-equal-including-proper.patch (text/x-diff, attachment)]

Message #44 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 6581 <at> debbugs.gnu.org, Christian Ohler <ohler+emacs <at> fastmail.net>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#6581: 24.0.50; `equal-including-properties' uses `eq' to
 compare property values
Date: Thu, 21 Oct 2021 09:06:52 -0400

> The attached patch should fix it.

The patch looks OK tho I can't see why you add `intervals_equal_1` to
intervals.h (instead of marking it static).

> (compare_string_intervals): Call intervals_equal1 with third
> argument as true.

Rather than paraphrase the code change this could say what's the
intended effect of passing that arg.


        Stefan

Message #47 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 6581 <at> debbugs.gnu.org, Christian Ohler <ohler+emacs <at> fastmail.net>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#6581: 24.0.50; `equal-including-properties' uses `eq' to
 compare property values
Date: Thu, 21 Oct 2021 11:05:57 -0700

[Message part 1 (text/plain, inline)]

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> The attached patch should fix it.
>
> The patch looks OK tho I can't see why you add `intervals_equal_1` to
> intervals.h (instead of marking it static).

Me neither, fixed in the attached.  (It was an artifact of some earlier
experimentation that shouldn't have survived.)

>> (compare_string_intervals): Call intervals_equal1 with third
>> argument as true.
>
> Rather than paraphrase the code change this could say what's the
> intended effect of passing that arg.

I'm not a fan of the GNU ChangeLog format, as it seems to encourage bad
habits like not useful descriptions and massive patches (as opposed to
patch sets).  In any case, I've made an attempt to force a useful
description into this clunky format in the attached.

I also see that ert had an inefficient re-implementation of
'equal-including-properties' lying around as a workaround for this bug,
so I've fixed that up in a second patch.

I hope that Eli or Lars could say if they prefer the first patch (with
the bug fix) on emacs-28 or master.

The second patch is not urgent and should definitely go to master.

[0001-Fix-bug-with-string-values-in-equal-including-proper.patch (text/x-diff, attachment)]

[0002-Remove-workaround-for-fixed-Bug-6541-from-ert.patch (text/x-diff, attachment)]

Message #50 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 6581 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, ohler+emacs <at> fastmail.net,
 dgutov <at> yandex.ru
Subject: Re: bug#6581: 24.0.50;
 `equal-including-properties' uses `eq' to compare property values
Date: Thu, 21 Oct 2021 21:15:39 +0300

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Thu, 21 Oct 2021 11:05:57 -0700
> Cc: 6581 <at> debbugs.gnu.org, Christian Ohler <ohler+emacs <at> fastmail.net>,
>  Dmitry Gutov <dgutov <at> yandex.ru>
> 
> I hope that Eli or Lars could say if they prefer the first patch (with
> the bug fix) on emacs-28 or master.

On master, please.  It's not really a bugfix, it's a kind-of
enhancement, and the change is not trivial.

Thanks.

Message #53 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 6581 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, ohler+emacs <at> fastmail.net,
 dgutov <at> yandex.ru
Subject: Re: bug#6581: 24.0.50; `equal-including-properties' uses `eq' to
 compare property values
Date: Thu, 21 Oct 2021 11:36:40 -0700

Eli Zaretskii <eliz <at> gnu.org> writes:

> On master, please.  It's not really a bugfix, it's a kind-of
> enhancement, and the change is not trivial.

That's fine by me, thank you.

Message #56 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 6581 <at> debbugs.gnu.org, Christian Ohler <ohler+emacs <at> fastmail.net>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#6581: 24.0.50; `equal-including-properties' uses `eq' to
 compare property values
Date: Thu, 21 Oct 2021 15:42:05 -0400

> +	  || (!use_equal && !EQ (XCAR (i1_val), XCAR (i0_cdr)))
> +	  || (use_equal && NILP (Fequal (XCAR (i1_val), XCAR (i0_cdr)))))

use_equal ? NILP (Fequal (XCAR (i1_val), XCAR (i0_cdr)))
          : !EQ (XCAR (i1_val), XCAR (i0_cdr))


-- Stefan

Message #61 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 6581 <at> debbugs.gnu.org, Christian Ohler <ohler+emacs <at> fastmail.net>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#6581: 24.0.50; `equal-including-properties' uses `eq' to
 compare property values
Date: Thu, 21 Oct 2021 14:02:46 -0700

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> +	  || (!use_equal && !EQ (XCAR (i1_val), XCAR (i0_cdr)))
>> +	  || (use_equal && NILP (Fequal (XCAR (i1_val), XCAR (i0_cdr)))))
>
> use_equal ? NILP (Fequal (XCAR (i1_val), XCAR (i0_cdr)))
>           : !EQ (XCAR (i1_val), XCAR (i0_cdr))

Yes, that's better.

Message #64 received at 6581 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 6581 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, ohler+emacs <at> fastmail.net,
 dgutov <at> yandex.ru
Subject: Re: bug#6581: 24.0.50; `equal-including-properties' uses `eq' to
 compare property values
Date: Sat, 30 Oct 2021 19:25:32 -0700

close 6581 29.1
thanks

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Stefan Kangas <stefan <at> marxist.se>
>> Date: Thu, 21 Oct 2021 11:05:57 -0700
>> Cc: 6581 <at> debbugs.gnu.org, Christian Ohler <ohler+emacs <at> fastmail.net>,
>>  Dmitry Gutov <dgutov <at> yandex.ru>
>>
>> I hope that Eli or Lars could say if they prefer the first patch (with
>> the bug fix) on emacs-28 or master.
>
> On master, please.  It's not really a bugfix, it's a kind-of
> enhancement, and the change is not trivial.

No further comments within almost 10 days, now pushed to master (commit
8227d1273e and 54b8ec4e6f).

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.