Clojure

Unintuitive error response in clojure 1.0

Details

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

Description

The following broken code:

(let [[x y] {}] x)

provides the following stack trace:

Exception in thread "main" java.lang.UnsupportedOperationException: nth not supported on this type: PersistentArrayMap (test.clj:0)
at clojure.lang.Compiler.eval(Compiler.java:4543)
at clojure.lang.Compiler.load(Compiler.java:4857)
at clojure.lang.Compiler.loadFile(Compiler.java:4824)
at clojure.main$load_script__5833.invoke(main.clj:206)
at clojure.main$script_opt__5864.invoke(main.clj:258)
at clojure.main$main__5888.doInvoke(main.clj:333)
at clojure.lang.RestFn.invoke(RestFn.java:413)
at clojure.lang.Var.invoke(Var.java:346)
at clojure.lang.AFn.applyToHelper(AFn.java:173)
at clojure.lang.Var.applyTo(Var.java:463)
at clojure.main.main(main.java:39)
Caused by: java.lang.UnsupportedOperationException: nth not supported on this type: PersistentArrayMap
at clojure.lang.RT.nth(RT.java:800)
at clojure.core$nth__3578.invoke(core.clj:873)
at user$eval__1.invoke(test.clj:1)
at clojure.lang.Compiler.eval(Compiler.java:4532)
... 10 more

The message "nth not supported on this type" while correct doesn't make the cause of the error very clear. Better error messages when destructuring would be very helpful.

  1. CLJ-5.patch
    11/Nov/11 7:38 PM
    2 kB
    Eugene Koontz
  2. clj-5-destructure-error.diff
    28/Apr/12 10:46 PM
    6 kB
    Carin Meier

Activity

Hide
Assembla Importer added a comment -
Show
Assembla Importer added a comment - Converted from http://www.assembla.com/spaces/clojure/tickets/5
Hide
Eugene Koontz added a comment -

Please see the attached patch which produces a (hopefully more clear) error message as shown below (given the broken code shown in the original bug report):

Clojure 1.4.0-master-SNAPSHOT
user=> (let [x 42 y 43] (+ x y))
85
user=> (let [[x y] {}] x)
UnsupportedOperationException left side of binding must be a symbol (found a PersistentVector instead).  clojure.lang.Compiler.checkLet (Compiler.java:6545)
user=>

In addition, this patch checks the argument of (let) as shown below:

user=> (let 42)
UnsupportedOperationException argument to (let)  must be a vector (found a Long instead).  clojure.lang.Compiler.checkLet (Compiler.java:6553)
Show
Eugene Koontz added a comment - Please see the attached patch which produces a (hopefully more clear) error message as shown below (given the broken code shown in the original bug report):
Clojure 1.4.0-master-SNAPSHOT
user=> (let [x 42 y 43] (+ x y))
85
user=> (let [[x y] {}] x)
UnsupportedOperationException left side of binding must be a symbol (found a PersistentVector instead).  clojure.lang.Compiler.checkLet (Compiler.java:6545)
user=>
In addition, this patch checks the argument of (let) as shown below:
user=> (let 42)
UnsupportedOperationException argument to (let)  must be a vector (found a Long instead).  clojure.lang.Compiler.checkLet (Compiler.java:6553)
Hide
Eugene Koontz added a comment -

Patch produced by doing git diff against commit ba930d95fc (master branch).

Show
Eugene Koontz added a comment - Patch produced by doing git diff against commit ba930d95fc (master branch).
Eugene Koontz made changes -
Field Original Value New Value
Attachment CLJ-5.patch [ 10697 ]
Hide
Eugene Koontz added a comment -

Sorry, this patch is wrong: it assumes that the left side of the binding is wrong - the [x y] in :

(let [[x y] {}] x)

