Fork me on GitHub
#squint
<
2023-02-20
>
lilactown16:02:53

I noticed that doseq emits things like _nth, unchecked_inc , chunk_first and chunk_rest. I don't see imports for those though. How does that work?

borkdude16:02:30

@lilactown doseq might work in cherry but not in squint, similar to for, I think it might be better to disable it in squint until a good replacement is implemented

borkdude16:02:49

Hmm, it seems to work:

borkdude@m1 /tmp/dude $ npx cherry run script.cljs
[cherry] Running script.cljs
1
2
3
borkdude@m1 /tmp/dude $ npx squint run script.cljs
[squint] Running script.cljs
1
2
3

borkdude16:02:59

(ns script)

(doseq [x [1 2 3]]
  (prn x))

lilactown16:02:04

yeah I've been playing with it

borkdude16:02:06

I think it works because it never hits the paths with unchecked-inc etc

lilactown16:02:12

aha ๐Ÿ˜„

lilactown16:02:07

I started with wanting to play with implementing for. maybe I should start with doseq

๐Ÿ‘ 2
borkdude16:02:57

In cherry for does work:

(ns script)

(prn
 (for [x [1 2 3]
       y [4 5 6]]
   [x y]))

lilactown16:02:01

just so I understand the project dependencies: if I'm editing squint/src/squint/internal/macros.cljc, I don't need to worry about cherry compat right?

borkdude16:02:28

only the compiler_common code is shared with cherry

lilactown16:02:46

so cherry does depend on squint, just the compiler-common ns. got it

borkdude16:02:54

I tried a few different approaches with git submodules, git history rewriting, etc, etc but I ended up with this to reduce the amount of hassle

lilactown16:02:32

yeah git submodules is a PITA

borkdude16:02:43

not only that but I wanted to have the common part in its own repo but also a way to run tests against that. then I tried having everything in one repo but then with history rewriting so cherry and squint could keep their own repositories (with their own issues, stars, etc)

borkdude16:02:34

but I think when tests are run against squint when the common compiler code changes, it's sufficient for now

lilactown16:02:44

I think it would be really nice if doseq emitted something like for (let x of seq) { ,,, }

lilactown17:02:05

I think I need a new special form

borkdude17:02:23

you can do this with js* probably?

lilactown17:02:20

maybe yeah. I'll try that

lilactown17:02:45

that worked!

lilactown17:02:29

I think I could use a lot of the same code for for, but I need to construct a lazy iterable. do you think we could export the constructor for that?

borkdude18:02:35

Yeah, maybe we could do it like __LazyIterable to indicate that it's a constructor only we can use

borkdude18:02:13

I made a comment in the PR about the usage of seq: note that this currently doesn't work correctly for (range .. ..) like things: seq never returns nil I think

borkdude19:02:54

The next function also hinges on that

borkdude19:02:03

Maybe we should go with some memoized sequence type that memorizes the realized elements like in Clojure...

borkdude19:02:04

This could maybe wrap all the existing iterator stuff

lilactown19:02:13

doseq doesn't use anything from the core lib, it's actually all JS

lilactown19:02:21

it literally emits

for (let ~{} of ~{}) { ~{} }
which works with all of our LazyIterables since for ... of calls the Symbol.iterator function under the hood

borkdude19:02:07

I'm already in a rabbit hole of implementing IteratorSeq from CLJS which is basically the immutable layer we could use to make seq and next work maybe, while also preventing multiple side effects when walking the seq twice. I realize it's now unrelated to your PR, good to know.

borkdude19:02:31

class IteratorSeq {
  constructor(value, iter) {
    this.iter = iter;
    this.value = value;
    this.rest = null;
  }
  [IIterable] = true;
  [Symbol.iterator]() {
    yield value;
    yield iteratorSeq(iter);
  }
}

function iteratorSeq(iter) {
  let v = iter.next();
  if (v.done) {
    return null;
  } else {
    return new IteratorSeq(v.value, iter);
  }
}

lilactown19:02:00

why don't seq and next work? because it tries to realize the whole seq?

borkdude19:02:55

no, but seq returns nil when there's no more elements and we can't decide that without realizing the first element once for the nil test and then later again when someone really wants the element

lilactown19:02:00

JS Iterators being mutable just sucks

borkdude19:02:22

yes, but with this iteratorseq you have caching

borkdude19:02:19

(this is unfinished code btw)

borkdude19:02:05

Merged the doseq stuff

lilactown19:02:30

I'm having trouble with for

lilactown19:02:37

when doseq is compiled, :let compiles to just let x = y. but similar code for for compiles to (function () { let x = y })() this is a problem because I'm trying to yield inside of that inner function, which doesn't work

borkdude20:02:59

This happens because you're in an expression context rather than a statement context I think

borkdude20:02:11

ok, watch this:

$ ./node_cli.js -e "(def xs [1 2 3]) (def ys (map prn xs)) (vec ys) (vec ys)"
1
2
3
1
2
3

borkdude20:02:18

$ ./node_cli.js -e "(def xs [1 2 3]) (def ys (iterator-seq (map prn xs))) (vec ys) (vec ys)"
1
2
3

lilactown20:02:51

how does the compiler know when it's in an expression context? can I manually twiddle that inside of the for macro impl?

borkdude20:02:28

this is a problem with js* , there's no way to currently tweak that, but maybe we can make that work somehow

borkdude20:02:46

the context is derived from the env that is passed around

borkdude20:02:10

so, it depends at the point in which you are in your CLJS expression where you call js*

