ClojureScript

Details

Type:
Defect

Status:
Closed

Priority:
Trivial

Resolution:
Completed

Affects Version/s:
None

Fix Version/s:
None

Component/s:
None

Labels:

None

Patch:

Code and Test

Description

An empty regexp of the form #"" compiles to //, which is not an empty regexp but a comment, causing the code following the supposed regexp on the affected line to be commented out. This tends to cause compiler errors when Closure Compiler tries to parse it.

Activity

CLJS-471: prevent empty regexps from causing compiler errors
This patch chooses to emit
(new RegExp(""))
rather than
/(?:)/
so that (pr-str #"") returns
"#\"\""
rather than
"#\"(?:)\""
A test for the above is included.

CLJS-471: prevent empty regexps from causing compiler errors
This patch chooses to emit
(new RegExp(""))
rather than
/(?:)/
so that (pr-str #"") returns
"#\"\""
rather than
"#\"(?:)\""
A test for the above is included.

Thanks for letting me know. I've checked the value of new RegExp("").source in a Node.js REPL (0.10.3) and it turns out to be '(?'. So, how important is it that we return "#\"\"" rather than "#\"(?\"" anyway? I'm thinking maybe not that much, seeing how regexp support in JS is different to that on the JVM in more fundamental ways. I'll just assume this is correct and attach a new patch accepting both representations soon. Of course if there's a better way to do it, I'll be happy to implement it.

Michał Marczyk
added a comment - 25/Apr/13 5:57 AM Thanks for letting me know. I've checked the value of new RegExp("").source in a Node.js REPL (0.10.3) and it turns out to be '(?'. So, how important is it that we return "#\"\"" rather than "#\"(?\"" anyway? I'm thinking maybe not that much, seeing how regexp support in JS is different to that on the JVM in more fundamental ways. I'll just assume this is correct and attach a new patch accepting both representations soon. Of course if there's a better way to do it, I'll be happy to implement it.

Added updated patch (471-2.diff) to account for the differences in how empty regexes print in different JS engines. I don't think there's any way to cleanly get away from this, unless we want to s/"(?"/""/g when printing regexes...but that'd be a different ticket, and perhaps not worth the trouble?

Michał, git simply refused to apply your prior patch (didn't even produce conflict changes?), so I just recreated it with the necessary tweak. This eliminated any attribution for you. Sorry about that; if anyone knows of a way to fix that, do let me know.

Chas Emerick
added a comment - 22/Oct/13 5:02 AM - edited Added updated patch (471-2.diff) to account for the differences in how empty regexes print in different JS engines. I don't think there's any way to cleanly get away from this, unless we want to s/"(?"/""/g when printing regexes...but that'd be a different ticket, and perhaps not worth the trouble?
Michał, git simply refused to apply your prior patch (didn't even produce conflict changes?), so I just recreated it with the necessary tweak. This eliminated any attribution for you. Sorry about that; if anyone knows of a way to fix that, do let me know.