<< Back to previous view

[CLJS-471] Empty regexp causes Closure Compiler error Created: 13/Feb/13  Updated: 24/Oct/13  Resolved: 24/Oct/13

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Bodil Stokke Assignee: Michał Marczyk
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-471-prevent-empty-regexps-from-causing-compiler.patch     File CLJS-471-2.diff    
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.

#"" should instead produce either /(?:)/ or new RegExp("").



 Comments   
Comment by Michał Marczyk [ 06/Apr/13 6:50 PM ]

Commit message:

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.
Comment by David Nolen [ 24/Apr/13 9:52 PM ]

I tried applying this in master, the included test fails.

Comment by Michał Marczyk [ 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.

Comment by Chas Emerick [ 22/Oct/13 5:02 AM ]

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.

Comment by David Nolen [ 24/Oct/13 2:56 PM ]

fixed, https://github.com/clojure/clojurescript/commit/acf3a6dda7c5808f5210a59ecf6d6f3bdca1d63f

Generated at Sun Nov 23 11:32:22 CST 2014 using JIRA 4.4#649-r158309.