because [x y] is a vector, when in fact, the left side is fine (per http://clojure.org/special_forms#let : "Clojure supports abstract structural binding, often called destructuring, in let binding lists".)

So it's the right side (the {}) that needs to be checked and flagged as erroneous, not the [x y].

Show
Eugene Koontz added a comment - Sorry, this patch is wrong: it assumes that the left side of the binding is wrong - the [x y] in :
(let [[x y] {}] x)
because [x y] is a vector, when in fact, the left side is fine (per http://clojure.org/special_forms#let : "Clojure supports abstract structural binding, often called destructuring, in let binding lists".) So it's the right side (the {}) that needs to be checked and flagged as erroneous, not the [x y].
Hide
Carin Meier added a comment -

Add patch better-error-for-let-vector-map-binding

This produces the following:

(let [[x y] {}] x)
Exception map binding to vector is not supported

There are other cases that are not handled by this though — like binding vector to a set

user=> (let [[x y] #{}] x)
UnsupportedOperationException nth not supported on this type: PersistentHashSet

Wondering if it might be better to try convert the map to a seq to support? Although this might be another issue.

Thoughts?

Show
Carin Meier added a comment - Add patch better-error-for-let-vector-map-binding This produces the following:
(let [[x y] {}] x)
Exception map binding to vector is not supported
There are other cases that are not handled by this though — like binding vector to a set
user=> (let [[x y] #{}] x)
UnsupportedOperationException nth not supported on this type: PersistentHashSet
Wondering if it might be better to try convert the map to a seq to support? Although this might be another issue. Thoughts?
Carin Meier made changes -
Attachment better-error-for-let-vector-map-binding.patch [ 10725 ]
Hide
Aaron Bedra added a comment -

This seems too specific. Is this issue indicative of a larger problem that should be addressed? Even if this is the only case where bindings produce poor error messages, all the cases described above should be addressed in the patch.

Show
Aaron Bedra added a comment - This seems too specific. Is this issue indicative of a larger problem that should be addressed? Even if this is the only case where bindings produce poor error messages, all the cases described above should be addressed in the patch.
Aaron Bedra made changes -
Patch Code and Test [ 10002 ]
Approval Incomplete [ 10006 ]
Priority Minor [ 4 ]
Reporter Assembla Importer [ importer ]
Carin Meier made changes -
Attachment clj-5-part2.patch [ 10748 ]
Attachment clj-5-part1.patch [ 10747 ]
Hide
Carin Meier added a comment -

Unfortunately, realized that this still does not cover the nested destructuring cases. Coming to the conclusion, that my approach above is not going to work for this.

Show
Carin Meier added a comment - Unfortunately, realized that this still does not cover the nested destructuring cases. Coming to the conclusion, that my approach above is not going to work for this.
Carin Meier made changes -
Attachment clj-5-part2.patch [ 10748 ]
Carin Meier made changes -
Attachment clj-5-part1.patch [ 10747 ]
Carin Meier made changes -
Attachment better-error-for-let-vector-map-binding.patch [ 10725 ]
Carin Meier made changes -
Comment [ Based on feedback, expanded checker to look for nth errors.
Uses patches: clj-5-part1.patch and clj5-part2.patch

Will produce
{code}
user=> (let [[x y] {}] x)
Exception let cannot destructure class clojure.lang.PersistentArrayMap. Try converting it to a seq. clojure.core/check-nth-errors (core.clj:3909)
user=> (let [[x y] #{}] x)
Exception let cannot destructure class clojure.lang.PersistentHashSet. Try converting it to a seq. clojure.core/check-nth-errors (core.clj:3909)
user=> (let [[x y] 1] x)
Exception let cannot destructure class java.lang.Long. clojure.core/check-nth-errors (core.clj:3910)
{code} ]
Hide
Carin Meier added a comment - - edited

File: clj-5-destructure-error.diff

Added support for nested destructuring errors

let [[[x1 y1][x2 y2]] [[1 2] {}]]
;=> UnsupportedOperationException let cannot destructure class clojure.lang.PersistentArrayMap.
Show
Carin Meier added a comment - - edited File: clj-5-destructure-error.diff Added support for nested destructuring errors
let [[[x1 y1][x2 y2]] [[1 2] {}]]
;=> UnsupportedOperationException let cannot destructure class clojure.lang.PersistentArrayMap.
Carin Meier made changes -
Attachment clj-5-destructure-error.diff [ 11124 ]
Alex Miller made changes -
Approval Incomplete [ 10006 ] Vetted [ 10003 ]
Alex Miller made changes -
Labels errormsgs
Hide
Kevin Downey added a comment -

I am not wild about that error message, let can destructure a map fine.

If there error message were to change, I would prefer to get something like "sequential destructing not supported on maps".

I actually like the "nth not supported" error message, because it is exactly the problem, nth, used by sequential destructuring, doesn't work on maps.

it conveys exactly what the problem is if you know how destructing works and what nth means, where as "UnsupportedOperationException let cannot destructure class clojure.lang.PersistentArrayMap" seems misleading when you are in the know

Show
Kevin Downey added a comment - I am not wild about that error message, let can destructure a map fine. If there error message were to change, I would prefer to get something like "sequential destructing not supported on maps". I actually like the "nth not supported" error message, because it is exactly the problem, nth, used by sequential destructuring, doesn't work on maps. it conveys exactly what the problem is if you know how destructing works and what nth means, where as "UnsupportedOperationException let cannot destructure class clojure.lang.PersistentArrayMap" seems misleading when you are in the know

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated: