[CLJ-1261] Invalid defrecord results in exception attributed to namespace that imports namespace with defrecord Created: 12/Sep/13 Updated: 18/Apr/14
|Affects Version/s:||Release 1.5|
|Fix Version/s:||Release 1.7|
|Reporter:||Howard Lewis Ship||Assignee:||Unassigned|
I was introducing a namespace that included a defrecord.
My defrecord was wrong; it used a keyword to define a field, not a symbol. Minimal test case:
However, the exception was perplexing:
The error was attributed to app.clj (useclj16.app), a namespace which requires useclj16.init, the namespace containing the defrecord.
No indication that this concerned a defrecord, or even what namespace contained the error, was present in the exception.
Approach: Check explicitly that the fields are all symbols, for both defrecord and deftype, and throw a CompilerException with file, line, and column number if not. Example of exception after patch is applied, in the case give above:
|Comment by Alex Miller [ 12/Sep/13 8:58 PM ]|
Can you include an example of the defrecord definition just so we're clear what it looks like?
|Comment by Alex Miller [ 12/Sep/13 8:59 PM ]|
Also, "feedback" is not a useful label. Please use "errormsgs" for stuff like this. See the list of many commonly used labels here: http://dev.clojure.org/display/community/Creating+Tickets
|Comment by Howard Lewis Ship [ 13/Sep/13 10:42 AM ]|
"Feedback" is my own personal crusade http://tapestryjava.blogspot.com/2013/05/once-more-feedback-please.html
In my case, my invalid code was:
And the mistake was that :shutdown-fn should be a symbol, not a keyword.
Here it is, more completely:
The error was attributed to this file.
|Comment by Andy Fingerhut [ 13/Sep/13 11:48 AM ]|
Patch clj-1261-v1.txt throws an exception if any fields given to defrecord or deftype are not symbols. They are CompilerExceptions, so include an accurate file, line, and column number.
|Comment by Andy Fingerhut [ 13/Sep/13 11:57 AM ]|
Updated description to give minimal test case.
|Comment by Andy Fingerhut [ 23/Nov/13 1:06 AM ]|
Patch clj-1261-2.diff is identical to clj-1261-v1.txt except that it applies cleanly to latest master. The only change was to some lines of context due to recent commits to Clojure.
|Comment by Alex Miller [ 18/Apr/14 1:16 PM ]|
I think the patch is ok but I have two suggestions in the error message - first, include the record/type ns+name (I think the classname in the patched fn is what you want). Second, I think the wording could be adjusted a bit and the parens should go away - those look like but don't actually have meaning in the original context (since you are filtering out the symbols). Maybe something like:
"defrecord and deftype fields must be symbols, useclj16.init.Application had: :shutdown-fn, :foo-bar"