ClojureScript

Warnings are generated when using a macro in argument position

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: 1.7.145
  • Fix Version/s: Next
  • Component/s: None
  • Labels:
    None
  • 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.

  1. CLJS-620.diff
    15/Oct/13 12:47 PM
    14 kB
    Jonas Enlund
  2. CLJS-620.patch
    01/Feb/16 6:34 PM
    2 kB
    Mike Fikes

Activity

Hide
Jozef Wagner added a comment -

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.

Show
Jozef Wagner added a comment - 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.
Hide
Jonas Enlund added a comment - - edited

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

Show
Jonas Enlund added a comment - - edited 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
Hide
Julien Eluard added a comment -

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.

Show
Julien Eluard added a comment - 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.
Hide
David Nolen added a comment -

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.

Show
David Nolen added a comment - 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.
Hide
Mike Fikes added a comment -

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.)

Show
Mike Fikes added a comment - 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.)
Hide
Mike Fikes added a comment -

Confirmed fixed with cljs.jar and downstream bootstrap.

Show
Mike Fikes added a comment - Confirmed fixed with cljs.jar and downstream bootstrap.

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: