[CLJ-1056] defprotocol: invalid method overload syntax getting accepted Created: 01/Sep/12 Updated: 25/Oct/13 Resolved: 25/Oct/13
|Affects Version/s:||Release 1.5|
|Fix Version/s:||Release 1.6|
|Reporter:||Víctor M. Valenzuela||Assignee:||Alex Miller|
|Attachments:||clj-1056-1.txt clj-1056-2.diff clj-1056-2.txt|
|Patch:||Code and Test|
The compiler accepts this erroneous form:
Analysis: defprotocol silently assoc's the last list of signatures found for any particular method name, without checking whether the method name was given earlier.
Approach: Modify defprotocol to check whether each method name has already been encountered earlier, and throw an exception if so. The patch also updates the error message for the case of a protocol function with no args specified.
Behavior with patch clj-1056-2.txt:
Screened by: Stuart Halloway
|Comment by Timothy Baldridge [ 29/Nov/12 4:02 PM ]|
Can not reproduce the fist error:
user=> (defprotocol Foo (f ([this]) ([this arg])))
But the 2nd one I can reproduce:
user=> (defprotocol Bar (m [this]) (m [this arg]))
Notice that :arglists only has one entry
|Comment by Alex Miller [ 22/Aug/13 10:36 PM ]|
Moving back to Triaged as Rich has not vetted.
|Comment by Andy Fingerhut [ 10/Sep/13 11:24 PM ]|
Patch clj-1056-1.txt changes defprotocol to throw an exception if the same method name appears more than once.
Without this patch, the last set of signatures for a method name "wins", silently overriding any earlier ones.
|Comment by Alex Miller [ 18/Oct/13 11:08 AM ]|
Updated patch to improve error messages and to switch from CompilerException (which is not used outside the Compiler) back to IllegalArgumentException.
I think it would be a good idea to have a general Exception that could carry file/line/col info that could be used by Compiler but also other errors (the EdnReader and LispReader both have their own variants). But I think that should be a separate enhancement, which I have filed as CLJ-1280.