<< Back to previous view

[CLJ-1436] Deref throws an unhelpful error message when used on something not dereferencable Created: 03/Jun/14  Updated: 26/Mar/15  Resolved: 25/Mar/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Phillip Lord Assignee: Unassigned
Resolution: Duplicate Votes: 3
Labels: errormsgs, newbie

Attachments: Text File deref.patch     File tests-patch.diff    
Patch: Code and Test

 Description   

Consider the following code:

(def x 1)
(def y (ref 2))

(+ @x y)

Clojure throws a ClassCastException on cast to Future. This is a very unhelpful error message; why a Future, why not Ref, Atom etc. It would be nice if this failed more gracefully.



 Comments   
Comment by Tobias Kortkamp [ 15/Sep/14 2:08 PM ]

Attached a patch with better error messages for deref. The above example now throws:

IllegalArgumentException class java.lang.Long is not derefable  clojure.core/deref (core.clj:2211)

and e.g.

(deref (delay 1) 500 :foo)

throws

IllegalArgumentException class clojure.lang.Delay is not derefable with a timeout  clojure.core/deref (core.clj:2222)
Comment by Mark Nutter [ 16/Sep/14 6:57 PM ]

Patch file clj-1436-patch-2014-09-16.diff updates the deref function so that it checks whether its arg is a future before sending it to deref-future. It also updates the deref function to provide clearer error messages. If the arg is not a future, and does not implement IDeref, the patched version of deref throws an IllegalArgumentException with a message that the arg cannot be dereferenced because it is not a ref/future/etc.

Comment by Mark Nutter [ 16/Sep/14 7:00 PM ]

Oops, I had this page open from yesterday and didn't see the patch submitted by Tobias. His has everything mine does, so I'll withdraw mine.

Comment by Mark Nutter [ 17/Sep/14 5:13 AM ]

One suggestion: the error message might sound better as "IllegalArgumentException cannot dereference clojure.lang.Delay; not a future or reference type".

Comment by Mark Nutter [ 17/Sep/14 5:44 AM ]

Tobias' patch does not contain the tests I had in mine, so I'm re-submitting just the tests as tests-patch.diff. If you install the tests patch without installing the deref patch, the tests will fail with the error message "Wrong exception type when passing non-IDeref/non-future to deref/@". Applying the deref patch as well will allow the tests to pass.

Comment by Alex Miller [ 28/Dec/14 11:06 AM ]

This patch does not seem to consider performance implications of adding this check and its impact on inlining.

Comment by Nicola Mometto [ 25/Mar/15 5:45 PM ]

Dupe of CLJ-1162

Comment by Andy Fingerhut [ 26/Mar/15 10:13 AM ]

Maybe it shouldn't be the deciding factor when selecting which duplicate ticket to close, but CLJ-1162 has 1 vote, this one has 3. Closing this one as the duplicate is discarding those votes, unless (a) they are copied over to the older ticket, or (b) consider closing the older ticket as a duplicate of this one instead.

Comment by Alex Miller [ 26/Mar/15 10:59 AM ]

I pondered that myself - I usually look at votes, current status, and patch. In this case, I liked the patch on CLJ-1162 better (although they were pretty similar).

Generated at Fri Apr 24 19:35:44 CDT 2015 using JIRA 4.4#649-r158309.