unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
@ 2017-03-02  7:17 Tino Calancha
  2017-03-02  8:56 ` Nicolas Petton
                   ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Tino Calancha @ 2017-03-02  7:17 UTC (permalink / raw)
  To: 25929; +Cc: nicolas, tino.calancha


X-Debbugs-CC: <nicolas@petton.fr>

(cl-loop for i below 3 collect
  (let ((map (list (cons 0 3)
                   (cons 1 4)
                   (cons 2 5))))
    (map-delete map i)
    (null (map-elt map i))))
=> (nil t t) ; The first pair, i.e. '(0 . 3) is not permanently deleted.

I've used the word 'permantly' because it seems it is temporary
deleted:

(cl-loop for i below 3 collect
  (let ((map (list (cons 0 3)
                   (cons 1 4)
                   (cons 2 5))))
    (null (map-elt
           (map-delete map i)
           i))))
=> (t t t)

*) It happens just for alist, other maps are fine.
*) The test suite doesn't detect this issue because `test-map-delete'
   happen to delete just the 2rd element of the maps.

In GNU Emacs 25.2.3 (x86_64-pc-linux-gnu, GTK+ Version 3.22.7)
 of 2017-03-02
Repository revision: 640661838dbba8185990e839712c91a14641ddf3





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-02  7:17 bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt Tino Calancha
@ 2017-03-02  8:56 ` Nicolas Petton
  2017-03-02 10:59 ` Nicolas Petton
  2022-04-26 13:34 ` Lars Ingebrigtsen
  2 siblings, 0 replies; 32+ messages in thread
From: Nicolas Petton @ 2017-03-02  8:56 UTC (permalink / raw)
  To: Tino Calancha, 25929; +Cc: tino.calancha

[-- Attachment #1: Type: text/plain, Size: 380 bytes --]

Tino Calancha <tino.calancha@gmail.com> writes:

> (cl-loop for i below 3 collect
>   (let ((map (list (cons 0 3)
>                    (cons 1 4)
>                    (cons 2 5))))
>     (map-delete map i)
>     (null (map-elt map i))))
> => (nil t t) ; The first pair, i.e. '(0 . 3) is not permanently
>   deleted.

Thanks for the report! I'll have a look shortly.

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-02  7:17 bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt Tino Calancha
  2017-03-02  8:56 ` Nicolas Petton
@ 2017-03-02 10:59 ` Nicolas Petton
  2017-03-02 11:30   ` Tino Calancha
                     ` (2 more replies)
  2022-04-26 13:34 ` Lars Ingebrigtsen
  2 siblings, 3 replies; 32+ messages in thread
From: Nicolas Petton @ 2017-03-02 10:59 UTC (permalink / raw)
  To: Tino Calancha, 25929; +Cc: Stefan Monnier, tino.calancha

[-- Attachment #1: Type: text/plain, Size: 1379 bytes --]

Tino Calancha <tino.calancha@gmail.com> writes:


> (cl-loop for i below 3 collect
>   (let ((map (list (cons 0 3)
>                    (cons 1 4)
>                    (cons 2 5))))
>     (map-delete map i)
>     (null (map-elt map i))))
> => (nil t t) ; The first pair, i.e. '(0 . 3) is not permanently deleted.
>
> I've used the word 'permantly' because it seems it is temporary
> deleted:
>
> (cl-loop for i below 3 collect
>   (let ((map (list (cons 0 3)
>                    (cons 1 4)
>                    (cons 2 5))))
>     (null (map-elt
>            (map-delete map i)
>            i))))
> => (t t t)

The alist is indeed modified within the `map-delete' function, but in an
unexpected way: if the first key is deleted, the variable `map' is
`setq'ed, which has no effect outside of the function.

One fix would be to make `map-delete' a macro:

(defmacro map-delete (map key)
  "Delete KEY from MAP and return MAP.
No error is signaled if KEY is not a key of MAP.  If MAP is an
array, store nil at the index KEY.

  MAP can be a list, hash-table or array."
    `(macroexp-let2 nil key
       (map--dispatch ,map
         :list (setf (alist-get ,key ,map nil t) nil)
         :hash-table (remhash ,key ,map)
         :array (and (>= ,key 0)
                     (<= ,key (seq-length ,map))
                     (aset ,map ,key nil)))
       ,map))

WDYT?

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-02 10:59 ` Nicolas Petton
@ 2017-03-02 11:30   ` Tino Calancha
  2017-03-02 12:27     ` Nicolas Petton
  2017-03-02 12:34   ` Nicolas Petton
  2017-03-02 12:36   ` npostavs
  2 siblings, 1 reply; 32+ messages in thread
From: Tino Calancha @ 2017-03-02 11:30 UTC (permalink / raw)
  To: Nicolas Petton; +Cc: 25929, Stefan Monnier, Tino Calancha

[-- Attachment #1: Type: text/plain, Size: 1265 bytes --]



On Thu, 2 Mar 2017, Nicolas Petton wrote:

> Tino Calancha <tino.calancha@gmail.com> writes:
>
>
>> (cl-loop for i below 3 collect
>>   (let ((map (list (cons 0 3)
>>                    (cons 1 4)
>>                    (cons 2 5))))
>>     (map-delete map i)
>>     (null (map-elt map i))))
>> => (nil t t) ; The first pair, i.e. '(0 . 3) is not permanently deleted.
>
> The alist is indeed modified within the `map-delete' function, but in an
> unexpected way: if the first key is deleted, the variable `map' is
> `setq'ed, which has no effect outside of the function.
>
> One fix would be to make `map-delete' a macro:
>
> (defmacro map-delete (map key)
>  "Delete KEY from MAP and return MAP.
> No error is signaled if KEY is not a key of MAP.  If MAP is an
> array, store nil at the index KEY.
>
>  MAP can be a list, hash-table or array."
>    `(macroexp-let2 nil key
>       (map--dispatch ,map
>         :list (setf (alist-get ,key ,map nil t) nil)
>         :hash-table (remhash ,key ,map)
>         :array (and (>= ,key 0)
>                     (<= ,key (seq-length ,map))
>                     (aset ,map ,key nil)))
>       ,map))
>

After this patch if i run the above recipe i get an error:
let*: Symbol’s function definition is void: map--dispatch


^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-02 11:30   ` Tino Calancha
@ 2017-03-02 12:27     ` Nicolas Petton
  0 siblings, 0 replies; 32+ messages in thread
From: Nicolas Petton @ 2017-03-02 12:27 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 25929, Stefan Monnier, Tino Calancha

[-- Attachment #1: Type: text/plain, Size: 284 bytes --]

Tino Calancha <tino.calancha@gmail.com> writes:

> After this patch if i run the above recipe i get an error:
> let*: Symbol’s function definition is void: map--dispatch

`map--dispatch' is wrapped into a `eval-when-compile' call.  Evaluate
the macro first?

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-02 10:59 ` Nicolas Petton
  2017-03-02 11:30   ` Tino Calancha
@ 2017-03-02 12:34   ` Nicolas Petton
  2017-03-02 13:34     ` Tino Calancha
  2017-03-02 15:12     ` Stefan Monnier
  2017-03-02 12:36   ` npostavs
  2 siblings, 2 replies; 32+ messages in thread
From: Nicolas Petton @ 2017-03-02 12:34 UTC (permalink / raw)
  To: Tino Calancha, 25929; +Cc: Stefan Monnier, tino.calancha

[-- Attachment #1: Type: text/plain, Size: 1116 bytes --]

Nicolas Petton <nicolas@petton.fr> writes:

> The alist is indeed modified within the `map-delete' function, but in an
> unexpected way: if the first key is deleted, the variable `map' is
> `setq'ed, which has no effect outside of the function.
>
> One fix would be to make `map-delete' a macro:

Here's a fixed version:

  (defmacro map-delete (map key)
    "Delete KEY from MAP and return MAP.
  No error is signaled if KEY is not a key of MAP.  If MAP is an
  array, store nil at the index KEY.
  
  MAP can be a list, hash-table or array."
    (macroexp-let2 nil key
      `(progn
         (map--dispatch ,map
           :list (setf (alist-get ,key ,map nil t) nil)
           :hash-table (remhash ,key ,map)
           :array (and (>= ,key 0)
                       (<= ,key (seq-length ,map))
                       (aset ,map ,key nil)))
         ,map)))

And the associated regression test:

  (ert-deftest test-map-delete-first-key-alist ()
    (let ((alist '((a . 1) (b . 2) (c . 3))))
      (map-delete alist 'a)
      (should (null (map-elt alist 'a)))))

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-02 10:59 ` Nicolas Petton
  2017-03-02 11:30   ` Tino Calancha
  2017-03-02 12:34   ` Nicolas Petton
@ 2017-03-02 12:36   ` npostavs
  2017-03-02 12:45     ` Nicolas Petton
  2 siblings, 1 reply; 32+ messages in thread
From: npostavs @ 2017-03-02 12:36 UTC (permalink / raw)
  To: Nicolas Petton; +Cc: 25929, Stefan Monnier, Tino Calancha

Nicolas Petton <nicolas@petton.fr> writes:

>     `(macroexp-let2 nil key

I guess the backquote is in the wrong place?





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-02 12:36   ` npostavs
@ 2017-03-02 12:45     ` Nicolas Petton
  0 siblings, 0 replies; 32+ messages in thread
From: Nicolas Petton @ 2017-03-02 12:45 UTC (permalink / raw)
  To: npostavs; +Cc: 25929, Stefan Monnier, Tino Calancha

[-- Attachment #1: Type: text/plain, Size: 111 bytes --]

npostavs@users.sourceforge.net writes:

> I guess the backquote is in the wrong place?

Yes, I posted a fix :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-02 12:34   ` Nicolas Petton
@ 2017-03-02 13:34     ` Tino Calancha
  2017-03-02 15:12     ` Stefan Monnier
  1 sibling, 0 replies; 32+ messages in thread
From: Tino Calancha @ 2017-03-02 13:34 UTC (permalink / raw)
  To: Nicolas Petton; +Cc: 25929, Stefan Monnier, Tino Calancha



On Thu, 2 Mar 2017, Nicolas Petton wrote:

> Nicolas Petton <nicolas@petton.fr> writes:
>
>> The alist is indeed modified within the `map-delete' function, but in an
>> unexpected way: if the first key is deleted, the variable `map' is
>> `setq'ed, which has no effect outside of the function.
>>
>> One fix would be to make `map-delete' a macro:
>
> Here's a fixed version:
>
>  (defmacro map-delete (map key)
>    "Delete KEY from MAP and return MAP.
>  No error is signaled if KEY is not a key of MAP.  If MAP is an
>  array, store nil at the index KEY.
>
>  MAP can be a list, hash-table or array."
>    (macroexp-let2 nil key
>      `(progn
>         (map--dispatch ,map
>           :list (setf (alist-get ,key ,map nil t) nil)
>           :hash-table (remhash ,key ,map)
>           :array (and (>= ,key 0)
>                       (<= ,key (seq-length ,map))
>                       (aset ,map ,key nil)))
>         ,map)))
>
> And the associated regression test:
>
>  (ert-deftest test-map-delete-first-key-alist ()
>    (let ((alist '((a . 1) (b . 2) (c . 3))))
>      (map-delete alist 'a)
>      (should (null (map-elt alist 'a)))))

Sorry, i don't understand why following doesn't work after the patch:
;; I have byte-compiled map.el after applying the patch.
emacs -Q

(let ((alist '((a . 1) (b . 2) (c . 3))))
   (map-delete alist 'a)
   (map-elt alist 'a))

Debugger entered--Lisp error: (void-function map--dispatch)

We shouldn't ask the user to compile the file on each interactive session
in order to `map--dispatch' be defined.

We might also mention in the doc strings that MAP must be a generalized
variable or something.
To explain that I) below is allowed but II) is not:
I)
  (assq-delete-all 'foo '((foo . 1) (bar . 2)))

II)
  (map-delete '((foo . 1) (bar . 2)) 'foo)


Regards,
Tino





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-02 12:34   ` Nicolas Petton
  2017-03-02 13:34     ` Tino Calancha
@ 2017-03-02 15:12     ` Stefan Monnier
  2017-03-04  0:04       ` Michael Heerdegen
  2017-03-21 11:35       ` bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt Nicolas Petton
  1 sibling, 2 replies; 32+ messages in thread
From: Stefan Monnier @ 2017-03-02 15:12 UTC (permalink / raw)
  To: Nicolas Petton; +Cc: 25929, Tino Calancha

>     (macroexp-let2 nil key
>       `(progn
>          (map--dispatch ,map
>            :list (setf (alist-get ,key ,map nil t) nil)
>            :hash-table (remhash ,key ,map)
>            :array (and (>= ,key 0)
>                        (<= ,key (seq-length ,map))
>                        (aset ,map ,key nil)))
>          ,map)))

Note that this will make it pretty much impossible to use
cl-generic dispatch.

A better option might be to provide a map-remove which works
functionally (i.e. doesn't modify its argument by side-effects), and
then change map-delete to signal an error when we ask to delete the
first element of the list (and to stop returning the "new map" since it
just works imperatively instead).


        Stefan





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-02 15:12     ` Stefan Monnier
@ 2017-03-04  0:04       ` Michael Heerdegen
  2017-03-04  0:16         ` Michael Heerdegen
  2017-03-21 20:41         ` Lars Ingebrigtsen
  2017-03-21 11:35       ` bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt Nicolas Petton
  1 sibling, 2 replies; 32+ messages in thread
From: Michael Heerdegen @ 2017-03-04  0:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25929, Nicolas Petton, Tino Calancha

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

> >     (macroexp-let2 nil key
> >       `(progn
> >          (map--dispatch ,map
> >            :list (setf (alist-get ,key ,map nil t) nil)
> >            :hash-table (remhash ,key ,map)
> >            :array (and (>= ,key 0)
> >                        (<= ,key (seq-length ,map))
> >                        (aset ,map ,key nil)))
> >          ,map)))
>
> Note that this will make it pretty much impossible to use
> cl-generic dispatch.

It will also break cases where MAP is an expression that is not a
symbol:

#+begin_src emacs-lisp
(let ((thing (cons 'tag (list (cons 0 1) (cons 2 3)))))
  (map-delete (cdr thing) 0))
==> nil

(map-delete (list (cons 0 1) (cons 2 3)) 0)
==> if: Symbol's function definition is void: \(setf\ list\)
#+end_src

> A better option might be to provide a map-remove which works
> functionally (i.e. doesn't modify its argument by side-effects), and
> then change map-delete to signal an error when we ask to delete the
> first element of the list (and to stop returning the "new map" since
> it just works imperatively instead).

Why not make it just like `delete': return the map with the entry
removed, the original map might be altered - that's it.

If you want to assign the result back to a symbol/place, use setf or
callf.  Any unnecessary magic just blows up the semantics.

With other words, I would fix this issue by altering the docstring
("return the result" instead of "return MAP") - or am I missing
something?


Michael.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-04  0:04       ` Michael Heerdegen
@ 2017-03-04  0:16         ` Michael Heerdegen
  2017-03-21 20:41         ` Lars Ingebrigtsen
  1 sibling, 0 replies; 32+ messages in thread
From: Michael Heerdegen @ 2017-03-04  0:16 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25929, Nicolas Petton, Tino Calancha

Michael Heerdegen <michael_heerdegen@web.de> writes:

> With other words, I would fix this issue by altering the docstring
> ("return the result" instead of "return MAP")

When the MAP is an alist with only one entry, the return value will be
something different than MAP anyway (a cons cannot be altered to become
nil) - unless we look at "MAP" as a place expression and rewrite the
thing accordingly.  I don't think that is necessary.


Michael.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-02 15:12     ` Stefan Monnier
  2017-03-04  0:04       ` Michael Heerdegen
@ 2017-03-21 11:35       ` Nicolas Petton
  2017-03-21 15:11         ` Stefan Monnier
  1 sibling, 1 reply; 32+ messages in thread
From: Nicolas Petton @ 2017-03-21 11:35 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25929, Tino Calancha

[-- Attachment #1: Type: text/plain, Size: 977 bytes --]

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

>>     (macroexp-let2 nil key
>>       `(progn
>>          (map--dispatch ,map
>>            :list (setf (alist-get ,key ,map nil t) nil)
>>            :hash-table (remhash ,key ,map)
>>            :array (and (>= ,key 0)
>>                        (<= ,key (seq-length ,map))
>>                        (aset ,map ,key nil)))
>>          ,map)))
>
> Note that this will make it pretty much impossible to use
> cl-generic dispatch.

Indeed, but I already have that issue.  To use `cl-generic', map.el has
to be basically rewritten from scratch.

> A better option might be to provide a map-remove which works
> functionally (i.e. doesn't modify its argument by side-effects)

I'd love to have that, but what about performance issues with
hash-tables?

hash-tables aren't persistent collections, and having `map-remove',
`map-add', etc. work functionally, the hash-table would have
to be copied for each operation.

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-21 11:35       ` bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt Nicolas Petton
@ 2017-03-21 15:11         ` Stefan Monnier
  2017-03-21 18:06           ` Nicolas Petton
  0 siblings, 1 reply; 32+ messages in thread
From: Stefan Monnier @ 2017-03-21 15:11 UTC (permalink / raw)
  To: Nicolas Petton; +Cc: 25929, Tino Calancha

>> A better option might be to provide a map-remove which works
>> functionally (i.e. doesn't modify its argument by side-effects)
> I'd love to have that, but what about performance issues with
> hash-tables?

I was thinking of providing map-remove additionally to map-delete (and
make map-delete signal an error when it can't do its job).  IOW declare
that some parts of the API can't be used with all types.


        Stefan





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-21 15:11         ` Stefan Monnier
@ 2017-03-21 18:06           ` Nicolas Petton
  2017-03-21 20:29             ` Stefan Monnier
  0 siblings, 1 reply; 32+ messages in thread
From: Nicolas Petton @ 2017-03-21 18:06 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25929, Tino Calancha

[-- Attachment #1: Type: text/plain, Size: 414 bytes --]

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

> I was thinking of providing map-remove additionally to map-delete (and
> make map-delete signal an error when it can't do its job).  IOW declare
> that some parts of the API can't be used with all types.

Yes, that's what I want as well, but then `map-remove' would be
functional (return another map), and it means that it would copy
hash-tables.

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-21 18:06           ` Nicolas Petton
@ 2017-03-21 20:29             ` Stefan Monnier
  2017-04-26  7:58               ` Nicolas Petton
  0 siblings, 1 reply; 32+ messages in thread
From: Stefan Monnier @ 2017-03-21 20:29 UTC (permalink / raw)
  To: Nicolas Petton; +Cc: 25929, Tino Calancha

>> I was thinking of providing map-remove additionally to map-delete (and
>> make map-delete signal an error when it can't do its job).  IOW declare
>> that some parts of the API can't be used with all types.
> Yes, that's what I want as well, but then `map-remove' would be
> functional (return another map), and it means that it would copy
> hash-tables.

That's right.  Or it could signal an error, if copying is considered to
be "never the right thing to do".


        Stefan





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-04  0:04       ` Michael Heerdegen
  2017-03-04  0:16         ` Michael Heerdegen
@ 2017-03-21 20:41         ` Lars Ingebrigtsen
  2017-03-22 11:55           ` Michael Heerdegen
  1 sibling, 1 reply; 32+ messages in thread
From: Lars Ingebrigtsen @ 2017-03-21 20:41 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 25929, Nicolas Petton, Stefan Monnier, Tino Calancha

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> >     (macroexp-let2 nil key
>> >       `(progn
>> >          (map--dispatch ,map
>> >            :list (setf (alist-get ,key ,map nil t) nil)
>> >            :hash-table (remhash ,key ,map)
>> >            :array (and (>= ,key 0)
>> >                        (<= ,key (seq-length ,map))
>> >                        (aset ,map ,key nil)))
>> >          ,map)))
>>
>> Note that this will make it pretty much impossible to use
>> cl-generic dispatch.
>
> It will also break cases where MAP is an expression that is not a
> symbol:
>
> #+begin_src emacs-lisp
> (let ((thing (cons 'tag (list (cons 0 1) (cons 2 3)))))
>   (map-delete (cdr thing) 0))
> ==> nil
>
> (map-delete (list (cons 0 1) (cons 2 3)) 0)
> ==> if: Symbol's function definition is void: \(setf\ list\)
> #+end_src

If MAP isn't a symbol, then map-delete could avoid doing the setf,
couldn't it?

I think that should make for a pretty useful form.  You can both say

...
(map-delete foo 'bar)
...

and

(setq foo (cons (map-delete (get-a-list) 'bar) 'zot))

and not be surprised at the results.  It's certainly more fun than the
current (setq foo (delete 'bar foo)) we have all over the place, even if
it's something of a departure from how functions like this has
traditionally worked in various Lisps.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-21 20:41         ` Lars Ingebrigtsen
@ 2017-03-22 11:55           ` Michael Heerdegen
  2017-03-22 12:01             ` Lars Ingebrigtsen
  2017-03-29 15:25             ` bug#25929: 25.2; plists and map-* Lars Ingebrigtsen
  0 siblings, 2 replies; 32+ messages in thread
From: Michael Heerdegen @ 2017-03-22 11:55 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 25929, Nicolas Petton, Stefan Monnier, Tino Calancha

Lars Ingebrigtsen <larsi@gnus.org> writes:

> If MAP isn't a symbol, then map-delete could avoid doing the setf,
> couldn't it?
>
> I think that should make for a pretty useful form.  You can both say
>
> ...
> (map-delete foo 'bar)
> ...
>
> and
>
> (setq foo (cons (map-delete (get-a-list) 'bar) 'zot))
>
> and not be surprised at the results.  It's certainly more fun than the
>current (setq foo (delete 'bar foo)) we have all over the place, even
>if it's something of a departure from how functions like this has
>traditionally worked in various Lisps.

So your suggestion is, with other words, that

(1)  (map-delete EXPR 'bar)

is implicitly transformed into

(2)  (cl-callf map-delete EXPR 'bar)

when EXPR is a valid place expression.

That's a relatively small gain, and some people might prefer the
explicit form (2) for readability.

OTOH, without such magic, we can avoid making `map-delete' a macro.
Personally I prefer this simpler approach.


Regards,

Michael.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-22 11:55           ` Michael Heerdegen
@ 2017-03-22 12:01             ` Lars Ingebrigtsen
  2017-03-22 12:56               ` Noam Postavsky
  2017-03-22 17:02               ` Stefan Monnier
  2017-03-29 15:25             ` bug#25929: 25.2; plists and map-* Lars Ingebrigtsen
  1 sibling, 2 replies; 32+ messages in thread
From: Lars Ingebrigtsen @ 2017-03-22 12:01 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 25929, Nicolas Petton, Stefan Monnier, Tino Calancha

Michael Heerdegen <michael_heerdegen@web.de> writes:

> That's a relatively small gain, and some people might prefer the
> explicit form (2) for readability.

If we don't do this, (map-delete foo 'bar) has surprising effects in
this one corner case.

That is, if foo is a hash table, then it's fine to say
(map-delete foo 'bar).  If you change foo to be an alist, you then have
to transform all the calls to be (setq foo (map-delete foo 'bar))
instead, which kind of breaks the abstraction that map-delete tried to
offer, I think?

Making map-delete signal an error in the
alist-where-bar-is-the-first-element sounds even worse to me, since the
user may already be writing this stuff as (setq foo (map-delete foo
'bar)).

(I think this was what Stefan suggested...)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-22 12:01             ` Lars Ingebrigtsen
@ 2017-03-22 12:56               ` Noam Postavsky
  2017-03-22 13:31                 ` Lars Ingebrigtsen
  2017-03-22 17:02               ` Stefan Monnier
  1 sibling, 1 reply; 32+ messages in thread
From: Noam Postavsky @ 2017-03-22 12:56 UTC (permalink / raw)
  To: Lars Ingebrigtsen
  Cc: Michael Heerdegen, 25929, Nicolas Petton, Stefan Monnier,
	Tino Calancha

On Wed, Mar 22, 2017 at 8:01 AM, Lars Ingebrigtsen <larsi@gnus.org> wrote:
> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> That's a relatively small gain, and some people might prefer the
>> explicit form (2) for readability.
>
> If we don't do this, (map-delete foo 'bar) has surprising effects in
> this one corner case.
>
> That is, if foo is a hash table, then it's fine to say
> (map-delete foo 'bar).  If you change foo to be an alist, you then have
> to transform all the calls to be (setq foo (map-delete foo 'bar))
> instead,

It's also fine to say (setq foo (map-delete foo 'bar)) if foo is hash
table, right?





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-22 12:56               ` Noam Postavsky
@ 2017-03-22 13:31                 ` Lars Ingebrigtsen
  2017-03-22 14:43                   ` Michael Heerdegen
  2017-03-22 17:04                   ` Stefan Monnier
  0 siblings, 2 replies; 32+ messages in thread
From: Lars Ingebrigtsen @ 2017-03-22 13:31 UTC (permalink / raw)
  To: Noam Postavsky
  Cc: Michael Heerdegen, 25929, Nicolas Petton, Stefan Monnier,
	Tino Calancha

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> It's also fine to say (setq foo (map-delete foo 'bar)) if foo is hash
> table, right?

Sure, but then this isn't a very convenient form to use at all.
Everybody who uses hash tables will use the old form, which makes this
generalisation useless in practice.

I'm not quite sure what the objection to map-delete being a macro is...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-22 13:31                 ` Lars Ingebrigtsen
@ 2017-03-22 14:43                   ` Michael Heerdegen
  2017-03-22 17:04                   ` Stefan Monnier
  1 sibling, 0 replies; 32+ messages in thread
From: Michael Heerdegen @ 2017-03-22 14:43 UTC (permalink / raw)
  To: Lars Ingebrigtsen
  Cc: 25929, Nicolas Petton, Noam Postavsky, Stefan Monnier,
	Tino Calancha

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I'm not quite sure what the objection to map-delete being a macro
> is...

It depends on what you find "convenient".  For any macro, I have to
remember which arguments are evaluated and which not, or read the docs
every time I want to use it.  Macros make debugging harder.  Etc.
That's why we usually avoid macros when a function would do.

I understand that you find your suggestion more convenient, but OTOH it
would not be consistent with similar functions we have and introduce
another inconsistency.  I just think it's not worth it.


Michael.





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-22 12:01             ` Lars Ingebrigtsen
  2017-03-22 12:56               ` Noam Postavsky
@ 2017-03-22 17:02               ` Stefan Monnier
  1 sibling, 0 replies; 32+ messages in thread
From: Stefan Monnier @ 2017-03-22 17:02 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Michael Heerdegen, 25929, Nicolas Petton, Tino Calancha

> Making map-delete signal an error in the
> alist-where-bar-is-the-first-element sounds even worse to me, since the
> user may already be writing this stuff as (setq foo (map-delete foo
> 'bar)).
> (I think this was what Stefan suggested...)

Good catch, no, what I'm suggesting is to signal an error when
map-delete is called on an alist (forcing the user to use map-remove
for that instead).


        Stefan





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-22 13:31                 ` Lars Ingebrigtsen
  2017-03-22 14:43                   ` Michael Heerdegen
@ 2017-03-22 17:04                   ` Stefan Monnier
  2017-03-22 17:23                     ` Lars Ingebrigtsen
  1 sibling, 1 reply; 32+ messages in thread
From: Stefan Monnier @ 2017-03-22 17:04 UTC (permalink / raw)
  To: Lars Ingebrigtsen
  Cc: Michael Heerdegen, 25929, Nicolas Petton, Noam Postavsky,
	Tino Calancha

> I'm not quite sure what the objection to map-delete being a macro is...

The problem is how to make it extensible to user-defined types
via something like cl-generic.el (e.g. add support for avl-tree.el
without having to make map.el aware of avl-tree.el).


        Stefan





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-22 17:04                   ` Stefan Monnier
@ 2017-03-22 17:23                     ` Lars Ingebrigtsen
  2017-03-22 20:31                       ` Stefan Monnier
  0 siblings, 1 reply; 32+ messages in thread
From: Lars Ingebrigtsen @ 2017-03-22 17:23 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Michael Heerdegen, 25929, Nicolas Petton, Noam Postavsky,
	Tino Calancha

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

> The problem is how to make it extensible to user-defined types
> via something like cl-generic.el (e.g. add support for avl-tree.el
> without having to make map.el aware of avl-tree.el).

Hm...  I haven't used cl-generic.el, but having map-delete be something
like this work?

(defmacro map-delete (map elem)
  (if (symbolp map)
      `(setq ,map (map-delete-1 ,map ,elem))
    `(map-delete-1 ,map ,elem)))

And then having map-delete-1 be the thing to support user-defined types?

It'd be kinda an even more unusual Lisp construct, but, on the other
hand, I don't think Lisp has ever come up with an elegant way to express
these mutating functions before, so we're free to innovate.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-22 17:23                     ` Lars Ingebrigtsen
@ 2017-03-22 20:31                       ` Stefan Monnier
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Monnier @ 2017-03-22 20:31 UTC (permalink / raw)
  To: Lars Ingebrigtsen
  Cc: Michael Heerdegen, 25929, Nicolas Petton, Noam Postavsky,
	Tino Calancha

>> The problem is how to make it extensible to user-defined types
>> via something like cl-generic.el (e.g. add support for avl-tree.el
>> without having to make map.el aware of avl-tree.el).

> Hm...  I haven't used cl-generic.el, but having map-delete be something
> like this work?

> (defmacro map-delete (map elem)
>   (if (symbolp map)
>       `(setq ,map (map-delete-1 ,map ,elem))
>     `(map-delete-1 ,map ,elem)))

It would work, yes.  I think the resulting semantics is pretty ugly, but
it's just a question of taste.


        Stefan





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; plists and map-*
  2017-03-22 11:55           ` Michael Heerdegen
  2017-03-22 12:01             ` Lars Ingebrigtsen
@ 2017-03-29 15:25             ` Lars Ingebrigtsen
  2017-03-29 15:36               ` Nicolas Petton
  1 sibling, 1 reply; 32+ messages in thread
From: Lars Ingebrigtsen @ 2017-03-29 15:25 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 25929, Nicolas Petton, Stefan Monnier, Tino Calancha

A thing just occurred to me: Does the map-* functions work for plists,
too?  plists are, like, the fourth major mapping structure we have in
Lisp.

Wouldn't it be easy to extend map-* to handle plists, too?  For
instance, map-delete (of course) knows whether it's looking at a hash
table or an array, but it can also easily distinguish between an alist
and a plist by just looking at the first element, can't it?

It's a bit hacky, though.  But might be nice.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no






^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; plists and map-*
  2017-03-29 15:25             ` bug#25929: 25.2; plists and map-* Lars Ingebrigtsen
@ 2017-03-29 15:36               ` Nicolas Petton
  0 siblings, 0 replies; 32+ messages in thread
From: Nicolas Petton @ 2017-03-29 15:36 UTC (permalink / raw)
  To: Lars Ingebrigtsen, Michael Heerdegen; +Cc: 25929, Stefan Monnier, Tino Calancha

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> A thing just occurred to me: Does the map-* functions work for plists,
> too?  plists are, like, the fourth major mapping structure we have in
> Lisp.

Not currently.

> Wouldn't it be easy to extend map-* to handle plists, too?  For
> instance, map-delete (of course) knows whether it's looking at a hash
> table or an array, but it can also easily distinguish between an alist
> and a plist by just looking at the first element, can't it?

Yes, it would, and was proposed a few weeks ago.

> It's a bit hacky, though.  But might be nice.

It would.  I didn't do it initially exactly because I thought it was a
bit hackish, but I agree that it would be nice to have.

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-21 20:29             ` Stefan Monnier
@ 2017-04-26  7:58               ` Nicolas Petton
  0 siblings, 0 replies; 32+ messages in thread
From: Nicolas Petton @ 2017-04-26  7:58 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 25929, Tino Calancha

[-- Attachment #1: Type: text/plain, Size: 378 bytes --]

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

> That's right.  Or it could signal an error, if copying is considered to
> be "never the right thing to do".

If hash-tables were persistent data structures, I'd say that it would
definitely be the right thing to do.

I didn't implement map.el as a functional library because vectors and
hash-tables are not persistent.

Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2017-03-02  7:17 bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt Tino Calancha
  2017-03-02  8:56 ` Nicolas Petton
  2017-03-02 10:59 ` Nicolas Petton
@ 2022-04-26 13:34 ` Lars Ingebrigtsen
  2022-04-28  3:17   ` Richard Stallman
  2 siblings, 1 reply; 32+ messages in thread
From: Lars Ingebrigtsen @ 2022-04-26 13:34 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 25929, nicolas

Tino Calancha <tino.calancha@gmail.com> writes:

> *) It happens just for alist, other maps are fine.

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

Various things were proposed here, but they all had problems.  So I
don't think there's much to be done here except documenting this, which
I've now done in Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2022-04-26 13:34 ` Lars Ingebrigtsen
@ 2022-04-28  3:17   ` Richard Stallman
  2022-04-28 16:00     ` Drew Adams
  0 siblings, 1 reply; 32+ messages in thread
From: Richard Stallman @ 2022-04-28  3:17 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 25929, nicolas, tino.calancha

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > *) It happens just for alist, other maps are fine.

  > (I'm going through old bug reports that unfortunately weren't resolved
  > at the time.)

  > Various things were proposed here, but they all had problems.  So I
  > don't think there's much to be done here except documenting this, which
  > I've now done in Emacs 29.

It is a natural consequence of list structure that deleting the first element
doesn't do it destructively, so you need to do
  (setq foo (delq elt foo))

This is a general principal of deletion in Lisp, so let's teach it to
people rather than trying to help people avoid the need to learn it.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)







^ permalink raw reply	[flat|nested] 32+ messages in thread

* bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt
  2022-04-28  3:17   ` Richard Stallman
@ 2022-04-28 16:00     ` Drew Adams
  0 siblings, 0 replies; 32+ messages in thread
From: Drew Adams @ 2022-04-28 16:00 UTC (permalink / raw)
  To: rms@gnu.org, Lars Ingebrigtsen
  Cc: 25929@debbugs.gnu.org, nicolas@petton.fr, tino.calancha@gmail.com

> It is a natural consequence of list structure that 
> deleting the first element doesn't do it destructively,
> so you need to do (setq foo (delq elt foo))
> 
> This is a general principal of deletion in Lisp, so
> let's teach it to people rather than trying to help
> people avoid the need to learn it.

+1.

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2022-04-28 16:00 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-02  7:17 bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt Tino Calancha
2017-03-02  8:56 ` Nicolas Petton
2017-03-02 10:59 ` Nicolas Petton
2017-03-02 11:30   ` Tino Calancha
2017-03-02 12:27     ` Nicolas Petton
2017-03-02 12:34   ` Nicolas Petton
2017-03-02 13:34     ` Tino Calancha
2017-03-02 15:12     ` Stefan Monnier
2017-03-04  0:04       ` Michael Heerdegen
2017-03-04  0:16         ` Michael Heerdegen
2017-03-21 20:41         ` Lars Ingebrigtsen
2017-03-22 11:55           ` Michael Heerdegen
2017-03-22 12:01             ` Lars Ingebrigtsen
2017-03-22 12:56               ` Noam Postavsky
2017-03-22 13:31                 ` Lars Ingebrigtsen
2017-03-22 14:43                   ` Michael Heerdegen
2017-03-22 17:04                   ` Stefan Monnier
2017-03-22 17:23                     ` Lars Ingebrigtsen
2017-03-22 20:31                       ` Stefan Monnier
2017-03-22 17:02               ` Stefan Monnier
2017-03-29 15:25             ` bug#25929: 25.2; plists and map-* Lars Ingebrigtsen
2017-03-29 15:36               ` Nicolas Petton
2017-03-21 11:35       ` bug#25929: 25.2; map-delete doesn't delete permanently 1st alist elt Nicolas Petton
2017-03-21 15:11         ` Stefan Monnier
2017-03-21 18:06           ` Nicolas Petton
2017-03-21 20:29             ` Stefan Monnier
2017-04-26  7:58               ` Nicolas Petton
2017-03-02 12:36   ` npostavs
2017-03-02 12:45     ` Nicolas Petton
2022-04-26 13:34 ` Lars Ingebrigtsen
2022-04-28  3:17   ` Richard Stallman
2022-04-28 16:00     ` Drew Adams

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).