lilactown20:02:11

I don't get it yet ๐Ÿ™‚ but I'll keep playing with it

borkdude20:02:30

maybe I got it wrong, I must take a look at your branch ;)

lilactown20:02:43

does that mean that if I had a special form that emitted for ... of I could control the context ?

borkdude20:02:26

what I mean is this:

$ ./node_cli.js --show -e "(if true (js* \"~{}\" (let [x 2])))"
if (true) {
let x1 = 2;
null};

borkdude20:02:35

vs:

$ ./node_cli.js --show -e "(let [x (if true (js* \"~{}\" (let [x 2])))])"
let x1 = (true) ? ((function () {
 let x2 = 2;
return null;
})()) : (null);
null;

borkdude20:02:13

so if you are inside a let binding, you are in an expression context

borkdude20:02:36

and expressions compiled by js* are emitted as expressions, rather than top level statements

borkdude20:02:54

in that case

lilactown20:02:25

that's not the case I'm running into, I think

lilactown20:02:55

here's the CLJS

(for [x (range 3)
      y (range 3)
      :let [z (+ x y)]]
  z)
and the output
import { lazy, range } from 'squint-cljs/core.js'
lazy(function* () {
for (let x of range(3)) {
for (let y of range(3)) {
(function () {
 let z1 = (x + y);
yield return z1;
})()
}
}
});

lilactown20:02:18

the proper output should be

import { lazy, range } from 'squint-cljs/core.js'
lazy(function* () {
for (let x of range(3)) {
for (let y of range(3)) {
let z1 = (x + y);
yield z1;
}
}
});

borkdude20:02:22

that does look like a similar problem: the js* env is inside an expr context which is why it's emitting a self-calling function

borkdude20:02:58

but maybe you should be able to "reset" the context in which js* starts, e.g. via metadata or so

borkdude20:02:04

or extra options

borkdude20:02:18

(js* {:context :statement} ...)

lilactown20:02:31

I'm curious why this works for doseq

borkdude20:02:41

probably lucky

lilactown20:02:13

that worked ๐Ÿ™‚

borkdude20:02:02

There is a bit of overhead of doing this cached thing:

$ ./node_cli.js -e "(def xs (range 1000)) (def ys (seq (map identity xs))) (time [(count (vec ys)) (count (vec ys))])"
"Elapsed time: 0.388041 msecs"

$ ./node_cli.js -e "(def xs (range 1000)) (def ys (es6-iterator-seq (map identity xs))) (time [(count (vec ys)) (count (vec ys))])"
"Elapsed time: 30.482041 msecs"
I think this is one of the reasons that CLJS implements chunking for these cached sequences

lilactown20:02:51

yeah it's basically a linked list. you have to chase the pointer of the tail all the time

lilactown20:02:04

it's funny how much of this stuff I forget, having not touched it for awhile ๐Ÿ™‚

lilactown20:02:53

maybe seq could return the cacheable seq

lilactown20:02:07

so you don't have to do it for every seq

lilactown21:02:43

idk, this stuff is hard. maybe Rich knew what he was doing when he implemented all of this ๐Ÿ˜„

borkdude21:02:51

yeah, I think optimally we'd move to cljs semantics, i.e. things are cached (side effects only once), but in an efficient way (chunks) and we have a Seq + Seqable protocol

borkdude21:02:28

for is merged!

borkdude21:02:31

the chunking happens only if the underlying seq is also chunked, else it's pretty much what I have in the iterator-seq branch

borkdude21:02:50

anyway this is a whole different topic :)

lilactown21:02:47

I hope it's OK that the for PR exposed lazy

lilactown21:02:53

meant to call that out

borkdude21:02:13

I guess it's fine

borkdude21:02:22

as long as we don't document it ;)

borkdude21:02:10

I exposed that caching thing as es6-iterable-seq - we can use that as a mechanism for caching, if people find that important, and keep the rest as is, for now. And maybe indeed, let seq return that automatically...

borkdude21:02:16

but what about first , rest etc. often you need both first and rest and currently you will do the calculation of the first element twice...

borkdude21:02:30

simply because we don't have a cached seq abstraction

borkdude21:02:11

for arrays that doesn't matter and keeping arrays as is is also good for perf

lilactown21:02:38

in JS, destructuring the tail of an iterator realizes the whole thing

lilactown21:02:53

what I'm learning here is fundamentally you need to embrace the mutability with iterators, or you need to do caching with immutability

lilactown21:02:00

๐Ÿงต for the core.js/core.edn issue I opened https://github.com/squint-cljs/squint/issues/294

lilactown21:02:04

the tests and other squint output also reference squint-cljs/core.js

lilactown21:02:28

it seems like it would be nice if I could make changes locally to core.js and everything would reference it inside of the squint project

lilactown21:02:06

I can think of two options 1. configure the squint compiler to output local paths instead of referencing the squint-cljs node package (maybe hard, not sure) 2. symlink node_modules/squint-cljs to the parent squint-cljs dir (not sure if this will work)

borkdude21:02:10

I think you just need to run npm install locally and then it already works

borkdude21:02:19

due to "squint-cljs": "."

borkdude21:02:22

in package.json

borkdude21:02:50

so I don't think the PR is necessary

lilactown21:02:25

ahhh I didn't realize that's what it was doing

lilactown21:02:31

my mistake ๐Ÿ™‚

borkdude22:02:08

maybe bb dev should do this automatically when there is no node_modules/squint-cljs yet