stu said: The choke point here is line 5286 of Compiler.java. At this point, we know we are in macro, but we don't know we if we have an arity problem. When the arity problem is throw, it is a plain old IllegalArgumentException. Some choices:

(1) Hack: parse the exception, pull out the number, subtract 2. Yuck.

(2) Make the exception smarter (subclass the exception as ArityException or some such, catch that here, subtract 2.

(3) Make AFunction smarter, so that it knows if it is a macro or not (also requires change to defmacro). Then throwArity can do the right thing at the point of the problem.

Option #2 feels smaller than #3, but maybe knowing when an AFunction is AMacro will be useful elsewhere.

Assembla Importer
added a comment - 18/Oct/10 11:07 PM stu said: The choke point here is line 5286 of Compiler.java. At this point, we know we are in macro, but we don't know we if we have an arity problem. When the arity problem is throw, it is a plain old IllegalArgumentException. Some choices:
(1) Hack: parse the exception, pull out the number, subtract 2. Yuck.
(2) Make the exception smarter (subclass the exception as ArityException or some such, catch that here, subtract 2.
(3) Make AFunction smarter, so that it knows if it is a macro or not (also requires change to defmacro). Then throwArity can do the right thing at the point of the problem.
Option #2 feels smaller than #3, but maybe knowing when an AFunction is AMacro will be useful elsewhere.

Assembla Importer
added a comment - 18/Oct/10 11:07 PM fogus said: I agree with #2 also, but in general I think in creating a (reasonable) set of exception subclasses we could start down the path of making error reporting and handling more friendly.

Second patch (arity-unwrapped) subsumes first, and does not return a wrapped exception. Wrapped exception would defeat the purpose of the patch, by giving REPL root-cause consumers an incorrect message.