Updating the Clojurescript Checker

456 views
Skip to first unread message

Vincent Heuken

unread,
Nov 29, 2015, 4:55:47 PM11/29/15
to core.typed
I recently made an issue on the typed clojurescript sample project's github regarding core.typed not working on the latest version of Clojurescript. You mentioned you had a list of things to get it working again. I'd love to try and help get it working again.

Ambrose Bonnaire-Sergeant

unread,
Nov 29, 2015, 5:53:46 PM11/29/15
to core.typed
Hi Vincent,

Awesome!

This issue needs to be resolved.

Here's what to do.

2. Notice how the implementation of tools.analyzer.js conforms to this spec.
    eg. case*, or delegates to tools.analyzer, eg. try
3. Compare this spec to the actual ClojureScript analyzer.
    eg. try
4. Note any differences between the specs.
    eg. :catch in ClojureScript :try node, vs :catches vector in tools.analyzer :try node.
5. Prepare a patch for the ClojureScript analyzer that updates the AST to conform to tools.analyzer.

I'm available to video chat over the week if that helps.

Thanks,
Ambrose
Message has been deleted

Vincent Heuken

unread,
Nov 30, 2015, 3:06:27 AM11/30/15
to core.typed
Thanks for getting back to me.

I won't have time to start for a couple of days, but I wanted to go in and try something simple to make sure I have the right idea, so I tried my hand at throw. The diff is below. Let me know if this is right or of I'm way off.

---
diff --git a/src/main/clojure/cljs/analyzer.cljc b/src/main/clojure/cljs/analyzer.cljc
index 437c8a0..6eb5b6b 100644
--- a/src/main/clojure/cljs/analyzer.cljc
+++ b/src/main/clojure/cljs/analyzer.cljc
@@ -994,8 +994,8 @@
   [op env [_ throw :as form] name _]
   (let [throw-expr (disallowing-recur (analyze (assoc env :context :expr) throw))]
     {:env env :op :throw :form form
-     :throw throw-expr
-     :children [throw-expr]}))
+     :exception throw-expr
+     :children [:exception]}))
 
 (defmethod parse 'try
   [op env [_ & body :as form] name _]

Ambrose Bonnaire-Sergeant

unread,
Nov 30, 2015, 3:15:49 AM11/30/15
to core.typed
Yes, that's a good start. You should also start thinking about how this affects the rest of the analyzer/compiler. eg. :throw is now :exception, and :children traversal is drastically different and keep notes about that too.

Eventually the patch will incorporate these changes, but I would leave them until last.

Thanks,
Ambrose

Sean Walker

unread,
Jan 22, 2016, 11:05:59 AM1/22/16
to core.typed
Has any progress been made on this? I am interested in helping get this updated, so I'm willing to assist/collaborate if you're still working on this, or look into it myself otherwise.

Vincent Heuken

unread,
Jan 22, 2016, 8:42:24 PM1/22/16
to clojure-c...@googlegroups.com
Unfortunately I haven't had time. Feel free to start on it yourself if you'd like. 
--
Vincent Heuken

Ambrose Bonnaire-Sergeant

unread,
Jan 23, 2016, 8:31:16 PM1/23/16
to core.typed
No progress from me either.

Sean, the instructions earlier in the thread should still be a good starting point to work on.

Thanks,
Ambrose

Vincent Heuken

unread,
Jan 23, 2016, 9:59:13 PM1/23/16
to clojure-c...@googlegroups.com
Sean, if you do end up starting, could you link to your Github (or wherever)? I'd like to watch progress and possibly help out if I can.
--
Vincent Heuken

Sean Walker

unread,
Jan 26, 2016, 2:29:51 AM1/26/16
to core.typed
Sure, my github is https://github.com/AnOctopus. I'll likely start on it later this week, once I've cleared a few other things off of my plate.

Sean Walker

unread,
Jan 30, 2016, 8:03:19 PM1/30/16
to core.typed
I have tried adjusting a few more forms to match the tools.analyzer ast, and I want to make sure I'm on the right track as well. The commit is here. I'm not sure about a few things (is ctor just class, or something else?), but I think I'm starting to understand how this part of the analyzer works.

Ambrose Bonnaire-Sergeant

unread,
Jan 30, 2016, 8:40:35 PM1/30/16
to core. typed

Looks good!

One strategy that helps is to evaluate a tools.analyzer.jvm AST and inspect the output.

Thanks,
Ambrose

Sean Walker

unread,
Jan 31, 2016, 1:14:21 AM1/31/16
to clojure-c...@googlegroups.com
Yeah, a repl is proving useful (I'd been without it due to temporary issues with cider). It looks like the clojurescript analyzer is generating the constructor node, rather than a class node. It shouldn't be too hard to convert it to the right form.

Sean Walker

unread,
Feb 7, 2016, 11:54:11 PM2/7/16
to clojure-c...@googlegroups.com
Is there some way to get a a copy of my repl environment (or something else easy to set up) as an :env value? I'd like to poke at var a bit, but since it needs the var to exist, and the only premade environment seems to be the empty one, I'm stuck using do and pulling it out of that.

Ambrose Bonnaire-Sergeant

unread,
Feb 8, 2016, 2:07:37 AM2/8/16
to core.typed
I believe cljs.env contains the state of a REPL environment. That might be a good place to start looking.

Thanks,
Ambrose

Sean Walker

unread,
Feb 8, 2016, 8:48:19 PM2/8/16
to clojure-c...@googlegroups.com
cljs.env didn't have quite what I was looking for. Calling (default-compiler-env) in the repl causes it to hang, and at this point it's going to be easier to either construct a minimal environment by hand or just stick to extracting info from analyzing a do form.

Sean Walker

unread,
Mar 10, 2016, 7:56:44 PM3/10/16
to clojure-c...@googlegroups.com
It's been about a month since my last update, so I thought I would share
the progress since then. It's not much, mostly analysis, because I've
been rather busy with interviews and other related job stuff.

I've determined that the quote form will need to be created from
scratch, because tools.analyzer has an actual quote node and cljs just
sets a flag in the environment. It's not particularly complicated of a
form, though.

Let, loop, and recur are a bit messy, because loop and let are the same
form with some extra info (loop target) stored for loop. It looks like
the :info map contains the same info as the parent node, so it should
maybe be changed the way :children has been. (I only seem to have one,
unlabeled ast form for loop in my notes, so I'll have to see what
changes need to happen.)
>>>>>> <https://github.com/AnOctopus/clojurescript/commit/07e553bd54945238c819795c4164a9f65d0ca23f>.
>>>>>>>>>>>> <http://clojure.github.io/tools.analyzer.js/spec/quickref.html#throw>. The
>>>>>>>>>>>> diff is below. Let me know if this is right or of I'm way off.
>>>>>>>>>>>>
>>>>>>>>>>>> ---
>>>>>>>>>>>> diff --git a/src/main/clojure/cljs/analyzer.cljc
>>>>>>>>>>>> b/src/main/clojure/cljs/analyzer.cljc
>>>>>>>>>>>> index 437c8a0..6eb5b6b 100644
>>>>>>>>>>>> --- a/src/main/clojure/cljs/analyzer.cljc
>>>>>>>>>>>> +++ b/src/main/clojure/cljs/analyzer.cljc
>>>>>>>>>>>> @@ -994,8 +994,8 @@
>>>>>>>>>>>> [op env [_ throw :as form] name _]
>>>>>>>>>>>> (let [throw-expr (disallowing-recur (analyze (assoc env
>>>>>>>>>>>> :context :expr) throw))]
>>>>>>>>>>>> {:env env :op :throw :form form
>>>>>>>>>>>> - :throw throw-expr
>>>>>>>>>>>> - :children [throw-expr]}))
>>>>>>>>>>>> + :exception throw-expr
>>>>>>>>>>>> + :children [:exception]}))
>>>>>>>>>>>>
>>>>>>>>>>>> (defmethod parse 'try
>>>>>>>>>>>> [op env [_ & body :as form] name _]
>>>>>>>>>>>>
>>>>>>>>>>>> On Sunday, November 29, 2015 at 2:53:46 PM UTC-8, Ambrose
>>>>>>>>>>>> Bonnaire-Sergeant wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Vincent,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Awesome!
>>>>>>>>>>>>>
>>>>>>>>>>>>> This issue <http://dev.clojure.org/jira/browse/CLJS-1461>
>>>>>>>>>>>>> needs to be resolved.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Here's what to do.
>>>>>>>>>>>>>
>>>>>>>>>>>>> 1. Look over the tools.analyzer.js AST format
>>>>>>>>>>>>> <http://clojure.github.io/tools.analyzer.js/spec/quickref.html>
>>>>>>>>>>>>> 2. Notice how the implementation of tools.analyzer.js conforms
>>>>>>>>>>>>> to this spec.
>>>>>>>>>>>>> eg. case*
>>>>>>>>>>>>> <https://github.com/clojure/tools.analyzer.js/blob/eec5e3dd3fbeb306511aa8cdbeb663bac94392f9/src/main/clojure/clojure/tools/analyzer/js.clj#L290-L325>,
>>>>>>>>>>>>> or delegates to tools.analyzer, eg. try
>>>>>>>>>>>>> <https://github.com/clojure/tools.analyzer/blob/f5d959f4e9e12a0a9d3d74293d9ff2ca0ad65ad4/src/main/clojure/clojure/tools/analyzer.clj#L373-L404>
>>>>>>>>>>>>> 3. Compare this spec to the actual ClojureScript analyzer.
>>>>>>>>>>>>> eg. try
>>>>>>>>>>>>> <https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/analyzer.cljc#L1000-L1059>
>>>>>>>>>>>>> 4. Note any differences between the specs.
>>>>>>>>>>>>> eg. :catch in ClojureScript
>>>>>>>>>>>>> <https://github.com/clojure/clojurescript/blob/3e1af212e114a9968c6f69719b0346df12bd6d0e/src/main/clojure/cljs/analyzer.cljc#L1058> :try
>>>>>>>>>>>>> node, vs :catches vector in tools.analyzer
>>>>>>>>>>>>> <https://github.com/clojure/tools.analyzer/blob/1c8693b30e8459ad3fbdb21871164ba82afb7b43/src/main/clojure/clojure/tools/analyzer.clj#L400>
>>>>>>>>>>>>> :try node.
>>>>>>>>>>>>> 5. Prepare a patch for the ClojureScript analyzer that updates
>>>>>>>>>>>>> the AST to conform to tools.analyzer.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'm available to video chat over the week if that helps.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Ambrose
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Sun, Nov 29, 2015 at 4:55 PM, Vincent Heuken <
>>>>>>>>>>>>> vhe...@gmail.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> I recently made an issue on the typed clojurescript sample
>>>>>>>>>>>>>> project's github
>>>>>>>>>>>>>> <https://github.com/frenchy64/typed-clojurescript-play/issues/1> regarding

Ambrose Bonnaire-Sergeant

unread,
Mar 10, 2016, 9:11:28 PM3/10/16
to core. typed

Awesome!

Sean Walker

unread,
Mar 20, 2016, 9:47:49 PM3/20/16
to clojure-c...@googlegroups.com
Figured I should try to provide updates semi-frequently. I updated all
of the analyzer/emitter usage of :var to :the-var, because that's what
tools.analyzer uses.

I've also finished up loop/let bindings and bodies, although there's
some extra stuff in each. Some of that seems to be cljs specific, like
setting the bindings :op to :the-var so the compiler routes them
correctly, and the use of *recur-frames*. It's unclear what is essential
and what could be cleaned up, but it's not going to interfere with
anything to leave it in, so I'll leave that be for now.

I also did a few changes to recur, but I haven't verified that I got everything.

Ambrose Bonnaire-Sergeant

unread,
Mar 20, 2016, 10:09:18 PM3/20/16
to core.typed
Are you sure the distinction between :var and :the-var is correct?

:var is for var references like `+`.

:the-var is for var expressions like `#'+`.

Thanks,
Ambrose

Sean Walker

unread,
Mar 20, 2016, 10:47:37 PM3/20/16
to clojure-c...@googlegroups.com
I was going off of parse-var returning :the-var, but I do now see that
analyze-symbol returns :var. I hadn't noticed the existence of both :var
and :the-var in the docs.

Is it correct that parse 'var should be :the-var, and analyze-symbol
should be :var? That shouldn't be too difficult to switch to, although
it means adding that case in the emitter, since it was all :var before.

(This is why I wanted to provide more updates; it keeps me from missing
things like this, so I can stay on track.)

Sean Walker

unread,
Mar 29, 2016, 8:54:22 PM3/29/16
to clojure-c...@googlegroups.com
So, I've had the chance to look through this again. It seems that :var
is probably just :var, and that the cljs :var-special is what serves the
role of the t.a :the-var. Does this sound about right?

Also, is there anywhere with documentation similar to the t.a quickref,
for the cljs analyzer?

Sean

Ambrose Bonnaire-Sergeant

unread,
Mar 29, 2016, 8:57:22 PM3/29/16
to core.typed
I'm not aware of any documentation, hand tracing the AST to the emitter has
been my best bet.

Can you link to the :var-special code?

Thanks,
Ambrose

Sean Walker

unread,
Mar 29, 2016, 9:04:05 PM3/29/16
to clojure-c...@googlegroups.com

>Can you link to the :var-special code?

This is the emitter dispatch:
https://github.com/clojure/clojurescript/blob/16666f37cc13ead5a66330046db82a2976b6f1f0/src/main/clojure/cljs/compiler.cljc#L327

The only other uses are in the emit :def method and the parse 'var method.

Ambrose Bonnaire-Sergeant

unread,
Mar 29, 2016, 9:19:42 PM3/29/16
to core.typed
Yes, since CLJS :var-special nodes correspond to (var ..) expressions
(as evidenced by the parse 'var method), they correspond to :the-var
in t.a.

Thanks,
Ambrose

Sean Walker

unread,
Apr 6, 2016, 8:09:37 PM4/6/16
to clojure-c...@googlegroups.com
Started working on writing the :quote form. It's really straightforward,
except I don't see anything equivalent to analyze-const to use for
analyzing the form. Is there one that I'm missing, or should I copy over
the t.a one? (it seems odd that there wouldn't be one, so I suspect I'm
just missing it). If it's missing, then I need something to handle
getting the type, which also doesn't seem to exist as-is.

Ambrose Bonnaire-Sergeant

unread,
Apr 15, 2016, 2:28:31 PM4/15/16
to core.typed
Hi Sean,

Sorry for the silence.

Did you get any further on this? I've talked to Nicola Mometto about the differences in :quote
in various compilers and it's rather subtle. He might be the right person if you're still stuck.

Thanks for you work! Looking forward to your updates.
Ambrose

Sean Walker

unread,
May 12, 2016, 11:33:01 PM5/12/16
to clojure-c...@googlegroups.com
Hey Ambrose,

I emailed Nicola earlier this week, but haven't gotten a response yet.

Could you elaborate on what kind of subtle differences there are? I can
see that the compiler uses a flag in the env rather than producing a
distinct node, but if there's a big difference beyond that, I haven't
seen it yet.

(I'm finally settled into my new job, so I'm hoping to apply much more
time/effort to this project now.)


Thanks,
Sean

Sean Walker

unread,
Jul 31, 2016, 10:27:55 PM7/31/16
to clojure-c...@googlegroups.com
Ok, I didn't get a followup and working on this kinda fell off my radar
in favor of job related things (lots to learn as a devops setting up
cloud infrastructure from scratch). I imagine this project might still
be useful, and I feel like I could make much more progress now.

Are the analyzer parse methods and the emitter the only components that
need updating? Actually, can you outline the tasks that need to be done
for the compiler to produce information needed for core.typed? It would
be helpful in focusing my efforts.


Sean

Ambrose Bonnaire-Sergeant

unread,
Aug 8, 2016, 5:19:53 AM8/8/16
to core.typed
Hi Sean,

The parse and emit functions are a good start, I'm not sure
where else would be needed (probably a few loose ends around
the place, eg. helper functions on AST nodes).

The input to core.typed is the output of `parse`, so that component
is particularly important to core.typed. All of the information needed is already
in the AST, it's just in a non-standard format. All of the :op's
and their :children should be standardised based on the tools.analyzer
AST.

Thanks,
Ambrose

Ambrose Bonnaire-Sergeant

unread,
Aug 8, 2016, 7:38:15 AM8/8/16
to core.typed
I found a few dozen places where workarounds are needed in core.typed
because of CLJS's non-standard AST.


Thanks,
Ambrose

Sean Walker

unread,
Apr 23, 2017, 1:14:37 PM4/23/17
to core.typed
6ish months later, I think I'm done with the :children conversion and I'm starting on some of the differences that require impl-case.

There's still probably going to be a lot of changes needed after, but I'd like to get at least that much done soon.

Ambrose Bonnaire-Sergeant

unread,
Apr 23, 2017, 2:21:50 PM4/23/17
to core.typed
Hi Sean,

I did some experimentation of my own in the mean time.

I think I got most of the way there, IIRC I implemented the :children conversion.
There was a few outstanding issues but I forgot what they were.

You might want to have a look here.

Thanks,
Ambrose

Sean Walker

unread,
Apr 23, 2017, 2:52:33 PM4/23/17
to core.typed
Hmm, yeah. I've already spotted one thing that still needs changing. I'll do a sweep through and either fix or note remaining stuff. Your changes are more complete than mine so I'll work from that.

Sean Walker

unread,
Apr 23, 2017, 9:04:32 PM4/23/17
to core.typed
I have opened a PR for two conversions that currently require impl-case.

There's a bunch more stuff that doesn't directly match up, but that doesn't seem to be affecting core.typed at least. Those will require a more careful comparison with the tools.analyzer spec.

Ambrose Bonnaire-Sergeant

unread,
Jul 12, 2017, 4:59:57 PM7/12/17
to core.typed
Sorry about the slow turnaround. I've made some time to review your PR and
think further issues.

I can't quite remember the state of this work, but for me, these are the latest
proposed changed to core.typed, and this is the latest for clojurescript.

Amanda Walker

unread,
Sep 12, 2017, 1:43:01 AM9/12/17
to core.typed
So, I've unfortunately been busy as well. I just moved (again), but I'm now relatively settled in and would like to give this another good go. I had previously been stuck trying to get the changes to pass all the tests, due to what was an extremely difficult to find bug triggering in parts of the code that hadn't changed in years and had no reason to fail that way.

I take it the end goal is to update the compiler, and then update core.typed to no longer need the impl-case workarounds? Are there any other things that will also need to be changed (by one of us)?

Ambrose Bonnaire-Sergeant

unread,
Sep 12, 2017, 8:12:03 AM9/12/17
to core.typed
Hi Amanda,

So I propose a different approach to the CLJS analyzer side of things, with the end goal of eliminating
all the impl-case workarounds.

Let's just write our own CLJS AST -> TAJ AST conversion, and convert before we call `check`.
This way, we're not dependent on any upstream CLJS changes, and we can finally have a working
CLJS checker again.

Thoughts?

Thanks,
Ambrose

Ambrose Bonnaire-Sergeant

unread,
Sep 12, 2017, 8:59:49 AM9/12/17
to core.typed
Alternatively, we could fork the CLJS analyzer like I have with Compiler.java. There are
other experiments with macroexpansion that I'd like to do that might also benefit from a forked
analyzer.

Ambrose Bonnaire-Sergeant

unread,
Sep 13, 2017, 7:58:54 AM9/13/17
to core.typed
First, I'll try a little harder to move the upstream changes along. Perhaps Amanda
can take the lead on the sequence of patches on that issue if we get the go ahead.

Amanda Walker

unread,
Sep 15, 2017, 11:44:55 PM9/15/17
to core.typed
Yeah, I'm willing to work with them on getting the patches accepted if they are willing to move forward with it.

As for the alternatives, I'm curious what kind of experiments you have in mind.

Ambrose Bonnaire-Sergeant

unread,
Sep 16, 2017, 12:43:55 AM9/16/17
to core.typed
Could you rebase the first patch I prepared and reupload?

I want to play around with delaying macroexpansion so we can better type check things like core.match expressions.

Ambrose

Amanda Walker

unread,
Sep 25, 2017, 11:31:41 PM9/25/17
to core.typed
I rebased the first patch and attached it to that issue. It should be good to go, though I haven't submitted any patches for clojure(script) before, so let me know if I did something wrong with it.

Amanda Walker

unread,
Oct 13, 2017, 3:25:34 PM10/13/17
to core.typed
Still waiting on this, although with clojure/conj going on right now isn't a great time.
Reply all
Reply to author
Forward
0 new messages