Welcome! Please see the About page for a little more info on how this works.

0 votes
in Errors by

The following are invalid and should produce errors when invoked on gen-class or gen-interface:

(gen-interface :name clj1419.IFail :methods [[myMethod java.lang.String]]) ;; no params, throws error (gen-interface :name clj1419.IFail :methods [[myMethod []]]) ;; no return type (gen-interface :name clj1419.IFail :methods [[myMethod]]) ;; no params or return type

The first example throws an error. The second and third do not but will generate an invalid class, verify with:

(.getMethods clj1419.IFail) ClassNotFoundException java.lang. java.net.URLClassLoader$1.run (URLClassLoader.java:366)

Add checks to prevent these errors.

14 Answers

0 votes
by

Comment made by: nathan7

I've implemented both fixes, and attached them as patches.

0 votes
by

Comment made by: nathan7

I'd argue that the behaviour of asm-type is at fault here (it can output an invalid type name when passed a nil argument), so I prefer that fix over the purely symptomatic generate-interface fix.

0 votes
by

Comment made by: jafingerhut

Nathan, were you planning on submitting a signed Clojure Contributor's Agreement, or already have? Details here if you have not: http://clojure.org/contributing

Patches from non-contributors cannot be committed to Clojure.

Note: I cannot promise you that one of your patches will be accepted into Clojure if you sign a CA -- only that it will not if you do not sign one.

0 votes
by

Comment made by: alexmiller

Please add an example of how this happens and the current error.

0 votes
by
_Comment made by: nathan7_

Andy ā€” Yep, I've read up on that. My CA will be underway to Rich soon. (filled in, signed, in an envelope, just need to await the arrival of those bloody international stampsā€¦)

Alex Miller ā€” Tahdah!

A demonstration of the issue, both attached and as a gist: https://gist.github.com/nathan7/3a7e3a09e458f1354cbb
0 votes
by

Comment made by: nathan7

and here's log of the compiler crash that results (also added to the gist now)

0 votes
by

Comment made by: nathan7

Whoops, both of my patches were rather broken due to a misunderstanding on my side.
I forgot entirely that asm-type takes a symbol, not a string.
Modifying asm-type was definitely a bad idea, that check just looks whether it should defer to prim->class.
Adding nil to prim->class would work (and I've attached my patch for that too), but it's starting to look rather inelegant compared to just patching gen-interface.
(on a side note: I'm having a lot of fun exploring the Clojure codebase! thanks for that, humans!)

0 votes
by

Comment made by: alexmiller

My reading of the docstring of gen-interface is that method declarations must specify a parameter list and a valid return type. I would expect all of these to be invalid:

(gen-interface :name clj1419.IFail :methods [[fail nil]]) (gen-interface :name clj1419.IFail :methods [[fail [] nil]]) (gen-interface :name clj1419.IFail :methods [[fail []]])

"nil" is not a valid type - you can use "void" for this purpose and this works fine:

(gen-interface :name clj1419.IFail :methods [[fail [] void]])

If this ticket is (as the title states) a request to allow omitting the return type or using "nil" as a return type, then I think the answer is no. If the ticket is a request to improve the error reporting of the failure cases above, then I think we can consider that but it will be very low priority.

0 votes
by

Comment made by: nathan7

The code seems to suggest otherwise though, seeing the explicit extra branch for pclasses being nil.
As much as I like PL trivia, I haven't run into void in Clojure anywhere else yet, and I'm surprised to see it here.
Maintaining the principle of least surprise seems preferable to pedantry about whether nil is a type: (= "nil" (str (type (.methodReturningVoid obj)))

0 votes
by

Comment made by: alexmiller

The two places to look for words to rely on are docstrings and the http://clojure.org/documentation pages. Implementation details are just that.

"nil" is not a type. "void" is a documented type identifier indicating the absence of a return value - http://clojure.org/java_interop#Java Interop-Aliases

0 votes
by

Comment made by: nathan7

Okay. Better error-checking in asm-type then?

0 votes
by

Comment made by: alexmiller

I have updated the title and description based on my understanding of what this ticket should be, which is enhanced error-checking on the method specs for gen-class and gen-interface. I'm not sure if that's in asm-type or somewhere earlier.

0 votes
by

Comment made by: alexmiller

As of 1.10, these all throw errors now:

`
user=> (gen-interface :name clj1419.IFail :methods [[myMethod java.lang.String]])
Syntax error macroexpanding gen-interface at (REPL:1:1).
Don't know how to create ISeq from: clojure.lang.Symbol

user=> (gen-interface :name clj1419.IFail :methods [[myMethod []]])
Unexpected error (ClassFormatError) macroexpanding gen-interface at (REPL:1:1).
Class name contains illegal character in descriptor in class file clj1419/IFail

user=> (gen-interface :name clj1419.IFail :methods [[myMethod]])
Unexpected error (ClassFormatError) macroexpanding gen-interface at (REPL:1:1).
Class name contains illegal character in descriptor in class file clj1419/IFail
`

0 votes
by
Reference: https://clojure.atlassian.net/browse/CLJ-1419 (reported by nathan7)
...