<< Back to previous view

[CLJS-620] Warnings are generated when using a macro in argument position Created: 14/Oct/13  Updated: 04/Feb/16  Resolved: 04/Feb/16

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

Type: Defect Priority: Minor
Reporter: Julien Eluard Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File CLJS-620.diff     Text File CLJS-620.patch    
Patch: Code

 Description   

Using a macro in argument position (e.g. (map macro [])) generates a warning:
WARNING: Use of undeclared Var test/node at line 4 src/test.cljs

Find a reproduction project here.



 Comments   
Comment by Jozef Wagner [ 15/Oct/13 3:30 AM ]

and what would you like, a better warning? Clojurescript allows same name for macro and for function, so you can both have macro + and fn +. Macro version will be used when is first in the list, fn version otherwise.

Comment by Jonas Enlund [ 15/Oct/13 3:38 AM ]

For reference, Clojure generates the following error message:

user=> (map when [])
CompilerException java.lang.RuntimeException: Can't take value of a macro: #'clojure.core/when, compiling:(NO_SOURCE_PATH:1:1)

The "obvious" approach would be to add

(when-let [m (get-expander sym env)]
  (throw (error env (str "Can’t take value of a macro: " m))))

to resolve-var[1]. Unfortunately this doesn’t work in ClojureScript due to the way inlining works. A simple workaround is to add {:inline true} metadata to macros that are later redefined as functions in core.cljs and check for invalid macro usage like this:

(when-let [m (get-expander sym env)]
  (and (-> m meta :inline not)    
       (throw (error env (str "Can’t take value of a macro: " m)))))

Another approach would perhaps be to rethink how inlining works as it seems kind of brittle to have macros in cljs/core.clj with the same name as functions in cljs/core.cljs (especially since both namespaces are auto-imported. Is cljs.core/inc a function, a macro, or both?). Maybe there’s a better way?

[1] https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/analyzer.clj#L193

Comment by Julien Eluard [ 15/Oct/13 6:23 AM ]

My bad, didn't realize it didn't make sense. Of course it's obvious now. I was confused by recent changes around function/macro name validation.
Now the warning could probably be improved a little but that doesn't seem very important. The Clojure warning is not that much better.

Comment by David Nolen [ 15/Oct/13 11:58 AM ]

We're not going to change how inlining works - the whole point is that we can reuse the same names. Adding :inline metadata seems like a promising way to warn of incorrect usage of inlining macros.

Comment by Mike Fikes [ 01/Feb/16 6:34 PM ]

Attaching CLJS-620.patch for comment.

It may at least improve things by producing the warning regarding it being a macro.

cljs.user=> (map when [])
WARNING: Can't take value of macro cljs.core/when at line 1 <cljs repl>
()

One advantage of the patch is it is pretty small.

A disadvantage is that it kind-of overloads the existing :undeclared-var warning. (The alternative would be to introduce a completely new warning that is emitted in exactly this situation.)

Comment by David Nolen [ 04/Feb/16 3:53 PM ]

fixed https://github.com/clojure/clojurescript/commit/58cd6be66877ba43d1642138360b916de0983917

Comment by Mike Fikes [ 04/Feb/16 8:41 PM ]

Confirmed fixed with cljs.jar and downstream bootstrap.

Generated at Mon Dec 05 04:39:28 CST 2016 using JIRA 4.4#649-r158309.