GNU bug report logs - #34852
26.1; seq-intersection ignores nil as element

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 34852 in the body.
You can then email your comments to 34852 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: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.1; seq-intersection ignores nil as element
Date: Wed, 13 Mar 2019 23:16:30 -0300

Hi.

seq-intersection skips nil as common element, so returns wrong result.

Reproducing from emacs -Q:

In *scratch* buffer, eval the expressions

(progn
  (require 'seq)
  (seq-intersection '(1 2 nil) '(1 nil) 'eq))

-> (1)

(seq-intersection '(1 2 nil) '(1 nil) 'equal)

-> (1)

Expected result in both cases: (1 nil)

Cheers

Miguel Frasson
--

In GNU Emacs 26.1 (build 1, i686-w64-mingw32)
 of 2018-05-30 built on CIRROCUMULUS
Repository revision: 07f8f9bc5a51f5aa94eb099f3e15fbe0c20ea1ea
Windowing system distributor 'Microsoft Corp.', version 10.0.17134
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
seq
Configured using:
 'configure --without-dbus --host=i686-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS THREADS LCMS2

Important settings:
  value of $LANG: PTB
  locale-coding-system: cp1252

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib elec-pair time-date
mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel dos-w32 ls-lisp disp-table term/w32-win w32-win
w32-vars term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote w32notify w32 lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 8 96961 9032)
 (symbols 32 20186 1)
 (miscs 32 38 140)
 (strings 16 29656 2096)
 (string-bytes 1 766583)
 (vectors 12 14052)
 (vector-slots 4 507930 10192)
 (floats 8 52 289)
 (intervals 28 263 59)
 (buffers 536 12))


-- 
Miguel Vinicius Santini Frasson
mvsfrasson <at> gmail.com

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Cc: 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 13:07:19 +0100

"Miguel V. S. Frasson" <mvsfrasson <at> gmail.com> writes:

> seq-intersection skips nil as common element, so returns wrong result.
>
> Reproducing from emacs -Q:
>
> In *scratch* buffer, eval the expressions
>
> (progn
>   (require 'seq)
>   (seq-intersection '(1 2 nil) '(1 nil) 'eq))
>
> -> (1)
>
> (seq-intersection '(1 2 nil) '(1 nil) 'equal)
>
> -> (1)
>
> Expected result in both cases: (1 nil)

Indeed.

Nicolas, can you have a look?  We probably can't use `seq-contains' in
the implementation.


Thanks,

Michael.

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Cc: Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 12:22:40 +0000

[0001-Do-not-use-seq-contains-as-a-predicate-bug-34852.patch (text/x-diff, attachment)]

[0001-Return-boolean-instead-of-element-in-seq-contains.patch (text/x-diff, attachment)]

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

"Miguel V. S. Frasson" <mvsfrasson <at> gmail.com> writes:

> seq-intersection skips nil as common element, so returns wrong result.
>
> Reproducing from emacs -Q:
>
> In *scratch* buffer, eval the expressions
>
> (progn
>   (require 'seq)
>   (seq-intersection '(1 2 nil) '(1 nil) 'eq))
>
> -> (1)
>
> (seq-intersection '(1 2 nil) '(1 nil) 'equal)
>
> -> (1)
>
> Expected result in both cases: (1 nil)

This is actually due to seq-contains returning the element found, rather
than a boolean indicating whether the element was found:

(seq-contains '(nil) nil) ; => nil

The nature of seq-contains as a predicate-ish has been discussed in the
past[1], and Stefan recently fixed a similar problem with
map-contains-key[2].

[1]: https://lists.gnu.org/archive/html/emacs-devel/2016-07/msg01256.html
[2]: * lisp/emacs-lisp/map.el: Make the functions generic
  2018-12-11 17:54:13 -0500
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=1691a51094d35ac4b2c311fa407c6b77eea7a105

One solution is to leave seq-contains as it is, and switch to using
seq-position (or some new predicate) as a predicate instead.  Another is
to make seq-contains return a boolean instead of the needle found, which
would be a backward-incompatible change similar to that for
map-contains-key.  I attach a patch for each of these respective
solutions; WDYT?

-- 
Basil

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

From: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 09:52:06 -0300

Hi.
In https://lists.gnu.org/archive/html/emacs-devel/2016-07/msg01256.html
it is mentioned that:

>    I believe `seq-contains' was introduced for convenience:
>    it is used elsewhere in seq.el and map.el.

so there may be problems in map.el as well if it is used as a
predicate there, or even in more files.

Miguel.

Em qui, 14 de mar de 2019 às 09:22, Basil L. Contovounesios
<contovob <at> tcd.ie> escreveu:
>
>
> "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com> writes:
>
> > seq-intersection skips nil as common element, so returns wrong result.
> >
> > Reproducing from emacs -Q:
> >
> > In *scratch* buffer, eval the expressions
> >
> > (progn
> >   (require 'seq)
> >   (seq-intersection '(1 2 nil) '(1 nil) 'eq))
> >
> > -> (1)
> >
> > (seq-intersection '(1 2 nil) '(1 nil) 'equal)
> >
> > -> (1)
> >
> > Expected result in both cases: (1 nil)
>
> This is actually due to seq-contains returning the element found, rather
> than a boolean indicating whether the element was found:
>
> (seq-contains '(nil) nil) ; => nil
>
> The nature of seq-contains as a predicate-ish has been discussed in the
> past[1], and Stefan recently fixed a similar problem with
> map-contains-key[2].
>
> [1]: https://lists.gnu.org/archive/html/emacs-devel/2016-07/msg01256.html
> [2]: * lisp/emacs-lisp/map.el: Make the functions generic
>   2018-12-11 17:54:13 -0500
>   https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=1691a51094d35ac4b2c311fa407c6b77eea7a105
>
> One solution is to leave seq-contains as it is, and switch to using
> seq-position (or some new predicate) as a predicate instead.  Another is
> to make seq-contains return a boolean instead of the needle found, which
> would be a backward-incompatible change similar to that for
> map-contains-key.  I attach a patch for each of these respective
> solutions; WDYT?
>
> --
> Basil



-- 
Miguel Vinicius Santini Frasson
mvsfrasson <at> gmail.com

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 14:09:15 +0100

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

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> One solution is to leave seq-contains as it is, and switch to using
> seq-position (or some new predicate) as a predicate instead.  Another is
> to make seq-contains return a boolean instead of the needle found, which
> would be a backward-incompatible change similar to that for
> map-contains-key.  I attach a patch for each of these respective
> solutions; WDYT?

We also have `seq-some' which can be used as a contain predicate,
e.g. to fix this bug:

[0001-Fix-seq-intersection-with-nil.patch (text/x-diff, inline)]

From c53a80c29e696ab64d4279ca6f495c8e0e1b16b4 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen <at> web.de>
Date: Thu, 14 Mar 2019 13:55:41 +0100
Subject: [PATCH] Fix seq-intersection with nil

---
 lisp/emacs-lisp/seq.el | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index 4a811d7895..5718343a8f 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -409,12 +409,13 @@ seq-sort-by
 (cl-defgeneric seq-intersection (sequence1 sequence2 &optional testfn)
   "Return a list of the elements that appear in both SEQUENCE1 and SEQUENCE2.
 Equality is defined by TESTFN if non-nil or by `equal' if nil."
-  (seq-reduce (lambda (acc elt)
-                (if (seq-contains sequence2 elt testfn)
-                    (cons elt acc)
-                  acc))
-              (seq-reverse sequence1)
-              '()))
+  (let ((testfn (or testfn #'equal)))
+    (seq-reduce (lambda (acc elt)
+                  (if (seq-some (apply-partially testfn elt) sequence2)
+                      (cons elt acc)
+                    acc))
+                (seq-reverse sequence1)
+                '())))

 (cl-defgeneric seq-difference (sequence1 sequence2 &optional testfn)
   "Return a list of the elements that appear in SEQUENCE1 but not in SEQUENCE2.
--
2.20.1

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

(probably needed in more places as you did in one of your patches)

So I think we don't necessarily need something new or a backward
incompatible change.


Michael.

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

From: Nicolas Petton <nicolas <at> petton.fr>
To: Michael Heerdegen <michael_heerdegen <at> web.de>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Cc: 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 14:09:58 +0100

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

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> Indeed.
>
> Nicolas, can you have a look?  We probably can't use `seq-contains' in
> the implementation.

Yes, I will.

Cheers,
Nico

[signature.asc (application/pgp-signature, inline)]

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 09:34:07 -0400

Yet another approach might be to make an exception in seq-contains if
the returned element is nil (and return something else in that case).
E.g.

    diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
    index 4a811d7895..d2398eb588 100644
    --- a/lisp/emacs-lisp/seq.el
    +++ b/lisp/emacs-lisp/seq.el
    @@ -360,7 +360,7 @@ seq-sort-by
     Equality is defined by TESTFN if non-nil or by `equal' if nil."
       (seq-some (lambda (e)
                   (when (funcall (or testfn #'equal) elt e)
    -                e))
    +                (or e t)))
                 sequence))
     
     (cl-defgeneric seq-set-equal-p (sequence1 sequence2 &optional testfn)


-- Stefan

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Cc: Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 16:16:51 +0000

"Miguel V. S. Frasson" <mvsfrasson <at> gmail.com> writes:

> In https://lists.gnu.org/archive/html/emacs-devel/2016-07/msg01256.html
> it is mentioned that:
>
>>    I believe `seq-contains' was introduced for convenience:
>>    it is used elsewhere in seq.el and map.el.
>
> so there may be problems in map.el as well if it is used as a
> predicate there, or even in more files.

All occurences of seq-contains I could find in emacs.git are addressed
in the two patches.

Thanks,

-- 
Basil

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 16:17:28 +0000

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> One solution is to leave seq-contains as it is, and switch to using
>> seq-position (or some new predicate) as a predicate instead.  Another is
>> to make seq-contains return a boolean instead of the needle found, which
>> would be a backward-incompatible change similar to that for
>> map-contains-key.  I attach a patch for each of these respective
>> solutions; WDYT?
>
> We also have `seq-some' which can be used as a contain predicate,
> e.g. to fix this bug:
>
> From c53a80c29e696ab64d4279ca6f495c8e0e1b16b4 Mon Sep 17 00:00:00 2001
> From: Michael Heerdegen <michael_heerdegen <at> web.de>
> Date: Thu, 14 Mar 2019 13:55:41 +0100
> Subject: [PATCH] Fix seq-intersection with nil
>
> ---
>  lisp/emacs-lisp/seq.el | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
> index 4a811d7895..5718343a8f 100644
> --- a/lisp/emacs-lisp/seq.el
> +++ b/lisp/emacs-lisp/seq.el
> @@ -409,12 +409,13 @@ seq-sort-by
>  (cl-defgeneric seq-intersection (sequence1 sequence2 &optional testfn)
>    "Return a list of the elements that appear in both SEQUENCE1 and SEQUENCE2.
>  Equality is defined by TESTFN if non-nil or by `equal' if nil."
> -  (seq-reduce (lambda (acc elt)
> -                (if (seq-contains sequence2 elt testfn)
> -                    (cons elt acc)
> -                  acc))
> -              (seq-reverse sequence1)
> -              '()))
> +  (let ((testfn (or testfn #'equal)))
> +    (seq-reduce (lambda (acc elt)
> +                  (if (seq-some (apply-partially testfn elt) sequence2)
> +                      (cons elt acc)
> +                    acc))
> +                (seq-reverse sequence1)
> +                '())))
>
>  (cl-defgeneric seq-difference (sequence1 sequence2 &optional testfn)
>    "Return a list of the elements that appear in SEQUENCE1 but not in SEQUENCE2.
> --
> 2.20.1
>
>
> (probably needed in more places as you did in one of your patches)
>
> So I think we don't necessarily need something new or a backward
> incompatible change.

My first patch makes an analogous backward-compatible change using the
more efficient seq-position in place of seq-some.

Thanks,

-- 
Basil

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 16:19:03 +0000

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

> Yet another approach might be to make an exception in seq-contains if
> the returned element is nil (and return something else in that case).
> E.g.
>
>     diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
>     index 4a811d7895..d2398eb588 100644
>     --- a/lisp/emacs-lisp/seq.el
>     +++ b/lisp/emacs-lisp/seq.el
>     @@ -360,7 +360,7 @@ seq-sort-by
>      Equality is defined by TESTFN if non-nil or by `equal' if nil."
>        (seq-some (lambda (e)
>                    (when (funcall (or testfn #'equal) elt e)
>     -                e))
>     +                (or e t)))
>                  sequence))
>      
>      (cl-defgeneric seq-set-equal-p (sequence1 sequence2 &optional testfn)

This is both backward-incompatible and inconsistent.  If we agree to
make a backward-incompatible change, I would rather turn seq-contains
into a proper predicate, as per my second patch.  Besides, returning the
element found isn't particularly useful to begin with, as the caller
already has access to that value.

Either way, we can install the backward-compatible first patch (which
uses seq-position in place of seq-contains) to fix this bug, and later
discuss any backward-incompatible changes to seq-contains.

Thanks,

-- 
Basil

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 17:35:08 +0100

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> My first patch makes an analogous backward-compatible change using the
> more efficient seq-position in place of seq-some.

Why is it more efficient?  The implementations are more or less
analogue, with the exception that seq-position additionally increments a
counter.


Michael.

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

From: Nicolas Petton <nicolas <at> petton.fr>
To: Michael Heerdegen <michael_heerdegen <at> web.de>,
 "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 34852 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 17:45:16 +0100

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

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

Hi Michael,

> We also have `seq-some' which can be used as a contain predicate,
> e.g. to fix this bug:

Using seq-contains was wrong, and I think seq-some should have been used
instead, so I like your patch better (it's IMO better to use seq-some
used as a predicate than seq-position).

Unless somebody disagrees, I'll install your patch.

Cheers,
Nico

[signature.asc (application/pgp-signature, inline)]

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 34852 <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>,
 Stefan Monnier <monnier <at> IRO.UMontreal.CA>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 17:45:30 +0100

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Besides, returning the element found isn't particularly useful to
> begin with, as the caller already has access to that value.

Depends on the TESTFN.  You could use seq-contains as a replacement for
alist-get, for example, when you make TESTFN look at the sequence's
car's.  This is more or less the same as using :key in cl-member, so
people might be doing this and there is some change that we break
existing code.


Michael.

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 17:02:22 +0000

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> My first patch makes an analogous backward-compatible change using the
>> more efficient seq-position in place of seq-some.
>
> Why is it more efficient?  The implementations are more or less
> analogue, with the exception that seq-position additionally increments a
> counter.

Because seq-some involves an additional level of function indirection.
This is confirmed by profiling and the attached mini-benchmark, when run
as follows:

emacs -batch -f batch-byte-compile bench.el
emacs -script bench.elc

A call to seq-position is also less verbose than one to seq-some for the
purpose of finding an element of a sequence.

Having said that, the differences are obviously very small.

-- 
Basil

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Nicolas Petton <nicolas <at> petton.fr>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 17:08:55 +0000

Nicolas Petton <nicolas <at> petton.fr> writes:

> Michael Heerdegen <michael_heerdegen <at> web.de> writes:
>
> Hi Michael,
>
>> We also have `seq-some' which can be used as a contain predicate,
>> e.g. to fix this bug:
>
> Using seq-contains was wrong, and I think seq-some should have been used
> instead, so I like your patch better (it's IMO better to use seq-some
> used as a predicate than seq-position).
>
> Unless somebody disagrees, I'll install your patch.

If you go with the seq-some patch then please edit it to avoid using
apply-partially, which is quite inefficient.

The implementations of seq-set-equal-p, seq-uniq, and seq-difference
will also need to be updated, in addition to that of seq-intersection.

-- 
Basil

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 34852 <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>,
 Stefan Monnier <monnier <at> IRO.UMontreal.CA>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 17:14:13 +0000

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> Besides, returning the element found isn't particularly useful to
>> begin with, as the caller already has access to that value.
>
> Depends on the TESTFN.  You could use seq-contains as a replacement for
> alist-get, for example, when you make TESTFN look at the sequence's
> car's.  This is more or less the same as using :key in cl-member, so
> people might be doing this and there is some change that we break
> existing code.

Fair enough, though I'd be interested to see an example of such logic
that can't be written in a less contrived way.

Thanks,

-- 
Basil

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 17:23:02 +0000

[bench.el (application/emacs-lisp, attachment)]

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

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Michael Heerdegen <michael_heerdegen <at> web.de> writes:
>
>> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>>
>>> My first patch makes an analogous backward-compatible change using the
>>> more efficient seq-position in place of seq-some.
>>
>> Why is it more efficient?  The implementations are more or less
>> analogue, with the exception that seq-position additionally increments a
>> counter.
>
> Because seq-some involves an additional level of function indirection.
> This is confirmed by profiling and the attached mini-benchmark, when run
> as follows:
>
> emacs -batch -f batch-byte-compile bench.el
> emacs -script bench.elc

Oops, forgot to attach.

-- 
Basil

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

From: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 16:08:36 -0300

Hi

In any case, of another implementation for seq-intersection or not, I
think that the solution from Stefan should be implemented anyway
because

* it makes seq-contains provide a useful return value when ELT=nil, so
it is a good exception; If ELT=nil, seq-contains currently returns nil
anyway;

* it makes seq-contains become a real predicate function, making it more useful;

* since seq-contains has been used as predicate before, it is
unpredictable which code uses it out of official repositories, so this
fix potentially fixes other code.

Cheers

Miguel


Em qui, 14 de mar de 2019 às 10:34, Stefan Monnier
<monnier <at> iro.umontreal.ca> escreveu:
>
> Yet another approach might be to make an exception in seq-contains if
> the returned element is nil (and return something else in that case).
> E.g.
>
>     diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
>     index 4a811d7895..d2398eb588 100644
>     --- a/lisp/emacs-lisp/seq.el
>     +++ b/lisp/emacs-lisp/seq.el
>     @@ -360,7 +360,7 @@ seq-sort-by
>      Equality is defined by TESTFN if non-nil or by `equal' if nil."
>        (seq-some (lambda (e)
>                    (when (funcall (or testfn #'equal) elt e)
>     -                e))
>     +                (or e t)))
>                  sequence))
>
>      (cl-defgeneric seq-set-equal-p (sequence1 sequence2 &optional testfn)
>
>
> -- Stefan



-- 
Miguel Vinicius Santini Frasson
mvsfrasson <at> gmail.com

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 17:43:16 -0400

> If ELT=nil, seq-contains currently returns nil anyway;

That's true when `testfn` is nil, but not if you provide
your own `testfn`.


        Stefan

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

From: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 20:08:57 -0300

Hi

Em qui, 14 de mar de 2019 18:43, Stefan Monnier
<monnier <at> iro.umontreal.ca> escreveu:
>
> > If ELT=nil, seq-contains currently returns nil anyway;
>
> That's true when `testfn` is nil, but not if you provide
> your own `testfn`.

Not really.

(seq-contains nil '(nil t foo) (lambda (x) t))  ->  nil

It returns *nil* if testfn fails, or *nil* (ELT) if it succeds.

Miguel

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 19:14:23 -0400

> (seq-contains nil '(nil t foo) (lambda (x) t))  ->  nil
> It returns *nil* if testfn fails, or *nil* (ELT) if it succeds.

Just because it sometimes does doesn't mean it always does:

    (seq-contains '(0 3 4) 1 #'<)
    
returns 3


        Stefan

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Nicolas Petton <nicolas <at> petton.fr>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Fri, 15 Mar 2019 00:15:20 +0100

"Miguel V. S. Frasson" <mvsfrasson <at> gmail.com> writes:

> Hi
>
> In any case, of another implementation for seq-intersection or not, I
> think that the solution from Stefan should be implemented anyway
> because
>
> * it makes seq-contains provide a useful return value when ELT=nil, so
> it is a good exception; If ELT=nil, seq-contains currently returns nil
> anyway;
>
> * it makes seq-contains become a real predicate function, making it
> more useful;
>
> * since seq-contains has been used as predicate before, it is
> unpredictable which code uses it out of official repositories, so this
> fix potentially fixes other code.

BTW, another (alternative) alternative would be to make it return
(list ELT).  That would be a bit less backward-compatible, but a
bit more consistent.

Michael.

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

From: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 20:21:41 -0300

You are right.

Em qui, 14 de mar de 2019 às 20:14, Stefan Monnier
<monnier <at> iro.umontreal.ca> escreveu:
>
> > (seq-contains nil '(nil t foo) (lambda (x) t))  ->  nil
> > It returns *nil* if testfn fails, or *nil* (ELT) if it succeds.
>
> Just because it sometimes does doesn't mean it always does:
>
>     (seq-contains '(0 3 4) 1 #'<)
>
> returns 3
>
>
>         Stefan



-- 
Miguel Vinicius Santini Frasson
mvsfrasson <at> gmail.com

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Fri, 15 Mar 2019 00:42:39 +0100

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

> > (seq-contains nil '(nil t foo) (lambda (x) t))  ->  nil
> > It returns *nil* if testfn fails, or *nil* (ELT) if it succeds.
>
> Just because it sometimes does doesn't mean it always does:
>
>     (seq-contains '(0 3 4) 1 #'<)
>
> returns 3

Maybe it's too late here, but why is that a counterexample for

> If ELT=nil, seq-contains currently returns nil anyway;"

The first element is returned for that the test succeeds, what's special
about it?  < is not symmetrical, though, so it's a weird kind of
equality test.


Michael.

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

From: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 20:45:55 -0300

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

Please ignore this e-mail of mine.

Em qui, 14 de mar de 2019 20:08, Miguel V. S. Frasson <mvsfrasson <at> gmail.com>
escreveu:

> Hi
>
> Em qui, 14 de mar de 2019 18:43, Stefan Monnier
> <monnier <at> iro.umontreal.ca> escreveu:
> >
> > > If ELT=nil, seq-contains currently returns nil anyway;
> >
> > That's true when `testfn` is nil, but not if you provide
> > your own `testfn`.
>
> Not really.
>
> (seq-contains nil '(nil t foo) (lambda (x) t))  ->  nil
>
> It returns *nil* if testfn fails, or *nil* (ELT) if it succeds.
>
> Miguel
>

[Message part 2 (text/html, inline)]

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 14 Mar 2019 22:40:27 -0400

>> > (seq-contains nil '(nil t foo) (lambda (x) t))  ->  nil
>> > It returns *nil* if testfn fails, or *nil* (ELT) if it succeds.
>> Just because it sometimes does doesn't mean it always does:
>>     (seq-contains '(0 3 4) 1 #'<)
>> returns 3
> Maybe it's too late here, but why is that a counterexample for
>> If ELT=nil, seq-contains currently returns nil anyway;"

Duh, sorry, I lost sight of the goal along the way.

A short real counter example is

    (seq-contains '(1) nil #'list)

but it's admittedly contrived.  A more real counter example is the one
alluded to by Nicolas earlier:

    (seq-contains '((t 1) (nil 2)) nil (lambda (x y) (equal x (car y))))

> The first element is returned for that the test succeeds, what's special
> about it?  < is not symmetrical, though, so it's a weird kind of
> equality test.

The test doesn't need to be an equality test.
IOW seq-contains is very close to seq-find.


        Stefan

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Fri, 15 Mar 2019 13:26:18 +0100

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

> The test doesn't need to be an equality test.

The doc doesn't specify in which order the args are delivered to the
TESTFN, so the behavior is unspecified.  For asymmetric TESTFNs I would
rather go with `seq-some'.


Michael.

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Nicolas Petton <nicolas <at> petton.fr>, 34852 <at> debbugs.gnu.org,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Fri, 15 Mar 2019 10:47:01 -0400

>> The test doesn't need to be an equality test.
> The doc doesn't specify in which order the args are delivered to the
> TESTFN, so the behavior is unspecified.

Indeed, that's another problem with seq-contains.


        Stefan

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 Nicolas Petton <nicolas <at> petton.fr>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Fri, 15 Mar 2019 15:55:58 +0000

"Miguel V. S. Frasson" <mvsfrasson <at> gmail.com> writes:

> In any case, of another implementation for seq-intersection or not, I
> think that the solution from Stefan should be implemented anyway
> because
>
> * it makes seq-contains provide a useful return value when ELT=nil, so
> it is a good exception; If ELT=nil, seq-contains currently returns nil
> anyway;

Special-casing nil invalidates any benefits of returning the element
found, as it conflates nil and t and is inconsistent with other non-nil
values.  If seq-contains is to be made a proper predicate that continues
returning the element found, then the best proposal so far has been
Michael's[1], namely to return (list ELT).  The alternative is to simply
return a boolean, rather than the element found, but this is lossier
than Michael's suggestion.

[1]: https://debbugs.gnu.org/34852#68

-- 
Basil

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 34852 <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Fri, 15 Mar 2019 15:56:19 +0000

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com> writes:
>
>> In any case, of another implementation for seq-intersection or not, I
>> think that the solution from Stefan should be implemented anyway
>> because
>>
>> * it makes seq-contains provide a useful return value when ELT=nil, so
>> it is a good exception; If ELT=nil, seq-contains currently returns nil
>> anyway;
>>
>> * it makes seq-contains become a real predicate function, making it
>> more useful;
>>
>> * since seq-contains has been used as predicate before, it is
>> unpredictable which code uses it out of official repositories, so this
>> fix potentially fixes other code.
>
> BTW, another (alternative) alternative would be to make it return
> (list ELT).  That would be a bit less backward-compatible, but a
> bit more consistent.

+1, either this or returning a boolean is fine with me.

-- 
Basil

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

From: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 Nicolas Petton <nicolas <at> petton.fr>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Fri, 15 Mar 2019 13:08:50 -0300

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

Hi

I was convinced by the discussions here that changing seq-contains is a bad
idea.

I had the false premise that ELT=nil makes seq-contains return nil.

If I could make a suggestion, it would be include an &optional NOT-FOUND to
be the value returned in case of not finding any element matching TESTFN.

Cheers

Miguel

Em sex, 15 de mar de 2019 12:56, Basil L. Contovounesios <contovob <at> tcd.ie>
escreveu:

> Michael Heerdegen <michael_heerdegen <at> web.de> writes:
>
> > "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com> writes:
> >
> >> In any case, of another implementation for seq-intersection or not, I
> >> think that the solution from Stefan should be implemented anyway
> >> because
> >>
> >> * it makes seq-contains provide a useful return value when ELT=nil, so
> >> it is a good exception; If ELT=nil, seq-contains currently returns nil
> >> anyway;
> >>
> >> * it makes seq-contains become a real predicate function, making it
> >> more useful;
> >>
> >> * since seq-contains has been used as predicate before, it is
> >> unpredictable which code uses it out of official repositories, so this
> >> fix potentially fixes other code.
> >
> > BTW, another (alternative) alternative would be to make it return
> > (list ELT).  That would be a bit less backward-compatible, but a
> > bit more consistent.
>
> +1, either this or returning a boolean is fine with me.
>
> --
> Basil
>

[Message part 2 (text/html, inline)]

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

From: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 Nicolas Petton <nicolas <at> petton.fr>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Sat, 16 Mar 2019 17:33:10 -0300

Hi

Maybe this is the same bug, now with seq-difference

(seq-difference '(nil) '(1 nil) #'eq)
-> (nil)

(In my applications, nil is a frequent element of my lists)

Is this fixed by the proposed patches?


Cheers.

Miguel

Em sex, 15 de mar de 2019 às 13:08, Miguel V. S. Frasson
<mvsfrasson <at> gmail.com> escreveu:
>
> Hi
>
> I was convinced by the discussions here that changing seq-contains is a bad idea.
>
> I had the false premise that ELT=nil makes seq-contains return nil.
>
> If I could make a suggestion, it would be include an &optional NOT-FOUND to be the value returned in case of not finding any element matching TESTFN.
>
> Cheers
>
> Miguel
>
> Em sex, 15 de mar de 2019 12:56, Basil L. Contovounesios <contovob <at> tcd.ie> escreveu:
>>
>> Michael Heerdegen <michael_heerdegen <at> web.de> writes:
>>
>> > "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com> writes:
>> >
>> >> In any case, of another implementation for seq-intersection or not, I
>> >> think that the solution from Stefan should be implemented anyway
>> >> because
>> >>
>> >> * it makes seq-contains provide a useful return value when ELT=nil, so
>> >> it is a good exception; If ELT=nil, seq-contains currently returns nil
>> >> anyway;
>> >>
>> >> * it makes seq-contains become a real predicate function, making it
>> >> more useful;
>> >>
>> >> * since seq-contains has been used as predicate before, it is
>> >> unpredictable which code uses it out of official repositories, so this
>> >> fix potentially fixes other code.
>> >
>> > BTW, another (alternative) alternative would be to make it return
>> > (list ELT).  That would be a bit less backward-compatible, but a
>> > bit more consistent.
>>
>> +1, either this or returning a boolean is fine with me.
>>
>> --
>> Basil



-- 
Miguel Vinicius Santini Frasson
mvsfrasson <at> gmail.com

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 Nicolas Petton <nicolas <at> petton.fr>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Sat, 16 Mar 2019 20:49:22 +0000

"Miguel V. S. Frasson" <mvsfrasson <at> gmail.com> writes:

> Maybe this is the same bug, now with seq-difference
>
> (seq-difference '(nil) '(1 nil) #'eq)
> -> (nil)
>
> (In my applications, nil is a frequent element of my lists)

It is indeed the same bug.

> Is this fixed by the proposed patches?

Yes.

-- 
Basil

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

From: "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>,
 Nicolas Petton <nicolas <at> petton.fr>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 34852 <at> debbugs.gnu.org
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Sat, 16 Mar 2019 18:32:09 -0300

Thanks, Basil.

Em sáb, 16 de mar de 2019 às 17:49, Basil L. Contovounesios
<contovob <at> tcd.ie> escreveu:
>
> "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com> writes:
>
> > Maybe this is the same bug, now with seq-difference
> >
> > (seq-difference '(nil) '(1 nil) #'eq)
> > -> (nil)
> >
> > (In my applications, nil is a frequent element of my lists)
>
> It is indeed the same bug.
>
> > Is this fixed by the proposed patches?
>
> Yes.
>
> --
> Basil



-- 
Miguel Vinicius Santini Frasson
mvsfrasson <at> gmail.com

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

From: Nicolas Petton <nicolas <at> petton.fr>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Mon, 18 Mar 2019 12:55:11 +0100

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

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

>> Unless somebody disagrees, I'll install your patch.
>
> If you go with the seq-some patch then please edit it to avoid using
> apply-partially, which is quite inefficient.

Yes, will do.

> The implementations of seq-set-equal-p, seq-uniq, and seq-difference
> will also need to be updated, in addition to that of seq-intersection.

Yes, thank you.

I'm now thinking about introducing a proper predicate to be used instead
of seq-contains for such cases, I would name it `seq-includes-p'.  Here
are my reasons:

- seq-some takes a predicate function, but we'd like a function that
  takes an elt to be tested.

- seq-contains returns the element found on purpose, it is not a
  predicate.  I think it has its use-cases, and I don't want to change
  it into a predicate now (I would have to rename it anyway).

Cheers,
Nico

[signature.asc (application/pgp-signature, inline)]

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Nicolas Petton <nicolas <at> petton.fr>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Mon, 18 Mar 2019 20:06:49 +0100

Nicolas Petton <nicolas <at> petton.fr> writes:

> - seq-contains returns the element found on purpose, it is not a
>   predicate.  I think it has its use-cases, and I don't want to change
>   it into a predicate now (I would have to rename it anyway).

I think most current appearances are using it as a predicate, however.
What do you intend - obsolete seq-contains?  I don't think we should
have an additional function - seq-contains already overlaps with
seq-some.

Why would you have to rename it btw?  To add a -p (I ask because
seq-some also doesn't end with -p)?  Just pondering...


Michael.

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

From: Nicolas Petton <nicolas <at> petton.fr>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Mon, 18 Mar 2019 21:14:27 +0100

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

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> I think most current appearances are using it as a predicate, however.

You have a point and we also have `seq-find' which returns the element
found.

> What do you intend - obsolete seq-contains?  I don't think we should
> have an additional function - seq-contains already overlaps with
> seq-some.

I didn't think about it, obsoleting `seq-contains' would work as well,
and it can actually make sense.  I would then name the new function
`seq-contains-p'.

> Why would you have to rename it btw?  To add a -p (I ask because
> seq-some also doesn't end with -p)?

Yes, to add a -p suffix.

Nicolas

[signature.asc (application/pgp-signature, inline)]

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

From: Nicolas Petton <nicolas <at> petton.fr>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Wed, 20 Mar 2019 21:51:30 +0100

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

Hi,

Here's a patch for master.

[0001-New-seq-contains-p-predicate-Bug-34852.patch (text/x-patch, attachment)]

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

Nico

[signature.asc (application/pgp-signature, inline)]

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Nicolas Petton <nicolas <at> petton.fr>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Wed, 20 Mar 2019 23:33:18 +0100

Nicolas Petton <nicolas <at> petton.fr> writes:

> +(cl-defgeneric seq-contains-p (sequence elt &optional testfn)
> +  "Return non-nil if SEQUENCE contains an element equal to ELT.
> +Equality is defined by TESTFN if non-nil or by `equal' if nil."
> +    (catch 'seq--break
> +      (seq-doseq (e sequence)
> +        (when (funcall (or testfn #'equal) e elt)
> +          (throw 'seq--break t)))
> +      nil))

A detail: could you factor out the result of (or testfn #'equal) so that
`or' is not called repeatedly in the loop?

Michael.

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

From: Nicolas Petton <nicolas <at> petton.fr>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 21 Mar 2019 09:02:56 +0100

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

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> A detail: could you factor out the result of (or testfn #'equal) so that
> `or' is not called repeatedly in the loop?

Yes, and I'll do that elsewhere as well.

Cheers,
Nico

[signature.asc (application/pgp-signature, inline)]

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Nicolas Petton <nicolas <at> petton.fr>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 21 Mar 2019 17:46:51 +0000

Nicolas Petton <nicolas <at> petton.fr> writes:

> Here's a patch for master.

Thanks, I have only some minor comments:

> diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
> index 4a811d7895..39c93e25ed 100644
> --- a/lisp/emacs-lisp/seq.el
> +++ b/lisp/emacs-lisp/seq.el
> @@ -356,6 +356,7 @@ seq-sort-by
>      count))
>  
>  (cl-defgeneric seq-contains (sequence elt &optional testfn)
> +  (declare (obsolete "Use `seq-contains-p' instead." "27.1"))

According to make-obsolete, "use instead" strings should not start with
a capital letter, but even better is (obsolete seq-contains-p "27.1"),
which adds a link to seq-contains-p in the *Help* buffer.

> @@ -420,7 +430,7 @@ seq-sort-by
>    "Return a list of the elements that appear in SEQUENCE1 but not in SEQUENCE2.
>  Equality is defined by TESTFN if non-nil or by `equal' if nil."
>    (seq-reduce (lambda (acc elt)
> -                (if (not (seq-contains sequence2 elt testfn))
> +                (if (not (seq-contains-p sequence2 elt testfn))
>                      (cons elt acc)
>                    acc))

Is there any harm in inverting this conditional structure, so that it
reads positively?

> diff --git a/test/lisp/emacs-lisp/seq-tests.el b/test/lisp/emacs-lisp/seq-tests.el
> index d8f00cfea4..6522def423 100644
> --- a/test/lisp/emacs-lisp/seq-tests.el
> +++ b/test/lisp/emacs-lisp/seq-tests.el
> @@ -185,6 +185,14 @@ test-sequences-oddp
>    (with-test-sequences (seq '(3 4 5 6))
>      (should (= 5 (seq-contains seq 5)))))
>  
> +(ert-deftest test-seq-contains-p ()
> +  (with-test-sequences (seq '(3 4 5 6))
> +    (should (eq (seq-contains-p seq 3) t))
> +    (should-not (seq-contains-p seq 7)))
> +  (with-test-sequences (seq '())
> +    (should-not (seq-contains-p seq 3))
> +    (should-not (seq-contains-p seq nil))))
> +

I think there should also be an explicit nil membership check:

(should (seq-contains-p  [nil] nil))
(should (seq-contains-p '(nil) nil))

-- 
Basil

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

From: Nicolas Petton <nicolas <at> petton.fr>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 21 Mar 2019 21:01:55 +0100

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

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Nicolas Petton <nicolas <at> petton.fr> writes:
>
>>  (cl-defgeneric seq-contains (sequence elt &optional testfn)
>> +  (declare (obsolete "Use `seq-contains-p' instead." "27.1"))
>
> According to make-obsolete, "use instead" strings should not start with
> a capital letter, but even better is (obsolete seq-contains-p "27.1"),
> which adds a link to seq-contains-p in the *Help* buffer.

Indeed, that's better, thank you.

>> @@ -420,7 +430,7 @@ seq-sort-by
>>    "Return a list of the elements that appear in SEQUENCE1 but not in SEQUENCE2.
>>  Equality is defined by TESTFN if non-nil or by `equal' if nil."
>>    (seq-reduce (lambda (acc elt)
>> -                (if (not (seq-contains sequence2 elt testfn))
>> +                (if (not (seq-contains-p sequence2 elt testfn))
>>                      (cons elt acc)
>>                    acc))
>
> Is there any harm in inverting this conditional structure, so that it
> reads positively?

It's not the purpose of this patch, but I can commit it separately.

>> +(ert-deftest test-seq-contains-p ()
>> +  (with-test-sequences (seq '(3 4 5 6))
>> +    (should (eq (seq-contains-p seq 3) t))
>> +    (should-not (seq-contains-p seq 7)))
>> +  (with-test-sequences (seq '())
>> +    (should-not (seq-contains-p seq 3))
>> +    (should-not (seq-contains-p seq nil))))
>> +
>
> I think there should also be an explicit nil membership check:
>
> (should (seq-contains-p  [nil] nil))
> (should (seq-contains-p '(nil) nil))

Good idea!

Nico

[signature.asc (application/pgp-signature, inline)]

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

From: Nicolas Petton <nicolas <at> petton.fr>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 34852 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 "Miguel V. S. Frasson" <mvsfrasson <at> gmail.com>
Subject: Re: bug#34852: 26.1; seq-intersection ignores nil as element
Date: Thu, 21 Mar 2019 21:16:14 +0100

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

tags 34852 fixed
close 34852 27.1
thanks

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Nicolas Petton <nicolas <at> petton.fr> writes:
>
>> Here's a patch for master.

I pushed a revised patch to master, so I'm closing this issue.

Cheers,
Nico

[signature.asc (application/pgp-signature, inline)]

Previous Next

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