Clojure

'list*' returns not a list

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Trivial Trivial
  • Resolution: Unresolved
  • Affects Version/s: Release 1.4
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test

Description

Function 'list?' returns sequence, but not a list.
It is a bit confusing.

user=> (list? (list* 1 '(2 3)))
false

Activity

Hide
Marek Srank added a comment -

Yep, these are all valid points, thanks! I see this as a question whether the list* function is a list constructor or not. If yes (and it would be possible to implement it in a satisfactory way), it should probably return a list.

We could avoid building a new list by sth like:

(if (list? args)
  args
  (apply list args))

(btw, 'vec' also creates a new vector even when the argument itself is a vector)

The contract of next seems to be to return a seq, not a list - its docstring reads: "Returns a seq of the items after the first. Calls seq on its argument. If there are no more items, returns nil."

Btw, in some Lisp/Scheme impls I checked, cons seems to be a list as well. E.g. in CLisp (and similar in Guile and Racket):

> (listp (cons 2 '()))
T
> (listp (list* 1 2 '()))
T
Show
Marek Srank added a comment - Yep, these are all valid points, thanks! I see this as a question whether the list* function is a list constructor or not. If yes (and it would be possible to implement it in a satisfactory way), it should probably return a list. We could avoid building a new list by sth like:
(if (list? args)
  args
  (apply list args))
(btw, 'vec' also creates a new vector even when the argument itself is a vector) The contract of next seems to be to return a seq, not a list - its docstring reads: "Returns a seq of the items after the first. Calls seq on its argument. If there are no more items, returns nil." Btw, in some Lisp/Scheme impls I checked, cons seems to be a list as well. E.g. in CLisp (and similar in Guile and Racket):
> (listp (cons 2 '()))
T
> (listp (list* 1 2 '()))
T
Hide
Michał Marczyk added a comment -

I'm in favour of the "list" -> "seq" tweak to the docstring though, assuming impl remains unchanged.

Show
Michał Marczyk added a comment - I'm in favour of the "list" -> "seq" tweak to the docstring though, assuming impl remains unchanged.
Hide
Michał Marczyk added a comment -

(apply list args) would make (list* (range)) hang, where currently it returns a partially-realized lazy seq. Also, even for non-lazy seqs – possibly lists themselves – it would always build a new list from scratch, right?

Also, if I'm reading the patch correctly, it would make 2+-ary list* overloads and cons return lists – that is, IPersistentList instances – always (Conses would now be lists), but repeatedly calling next on such a list might eventually produce a non-list. The only way around that would be to make all seqs into IPersistentLists – that doesn't seem desirable at first glance...?

On a side note, for the case where the final "args" argument is in fact a list, we already have a "prepend many items" function, namely conj. list* currently acts as the vararg variant of cons (in line with Lisp tradition); I'm actually quite happy with that.

Show
Michał Marczyk added a comment - (apply list args) would make (list* (range)) hang, where currently it returns a partially-realized lazy seq. Also, even for non-lazy seqs – possibly lists themselves – it would always build a new list from scratch, right? Also, if I'm reading the patch correctly, it would make 2+-ary list* overloads and cons return lists – that is, IPersistentList instances – always (Conses would now be lists), but repeatedly calling next on such a list might eventually produce a non-list. The only way around that would be to make all seqs into IPersistentLists – that doesn't seem desirable at first glance...? On a side note, for the case where the final "args" argument is in fact a list, we already have a "prepend many items" function, namely conj. list* currently acts as the vararg variant of cons (in line with Lisp tradition); I'm actually quite happy with that.
Hide
Marek Srank added a comment - - edited

The question is what to do with the one-argument case of list*, because in cases like: (list* {:a 1 :b 2}) it doesn't return IPersistentList as well. I propose just applying 'list'.

I added patch 'list-star-fix.diff' (dated 04/Jan/2013) with Cons implementing IPersistentList and doing (apply list args) in one-argument case of list*. To be able to use 'apply' in list* I had to declare it before the definition of list* in the source code. The apply itself also uses list*, but luckily not the one-argument version of list*, so it should be safe... The patch also contains simple unit tests.

Discussion is also here: https://groups.google.com/forum/#!topic/clojure/co8lcKymfi8

Show
Marek Srank added a comment - - edited The question is what to do with the one-argument case of list*, because in cases like: (list* {:a 1 :b 2}) it doesn't return IPersistentList as well. I propose just applying 'list'. I added patch 'list-star-fix.diff' (dated 04/Jan/2013) with Cons implementing IPersistentList and doing (apply list args) in one-argument case of list*. To be able to use 'apply' in list* I had to declare it before the definition of list* in the source code. The apply itself also uses list*, but luckily not the one-argument version of list*, so it should be safe... The patch also contains simple unit tests. Discussion is also here: https://groups.google.com/forum/#!topic/clojure/co8lcKymfi8
Hide
Timothy Baldridge added a comment -

Is there a reason why we can't have list* actually return a list? The cost of creating a list vs a cons is negligible.

Show
Timothy Baldridge added a comment - Is there a reason why we can't have list* actually return a list? The cost of creating a list vs a cons is negligible.
Hide
Stuart Halloway added a comment -

should the docstring for list* change to say it returns a seq?

Show
Stuart Halloway added a comment - should the docstring for list* change to say it returns a seq?

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated: