Clojure

Deref throws an unhelpful error message when used on something not dereferencable

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Triaged

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.

  1. deref.patch
    15/Sep/14 2:08 PM
    2 kB
    Tobias Kortkamp
  2. tests-patch.diff
    17/Sep/14 5:44 AM
    1.0 kB
    Mark Nutter

Activity

Alex Miller made changes -
Field Original Value New Value
Approval Triaged [ 10120 ]
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.
Consider the following code:

{code}
(def x 1)
(def y (ref 2))

(+ @x y)
{code}

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.
Issue Type Defect [ 1 ] Enhancement [ 4 ]
Alex Miller made changes -
Labels errormsgs errormsgs newbie
Hide
Tobias Kortkamp added a comment -

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)
Show
Tobias Kortkamp added a comment - 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)
Tobias Kortkamp made changes -
Attachment deref.patch [ 13341 ]
Hide
Mark Nutter added a comment - - edited

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.

Show
Mark Nutter added a comment - - edited 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.
Mark Nutter made changes -
Attachment clj-1436-patch-2014-09-16.diff [ 13344 ]
Hide
Mark Nutter added a comment -

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.

Show
Mark Nutter added a comment - 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.
Mark Nutter made changes -
Attachment clj-1436-patch-2014-09-16.diff [ 13344 ]
Hide
Mark Nutter added a comment -

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

Show
Mark Nutter added a comment - One suggestion: the error message might sound better as "IllegalArgumentException cannot dereference clojure.lang.Delay; not a future or reference type".
Hide
Mark Nutter added a comment -

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.

Show
Mark Nutter added a comment - 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.
Mark Nutter made changes -
Attachment tests-patch.diff [ 13345 ]
Andy Fingerhut made changes -
Patch Code and Test [ 10002 ]

People

Vote (3)
Watch (1)

Dates

  • Created:
    Updated: