ClojureScript

Compiler emits invalid JS code for NaN/Inf/-Inf if used with CLJ >= 1.9.0-alpha20

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: 1.9.908
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test
  • Approval:
    Accepted

Description

Since 1.9.0-alpha20, Clojure has its own external representations for NaN/Inf/-Inf: ##NaN / ##Inf / ##-Inf, respectively.

The current CLJS compiler emits code as is for those values, so if it is used with the latest Clojure, it will end up emitting invalid JS code:

=> (let [env (assoc (ana/empty-env) :context :expr)]
     (comp/emit-str (ana/analyze env ##NaN)))
"##NaN"
=> (let [env (assoc (ana/empty-env) :context :expr)]
     (comp/emit-str (ana/analyze env ##Inf)))
"##Inf"
=> (let [env (assoc (ana/empty-env) :context :expr)]
     (comp/emit-str (ana/analyze env ##-Inf)))
"##-Inf"
  1. CLJS-2352.patch
    12/Sep/17 12:43 AM
    1 kB
    Shogo Ohta
  2. CLJS-2352-2.patch
    17/Sep/17 12:54 AM
    2 kB
    Shogo Ohta

Activity

Hide
Shogo Ohta added a comment -

Added a patch.
I tested the code on my local env and saw the things worked fine, but I'm not sure I should have added CLJ 1.9.0-alpha for test profile.

Show
Shogo Ohta added a comment - Added a patch. I tested the code on my local env and saw the things worked fine, but I'm not sure I should have added CLJ 1.9.0-alpha for test profile.
Hide
David Nolen added a comment -

Patch looks fine but could we please add a test case.

Show
David Nolen added a comment - Patch looks fine but could we please add a test case.
Hide
Shogo Ohta added a comment -

Updated the patch.

Is it OK to directly use the raw Java constants for NaN/Infinity/-Infinity in test cases since we can't use new literals for those values yet until the dependent tools.reader is updated to 1.1.0?

Show
Shogo Ohta added a comment - Updated the patch. Is it OK to directly use the raw Java constants for NaN/Infinity/-Infinity in test cases since we can't use new literals for those values yet until the dependent tools.reader is updated to 1.1.0?

People

Vote (3)
Watch (6)

Dates

  • Created:
    Updated:
    Resolved: