<< Back to previous view

[TRDR-51] Invalid character literal message malformed in ClojureScript Created: 18/Jan/18  Updated: 18/Jan/18  Resolved: 18/Jan/18

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: errormsgs

Attachments: Text File TRDR-51.patch    
Patch: Code and Test

 Description   

In the Clojure an attempt to read an invalid character literal such as \ud800 produces an error message "Invalid character literal \ud800", but the ClojureScript implementation inadvertently attempts to put the character after the \u instead of the hexadecimal representation.



 Comments   
Comment by Nicola Mometto [ 18/Jan/18 1:00 PM ]

Thanks, fixed: https://github.com/clojure/tools.reader/commit/105092b84ab24b5cc57ac75efdd67cfa7d74c92e





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

Status: Closed
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: Completed 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

Comment by Nicola Mometto [ 17/Jan/18 3:48 AM ]

Thanks, fixed: https://github.com/clojure/tools.reader/commit/c5421fd8809784ef893e2afaf0d8996229864908





Generated at Sun Jan 21 00:59:41 CST 2018 using JIRA 4.4#649-r158309.