Fix confusing macroexpand1 ArityException handling

Description

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:

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.

Proposed: Store full class name in ArityException, demunge in exception string (so no change there), then munge the macro being expanded and compare to the munged class name in ArityException. If the same, then the arity problem is in the macro, otherwise from some function inside the macro.

Patch: clj-1279-3.patch

Environment

None

Activity

Show:
Alex Coventry
October 25, 2013, 4:26 AM

This patch causes Var.setMacro to set instance attribute AFn.macrop to true, so that AFn.throwArity can reduce the number of arguments reported.

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.

The real drawback I see with this approach is the duplicated state, though: ^{:macro true} vs AFn.macrop==true.

Andy Fingerhut
October 26, 2013, 12:33 AM

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.

Alex Coventry
August 20, 2018, 7:59 AM

Wow, that's a blast from the past. Glad it's proved useful!

Alex Miller
August 22, 2018, 4:44 AM

Attached -3 patch with new approach.

Alex Coventry
August 22, 2018, 4:49 AM

That does look like a better approach.

Completed

Assignee

Alex Miller

Reporter

Alex Coventry

Approval

Ok

Patch

Code and Test

Fix versions

Affects versions

Priority

Major
Configure