<< Back to previous view

[CLJS-480] *cljs-data-readers* is bound to *data-readers* inconsistently Created: 01/Mar/13  Updated: 23/Nov/13  Resolved: 23/Nov/13

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-480-bind-data-readers-in-parse-ns.patch     Text File 0002-Bring-back-multiple-test-compilations-and-overhaul-o.patch    

 Description   

It looks like *cljs-data-readers* is not being bound to *data-readers* where it should be.

At a minimum, cljs.repl/load-stream does not. The practical effect of this instance is that e.g. (load-file "some/cljs/file.cljs") will fail if that file or any other contains a user-defined literal tag. Similar behaviour exists elsewhere, e.g. (load-namespace 'some.cljs.ns) will not load a namespace that contains user-defined tags (I think that one is due to *cljs-data-readers* not being bound in the analyzer).

This is related to CLJS-479 insofar as it's another symptom of ClojureScript loading code in subtly different ways in multiple places.

I'm happy to do either of:

  1. submit issues+patches fixing issues as I encounter them, or
  2. attempt a more significant refactoring that sets up (hopefully) one and only one place where ClojureScript reads code (i.e. uses LispReader) so that behaviour can be consistent

I'm somewhat hesitant re: (2) insofar as the specific requirements of the compiler and analyzer and REPL may be divergent in ways that I'm not yet aware of, but it seems like it's worth a shot.



 Comments   
Comment by Herwig Hochleitner [ 17/May/13 3:10 PM ]

Attached patch 0001 binds data-readers in cljs.compiler/parse-ns.

This allows the compiler to run, even if files with reader tags are already compiled.

This in turn allows multiple test compilations to be brought back, which I'll submit as part of a patch for an additional debugging flag, if nobody beats me to it.

I think this won't fix load-file , but after the hotfix we should probably look for the proper place in the compiler to bind data-readers.

Comment by Herwig Hochleitner [ 17/May/13 5:25 PM ]

Patch 0002 is my overhaul for the test script.
Since it's a test case for patch 0001, I think it's justifiable on this ticket.

Note: On my machine, with SpiderMonkey 185, the test case (not (integer? 1e+308)) fails. It probably doesn't have to do anything with the removed -n flag (there is no js -n omm), I just wanted to mention it.

Comment by David Nolen [ 21/May/13 10:06 AM ]

Please move the test script patch to a different ticket. Thanks.

Comment by David Nolen [ 23/Nov/13 10:48 PM ]

fixed, https://github.com/clojure/clojurescript/commit/a1514b18d1367295b7dd0405591e5fa5b580c529

Generated at Wed Apr 23 09:26:04 CDT 2014 using JIRA 4.4#649-r158309.