It's preferable to throw ex-info, look around the compiler for an example. Also this error should include file, line and column information - again look around for ex-info examples to see what I mean. Thanks!

David Nolen
added a comment - 17/Oct/13 2:30 PM It's preferable to throw ex-info, look around the compiler for an example. Also this error should include file, line and column information - again look around for ex-info examples to see what I mean. Thanks!

Travis Thieman
added a comment - 17/Oct/13 3:59 PM - edited v2 of the 20131017 patch uses ex-info to throw a more descriptive error message. To support this, ana/cljs-file is now bound in closure.clj/compile-file.

The `binding` to setup `cljs.analyzer/cljs-file` is not necessary. If it is necessary you will need to show why it is. `cljs.compiler/compile-file` calls `cljs.compiler/compile-file*` which already does this.

David Nolen
added a comment - 17/Oct/13 4:13 PM The `binding` to setup `cljs.analyzer/cljs-file` is not necessary. If it is necessary you will need to show why it is. `cljs.compiler/compile-file` calls `cljs.compiler/compile-file*` which already does this.

I encountered this as an issue because of the specific way I was testing compilation. I create a test file and compiled it using "/bin/cljsc test.cljs". This compiles from source files.

However, when it calls compile-file, no output file is specified, instead writing to stdout, e.g. opts contains {:output-to :print} and does not contain :output-file. As a result, the second branch of the if gets called, which compiles a sequence of forms instead of a file. As a result, the cljs-file never gets bound, even though there are input files.

The binding should still be moved to only affect the compile-form-seq branch.

Travis Thieman
added a comment - 17/Oct/13 4:26 PM I encountered this as an issue because of the specific way I was testing compilation. I create a test file and compiled it using "/bin/cljsc test.cljs". This compiles from source files.
However, when it calls compile-file, no output file is specified, instead writing to stdout, e.g. opts contains {:output-to :print} and does not contain :output-file. As a result, the second branch of the if gets called, which compiles a sequence of forms instead of a file. As a result, the cljs-file never gets bound, even though there are input files.
The binding should still be moved to only affect the compile-form-seq branch.

Upon further reflection, it's undesirable for this to throw. We should just convert it into a warning. I need to refactor warnings a bit so patch for this ticket should hold off until I get around to that.

David Nolen
added a comment - 18/Oct/13 10:23 AM Upon further reflection, it's undesirable for this to throw. We should just convert it into a warning. I need to refactor warnings a bit so patch for this ticket should hold off until I get around to that.

If the exception is not thrown here, a more generic exception is still thrown further down the line after the defprotocol macroexpansion, something about an invalid dot form. Is the plan to also fix that so that (defprotocol P (f [])) would fully compile? If not, what are the reasons for not wanting to throw here?

Travis Thieman
added a comment - 18/Oct/13 10:42 AM If the exception is not thrown here, a more generic exception is still thrown further down the line after the defprotocol macroexpansion, something about an invalid dot form. Is the plan to also fix that so that (defprotocol P (f [])) would fully compile? If not, what are the reasons for not wanting to throw here?