Fork me on GitHub
#clojure-dev
<
2020-03-25
>
favila12:03:25

(empty lazyseq) does not copy metadata. This seems like a bug?

alexmiller12:03:41

can you give an example?

gfredericks13:03:09

next and rest won't copy metadata either -- is this specifically about the case where lazyseq is empty?

gfredericks13:03:40

I assume at the moment empty doesn't even check if the arg is empty, and for a lazy seq that check could potentially block forever

favila13:03:41

(def x (with-meta (map inc (range 10)) {:foo :bar}))
=> #'user/x
(meta x)
=> {:foo :bar}
(meta (empty x))
=> nil

favila13:03:15

the real example is from a json array decoded by cheshire, but it is also a clojure.lang.LazySeq

gfredericks13:03:37

yeah I don't think seqs in general are expected to have the same metadata for different sub-sequences of them because of the way cons works, a seq is sort of like a vector but also sort of like a (linear) tree structure in the tree view it makes as much sense to ask whether next or empty should preserve metadata as it does to expect that when you pull a value out of a map that value should have the same metadata as the map

favila13:03:39

then why does lazy-seq have a metadata field at all?

gfredericks13:03:42

there'd have to be a lot more copying to support metadata preservation; like if I have a seq, and then tweak its metadata, that'd ultimately be a linear time operation instead of constant time, because it'd have to be tweaked for each next

favila13:03:52

(meta (with-meta (rest (map inc (range 10))) {:foo :bar}))
=> {:foo :bar}

gfredericks13:03:57

well it at least has it because the compiler uses it for syntax

favila13:03:09

also I don’t see why getting metadata would realize a seq

gfredericks13:03:13

symbols have metadata

gfredericks13:03:24

I didn't say anything about realizing

favila13:03:42

> I assume at the moment empty doesn’t even check if the arg is empty, and for a lazy seq that check could potentially block forever

gfredericks13:03:45

I'm saying (with-meta (range 10) {:foo :bar}) is a linear-time operation

favila13:03:01

(empty lazy-seq) returns a persistentlist

favila13:03:04

I’m fine with that

favila13:03:07

that’s expected

gfredericks13:03:13

that comment was about an alternate interpretation of your original question where you were only interested in preserving metadata when the arg was empty

favila13:03:22

but what’s not expected is that it doesn’t have the metadata of the original lazyseq

gfredericks13:03:02

I realize it's surprising, and that's unfortunate, I'm just pointing out why changing this behavior would be problematic in other ways

favila13:03:41

I didn’t catch that. why would it be problematic?

gfredericks13:03:38

so I'm talking about how metadata on seqs works more broadly does this do what you expect or would want?

user> (def x0 (with-meta () {:empty :meta}))
#'user/x0
user> (def x1 (with-meta (cons 1 x0) {:one :thing}))
#'user/x1
user> (meta x0)
{:empty :meta}
user> (meta x1)
{:one :thing}
user> (meta (rest x1))
{:empty :meta}

gfredericks13:03:25

in particular the last two steps, where (meta x1) is different from (meta (rest x1))

favila13:03:00

yes, that is all fine

favila13:03:10

I’m really more concerned with empty though

gfredericks13:03:20

so what should (meta (empty x1)) do for that particular x1?

gfredericks13:03:33

should it give you the metadata that x1 has, or the metadata that x0 has?

alexmiller13:03:24

I looked at the code and agree this is a bug

😂 1
favila13:03:35

public IPersistentCollection empty(){
                                     PersistentList o PersistentList.EMPTY;
                                     return o.withMeta (o, meta ())
                                     }

favila13:03:43

is basically what I expect this to be

favila13:03:52

(not tested, formatted weird because typed into a repl)

alexmiller13:03:55

in general, most of the colls do retain metadata on empty

alexmiller13:03:17

that seems to be missing on ASeq.empty() (which affects many subclasses) and LazySeq.empty()

alexmiller13:03:56

some random subclasses do actually override and fix that, like ChunkedSeq.empty()

favila13:03:03

The reason I ask is because empty doesn’t explicitly say “copies metadata” but I know bugs have been filed and fixed against it not copying in the past, and I also intuitively expect it to copy

alexmiller13:03:19

in general, all collection operations are expected to retain metadata

alexmiller13:03:29

(not all seq operations though)

alexmiller13:03:00

empty is a collection operation though (and seqs are logical collections), so I think all that holds

👍 2
alexmiller13:03:13

do you have jira access?

alexmiller13:03:49

if you can file a ticket, that'd be great

seancorfield21:03:58

FYI, we have Clojure 1.10.2 Alpha 1 in production (as of yesterday morning) and -- shock, horror -- it's running just fine, as expected.

🎉 4
🙃 1