ClojureScript

*cljs-data-readers* is bound to *data-readers* inconsistently

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

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.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Herwig Hochleitner added a comment -

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.

Show
Herwig Hochleitner added a comment - 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.
Hide
Permalink
Herwig Hochleitner added a comment -

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.

Show
Herwig Hochleitner added a comment - 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.
Hide
Permalink
David Nolen added a comment -

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

Show
David Nolen added a comment - Please move the test script patch to a different ticket. Thanks.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.