Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

defprotocol: invalid method overload syntax getting accepted

Description

The compiler accepts this erroneous form:

user=> (defprotocol Bar (m [this]) (m [this arg])) Bar

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.

Patch: clj-1056-2.diff

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:

user=> (defprotocol Bar (m [this]) (m [this arg])) IllegalArgumentException Function m in protocol Bar was redefined. Specify all arities in single definition. user=> (defprotocol Foo (m [])) IllegalArgumentException Definition of function m in protocol Foo must take at least one arg.

Screened by: Stuart Halloway

Environment

None

Attachments

3
  • 22 Oct 2013, 03:14 PM
  • 18 Oct 2013, 05:08 PM
  • 11 Sep 2013, 05:24 AM

Activity

Show:

Alex Miller October 18, 2013 at 5:08 PM

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.

Andy Fingerhut September 11, 2013 at 5:24 AM

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.

Alex Miller August 23, 2013 at 4:36 AM

Moving back to Triaged as Rich has not vetted.

Timothy Baldridge November 29, 2012 at 10:02 PM

Can not reproduce the fist error:

user=> (defprotocol Foo (f ([this]) ([this arg])))
CompilerException java.lang.IllegalArgumentException: Parameter declaration missing, compilingNO_SOURCE_PATH:5:1)

But the 2nd one I can reproduce:

user=> (defprotocol Bar (m [this]) (m [this arg]))
Bar
user=> Bar
{:on-interface user.Bar, :on user.Bar, :sigs {:m {:doc nil, :arglists ([this arg]), :name m}}, :var #'user/Bar, :method-map {:m :m}, :method-builders {#'user/m #<user$eval71$fn_72 user$eval71$fn_72@1a2b53fb>}}
user=>

Notice that :arglists only has one entry

Vetting

Completed

Details

Assignee

Reporter

Labels

Approval

Ok

Patch

Code and Test

Priority

Affects versions

Fix versions

Created September 2, 2012 at 1:19 AM
Updated October 25, 2013 at 10:03 PM
Resolved October 25, 2013 at 10:03 PM

Flag notifications