Clojure JIRAhttp://dev.clojure.org/jira
This file is an XML representation of an issueen-us4.464925-07-2011[CLJ-1279] Fix confusing macroexpand1 ArityException handlinghttp://dev.clojure.org/jira/browse/CLJ-1279
Clojure<p>macros can give very confusing error messages when they execute a form which generates an ArityException. clojure.lang.Compiler.macroexpand1 assumes that any ArityException comes from the call to the macro itself, which need not be the case. For instance:</p>
<p> user&gt; (do (defmacro f [] (assoc)) (f))<br/>
ArityException Wrong number of args (-2) passed to: core$assoc clojure.lang.Compiler.macroexpand1 (Compiler.java:6488)<br/>
user&gt; (use 'clojure.repl) (pst)<br/>
nil<br/>
ArityException Wrong number of args (-2) passed to: core$assoc<br/>
clojure.lang.Compiler.macroexpand1 (Compiler.java:6488)<br/>
clojure.lang.Compiler.macroexpand (Compiler.java:6544)<br/>
clojure.lang.Compiler.eval (Compiler.java:6618)<br/>
clojure.lang.Compiler.eval (Compiler.java:6624)<br/>
clojure.lang.Compiler.eval (Compiler.java:6597)<br/>
clojure.core/eval (core.clj:2864)<br/>
clojure.main/repl/read-eval-print-<del>6596/fn</del>-6599 (main.clj:260)<br/>
clojure.main/repl/read-eval-print--6596 (main.clj:260)<br/>
clojure.main/repl/fn--6605 (main.clj:278)<br/>
clojure.main/repl (main.clj:278)<br/>
clojure.tools.nrepl.middleware.interruptible-eval/evaluate/fn--1251 (interruptible_eval.clj:56)<br/>
clojure.core/apply (core.clj:617)<br/>
nil</p>
<p>Easy enough to see the source of the problem in this case, but because both the number of arguments actually passed is off by two, and the stacktrace element for the call to assoc has been dropped, this shortcut by macroexpand1 can get super confusing.</p>
<p>The attached patch corrects this behavior. E.g.</p>
<p> user=&gt; (do (defmacro f [] (assoc)) (f))<br/>
ArityException Wrong number of args (0) passed to: core$assoc clojure.lang.AFn.throwArity (AFn.java:437)<br/>
user=&gt; (use 'clojure.repl) (pst)<br/>
nil<br/>
ArityException Wrong number of args (0) passed to: core$assoc<br/>
user/f (NO_SOURCE_FILE:1)<br/>
clojure.lang.Var.invoke (Var.java:419)<br/>
clojure.lang.Var.applyTo (Var.java:532)<br/>
clojure.lang.Compiler.macroexpand1 (Compiler.java:6507)<br/>
clojure.lang.Compiler.macroexpand (Compiler.java:6580)<br/>
clojure.lang.Compiler.eval (Compiler.java:6654)<br/>
clojure.lang.Compiler.eval (Compiler.java:6660)<br/>
clojure.lang.Compiler.eval (Compiler.java:6633)<br/>
clojure.core/eval (core.clj:2864)<br/>
clojure.main/repl/read-eval-print-<del>6594/fn</del>-6597 (main.clj:260)<br/>
clojure.main/repl/read-eval-print--6594 (main.clj:260)<br/>
clojure.main/repl/fn--6603 (main.clj:278)<br/>
nil</p>CLJ-1279Fix confusing macroexpand1 ArityException handlingDefectMajorOpenUnresolvedUnassignedAlex CoventryCompilererrormsgsmacroWed, 16 Oct 2013 22:12:55 -0500Wed, 5 Feb 2014 12:25:20 -0600Release 1.632<p>Patch with test</p><p>Amended patch to deal more gracefully with unexpected stack trace structure.</p><p>Also see <a href="http://dev.clojure.org/jira/browse/CLJ-397" title="better error message when calling macros with arity"><del>CLJ-397</del></a> and <a href="http://dev.clojure.org/jira/browse/CLJ-383" title="different arrities of macro don&#39;t work in 1.2 (20100607 build)"><del>CLJ-383</del></a>. </p><p>Thanks, Alex. It would be easy enough to move most of the logic into ArityException, which would be a compromise between Stu's<span class="error">&#91;1&#93;</span> options 1 and 2. Is that worth doing?</p>
<p>Amending clojure.lang.AFn.throwArity to check whether "this" is a macro and adjust the arg count there accordingly might be the simplest way. I can see why Rich prefers all the logic to go into ArityException, but since ArityExceptions are used for things other than macros, I don't see a way to make an honest error message there without groveling the stack trace.</p>
<p><span class="error">&#91;1&#93;</span> <a href="http://dev.clojure.org/jira/browse/CLJ-397?focusedCommentId=24090&amp;page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-24090">http://dev.clojure.org/jira/browse/CLJ-397?focusedCommentId=24090&amp;page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-24090</a></p><p>I would have to take more time than I have to make an informed opinion but I can say that from a general point of view inspecting StackTraceElements does not seem like the right solution to (almost) any problem. <img class="emoticon" src="http://dev.clojure.org/jira/images/icons/emoticons/smile.gif" height="20" width="20" align="absmiddle" alt="" border="0"/> </p><p>This patch causes Var.setMacro to set instance attribute AFn.macrop to true, so that AFn.throwArity can reduce the number of arguments reported.</p>
<p>I'm not used to negotiating java class hierarchies, so it's possible there's a cleaner way. Since Var.fn() returns an IFn, I added macrop handling methods IFn.setMacro and IFn.isMacro. These then needed to be implemented in Ref and Keyword, as well as AFn (where I wanted them) because they implement the IFn interface but don't inherit from AFn.</p>
<p>The real drawback I see with this approach is the duplicated state, though: ^{:macro true} vs AFn.macrop==true.</p><p>I have not investigated the reason yet, but neither patch applies cleanly after the latest commits to Clojure master on Oct 25 2013. Given that what kinds of solution methods would be acceptable for this issue, it sounds like more thinking and code changes are probably needed anyway before worrying too much about that.</p>Global RankPatchCode and Test