tools.reader

Warning when using upper-limit in self-host cljs

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

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.

Activity

Hide
Andrea Richiardi added a comment -

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

Show
Andrea Richiardi added a comment - I can provide a simple build script for lumo to reproduce the issue within the tools.reader repository.
Hide
Andrea Richiardi added a comment - - edited

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.

Show
Andrea Richiardi added a comment - - edited 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.
Hide
Andrea Richiardi added a comment -

Oh nice catch! I can put a patch together.

Show
Andrea Richiardi added a comment - Oh nice catch! I can put a patch together.
Hide
Andrea Richiardi added a comment -

Ok done, patch attached

Show
Andrea Richiardi added a comment - Ok done, patch attached

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: