<< Back to previous view

[TRDR-50] Warning when using upper-limit in self-host cljs Created: 16/Jan/18  Updated: 16/Jan/18

Status: Open
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andrea Richiardi Assignee: Nicola Mometto
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File TRDR-50_1.patch    

 Description   

Hi!

I have found out that a warning is triggered when upper-limit is used in a self-host environment like lumo. For testing it out, the following can be done:

$ lumo
Lumo 1.8.0-beta
ClojureScript 1.9.946
Node.js v9.2.0
 Docs: (doc function-name-here)
       (find-doc "part-of-name-here")
 Source: (source function-name-here)
 Exit: Control+D or :cljs/quit or exit

cljs.user=> (def ^:private ^:const upper-limit (int \uD7ff))
                                               ⬆
WARNING: cljs.core/bit-or, all arguments must be numbers, got [string number] instead. at line 1 
                                               ⬆
WARNING: cljs.core/bit-or, all arguments must be numbers, got [string number] instead. at line 1 
#'cljs.user/upper-limit
cljs.user=>

It seems like the \u unicode notation is not really tranformed because I see the code compiled is something like:

cljs.tools.reader.edn.upper_limit = ("\uD7FF" | (0));

While probably this should be converted to an integer?

Not sure how this should be handled actually.



 Comments   
Comment by Andrea Richiardi [ 16/Jan/18 6:26 PM ]

I can provide a simple build script for lumo to reproduce the issue within the tools.reader repository.

Comment by Andrea Richiardi [ 16/Jan/18 6:32 PM ]

Can confirm that changing to:

(def ^:private ^:const upper-limit 55295)
(def ^:private ^:const lower-limit 57344)

Get rids of the warning. I don't think it's desirable, just wanted to naively verify.

Comment by Mike Fikes [ 16/Jan/18 6:36 PM ]

It appears that these lines were ported correctly:
https://github.com/clojure/tools.reader/blob/80c9f0311cf476f44645cfab2f78ac62c0da2d78/src/main/cljs/cljs/tools/reader.cljs#L140-L141

While these were not:
https://github.com/clojure/tools.reader/blob/80c9f0311cf476f44645cfab2f78ac62c0da2d78/src/main/cljs/cljs/tools/reader/edn.cljs#L117-L118

Comment by Andrea Richiardi [ 16/Jan/18 6:41 PM ]

Oh nice catch! I can put a patch together.

Comment by Andrea Richiardi [ 16/Jan/18 7:07 PM ]

Ok done, patch attached





Generated at Tue Jan 16 21:05:59 CST 2018 using JIRA 4.4#649-r158309.