Make assoc a macro instead of a special form

[Kodiologist]

• Closes Make assoc a function instead of a special form #841.
• Fixes make assoc use a gensym #1068.

See the discussion in #1346.

[gilch]

I see you're not using defmacro! That must be to avoid the gensym in the case of only one pair.

[gilch]

Since #1349, splice works on any False value. This could be ~@(if other-kvs [c coll]) (or when).

[Kodiologist]

Oh, right.

[gilch]

~@(and other-kvs [c coll]) would also work.

[gilch]

sum and zip are not indented. And is there a reason you're avoiding partition? That would be simpler.

~@(sum (list-comp [`(get ~c ~k) v] [[k v] (partition other-kvs)])
       [`(get ~c ~k1) v1])

[gilch]

~@(sum (list-comp [`(get ~c ~k) v] [[k v] (cons [k1 v1] (partition other-kvs))]) should also work. I should be testing these at the repl.

[gilch]

Hmm, nope. A cons onto a generic iterable doesn't work like that for some reason. It would have to be (chain [[k1 v1]] (partition other-kvs)), or spliced like

~@(sum (list-comp [`(get ~c ~k) v] [[k v] `(~[k1 v1] ~@(partition other-kvs))])

(I'd have used #* instead of the splice, but that doesn't work in older versions of Python.)

[Kodiologist]

How's that?

[gilch]

Not what I meant. Sorry I got it wrong the first try, you probably didn't see the edit. How about like this.

(defmacro assoc [coll k1 v1 &rest other-kvs]
  (if (odd? (len other-kvs))
    (macro-error (last other-kvs) "`assoc` takes an odd number of arguments"))
  (setv c (if other-kvs (gensym "g-assoc") coll))
  `(setv
    ~@(and other-kvs [c coll])
    ~@(sum (list-comp [`(get ~c ~k) v] [[k v] (partition other-kvs)])
           [`(get ~c ~k1) v1])))

[Kodiologist]

I see the motivation for using partition instead of zip, but why take the [`(get ~c ~k1) v1]) out of the list-comp?

[gilch]

Hmm, yes. I don't like it there either. I think it's clearer to put all the assignments in the order they'd appear in the expansion.

(defmacro assoc [coll k1 v1 &rest other-kvs]
  (if (odd? (len other-kvs))
    (macro-error (last other-kvs)
                 "`assoc` takes an odd number of arguments"))
  (setv c (if other-kvs
            (gensym "assoc-coll")
            coll))
  `(setv
     ~@(and other-kvs [c coll])
     (get ~c ~k1) ~v1
     ~@(+ [] #* (list-comp [`(get ~c ~k) v]
                           [[k v] (partition other-kvs)]))))

This looks more like a template for the expansion. I like it there better.

We shouldn't ever be using sum on lists anyway. It's a mere implementation detail that this works at all. sum's docstring says

This function is intended specifically for use with numeric values and may
reject non-numeric types.

[Edit: the (+ [] is required even given #1349. Though (+ #* []) is 0 which works in the splice, the one-argument case (+ #* [[foo bar]]) uses the unary +, which isn't implemented in list.]

We do have the shadow + here, right? If not, chain.from-iterable would also avoid the last [] in the sum altogether, though it's an unfortunately long name for such a useful operation. Maybe we could give it a *chain alias or something, but that's another issue. I see this pattern a lot in macros, so I added a chain-comp macro in my destructure PR. It might be useful in core if we see this kind of pattern a lot there. But again, another issue.

[Kodiologist]

That's true about sum, and we do indeed have shadow + here. Would you accept this?

[gilch]

This is not indented properly, which makes it hard to read. Emacs can do this for you.

[Kodiologist]

It's indented with two spaces, same as everything else.

[gilch]

No, it is not indented 2 spaces. After bracket counting, I can see that arguments to genexpr are indented as if they were arguments to setv. It will take 42 more spaces to get it indented two spaces past the start of their list. To avoid excessively wide lines, instead of just inserting 42 spaces at the start of those two lines, add some newlines.

One must never put list elements on a column before (or equal to) the bracket opening the list, in any Lisp. Elements must always appear on a column following the opening bracket of the list they belong to. And when I say "list", this applies to all Hy models inheriting from HyList.

[Kodiologist]

Okay.

[gilch]

genexpr is now indented at -7 from the opening bracket of its list. Line it up with the (if, or add a newline and line up both the (if and (genexpr at +2.

[gilch]

It would also look better if #* were on the same line as the thing it's unpacking.

[Kodiologist]

Please stop.

[gilch]

This is not legible, Kodi.
I cannot approve obfuscated code.

You don't seem to understand how to format Lisp, or why it is important to. Please read up on it, or let Emacs do it for you.

[gilch]

I amended a commit for formatting and I fixed the conflict with NEWS using the web editor, which added a merge commit. @Kodiologist is this commit structure OK?

[Kodiologist]

I don't like to keep merge commits inside a PR (as opposed to the merge commit that brings the PR into master), so I rebased it away.

[Kodiologist]

Thanks!